grub-devel
[Top][All Lists]
Advanced

[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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]