[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 5/7] grub-core/net/net.c: Fix uninitialized scalar variable
From: |
Daniel Kiper |
Subject: |
Re: [PATCH 5/7] grub-core/net/net.c: Fix uninitialized scalar variable |
Date: |
Thu, 17 Mar 2022 22:20:01 +0100 |
User-agent: |
NeoMutt/20170113 (1.7.2) |
On Tue, Mar 15, 2022 at 08:16:01PM -0500, Glenn Washburn wrote:
> On Tue, 15 Mar 2022 16:24:07 -0400
> Alec Brown <alec.r.brown@oracle.com> wrote:
>
> > In the function grub_net_ipv6_get_link_local(),
> > grub_net_network_level_address_t
> > addr is called but isn't being initialized. To prevent contents of this
> > structure from being filled with junk data from the stack, we can
> > initialize it
> > to 0 by setting addr to {};
> >
> > Fixes: CID 375033
> >
> > Signed-off-by: Alec Brown <alec.r.brown@oracle.com>
> > ---
> > grub-core/net/net.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/grub-core/net/net.c b/grub-core/net/net.c
> > index 4d3eb5c1a..4e93365a7 100644
> > --- a/grub-core/net/net.c
> > +++ b/grub-core/net/net.c
> > @@ -287,7 +287,7 @@ grub_net_ipv6_get_link_local (struct grub_net_card
> > *card,
> > struct grub_net_network_level_interface *inf;
> > char *name;
> > char *ptr;
> > - grub_net_network_level_address_t addr;
> > + grub_net_network_level_address_t addr = {};
> >
> > addr.type = GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV6;
> > addr.ipv6[0] = grub_cpu_to_be64_compile_time (0xfe80ULL << 48);
>
> This seems not quite necessary. The local "addr" is initialized just
> below its initialization, so "junk" data doesn't matter. Only the
> "option" member is not initialized, so we could just add another line to
> initialize that.
Yeah, I chatted about that with Alec and he will fix it.
> The "{}" syntax seems to not be used much either, "{0}" being
> preferred, but also not used much.
Good point! I asked Alec to fix it too.
> I think I remember Vladimir saying that GRUB doesn't use initializers,
> but there are some in the code, so perhaps this isn't a thing
> anymore.
If Coverity complains I think we should make it happy. I think it is
better to be on safe side than sorry.
> Another option, which would be my preference, would be to move the 3
> lines below the declaraction of "addr" into the initializer and use
> C99's designated initializer, so something like:
>
> grub_net_network_level_address_t addr = {
> .type = GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV6,
> .ipv6 = {
> grub_cpu_to_be64_compile_time (0xfe80ULL << 48),
> grub_net_ipv6_get_id (hwaddr)
> }
> };
I think it is rather an overkill here.
Daniel
- [PATCH 0/7] Fix coverity uninitialized scalar variable bugs in grub-core, Alec Brown, 2022/03/15
- [PATCH 3/7] grub-core/net/arp.c: Fix uninitialized scalar variable, Alec Brown, 2022/03/15
- [PATCH 2/7] grub-core/loader/i386/pc/linux.c: Fix uninitialized scalar variable, Alec Brown, 2022/03/15
- [PATCH 7/7] grub-core/net/bootp.c: Fix uninitialized scalar variable, Alec Brown, 2022/03/15
- [PATCH 1/7] grub-core/loader/i386/bsd.c: Fix uninitialized scalar variable, Alec Brown, 2022/03/15
- [PATCH 4/7] grub-core/loader/i386/xnu.c: Fix uninitialized scalar variable, Alec Brown, 2022/03/15
- [PATCH 5/7] grub-core/net/net.c: Fix uninitialized scalar variable, Alec Brown, 2022/03/15
- [PATCH 6/7] grub-core/loader/i386/xnu.c: Fix uninitialized scalar variable, Alec Brown, 2022/03/15
- Re: [PATCH 0/7] Fix coverity uninitialized scalar variable bugs in grub-core, Darren Kenny, 2022/03/15