Re: [PATCH 5/7] grub-core/net/net.c: Fix uninitialized scalar variable

[Glenn Washburn <development@efficientek.com>]

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.

The "{}" syntax seems to not be used much either, "{0}" being
preferred, but also not used much.

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.

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)
    }
  };

There is precedent for using function calls in initializers
(net/dns.c:453), but its also rare.

Daniel, do you have a preference here?

Glenn