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.br...@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.br...@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
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
