[PATCH v2 0/7] Fix coverity uninitialized scalar variable bugs in grub-core

[PATCH v2 0/7] Fix coverity uninitialized scalar variable bugs in grub-core

[Alec Brown]

v2: Set structs with multiple uninitialized members to {0} and set single
uninitialized members to 0.

Coverity identified multiple uninitialized scalar variable bugs in multiple
components of the grub-core. These patches address these issues.

The Coverity bugs being addressed are:
CID 375026
CID 375028
CID 375030
CID 375031
CID 375033
CID 375035
CID 375036

Alec Brown (7):
      grub-core/loader/i386/bsd.c: Fix uninitialized scalar variable
      grub-core/loader/i386/pc/linux.c: Fix uninitialized scalar variable
      grub-core/net/arp.c: Fix uninitialized scalar variable
      grub-core/loader/i386/xnu.c: Fix uninitialized scalar variable
      grub-core/net/net.c: Fix uninitialized scalar variable
      grub-core/loader/i386/xnu.c: Fix uninitialized scalar variable
      grub-core/net/bootp.c: Fix uninitialized scalar variable

 grub-core/loader/i386/bsd.c      | 2 +-
 grub-core/loader/i386/pc/linux.c | 2 +-
 grub-core/loader/i386/xnu.c      | 4 ++--
 grub-core/net/arp.c              | 2 ++
 grub-core/net/bootp.c            | 1 +
 grub-core/net/net.c              | 1 +
 6 files changed, 8 insertions(+), 4 deletions(-)

[PATCH v2 1/7] grub-core/loader/i386/bsd.c: Fix uninitialized scalar variable

[Alec Brown]

In the function grub_netbsd_setup_video(), struct grub_netbsd_btinfo_framebuf
params is called but isn't being initialized. The member grub_uint8_t
reserved[16] isn't set to any values and is instead filled with junk data from
the stack. We can prevent this by setting params to {0}.

Fixes: CID 375026

Signed-off-by: Alec Brown <alec.r.brown@oracle.com>
---
 grub-core/loader/i386/bsd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/grub-core/loader/i386/bsd.c b/grub-core/loader/i386/bsd.c
index 5f3290ce1..de63ca8dc 100644
--- a/grub-core/loader/i386/bsd.c
+++ b/grub-core/loader/i386/bsd.c
@@ -929,7 +929,7 @@ grub_netbsd_setup_video (void)
   struct grub_video_mode_info mode_info;
   void *framebuffer;
   const char *modevar;
-  struct grub_netbsd_btinfo_framebuf params;
+  struct grub_netbsd_btinfo_framebuf params = {0};
   grub_err_t err;
   grub_video_driver_id_t driv_id;
 
-- 
2.27.0

[PATCH v2 2/7] grub-core/loader/i386/pc/linux.c: Fix uninitialized scalar variable

[Alec Brown]

In the function grub_linux16_boot(), struct grub_relocator16_state state is
called but isn't being initialized. This results in the members grub_uint32_t
ebx, grub_uint32_t edx, grub_uint32_t esi, and grub_uint32_t ebp being filled
with junk data from the stack since none of them are being set to any values. We
can prevent this by setting state to {0}.

Fixes: CID 375028

Signed-off-by: Alec Brown <alec.r.brown@oracle.com>
---
 grub-core/loader/i386/pc/linux.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/grub-core/loader/i386/pc/linux.c b/grub-core/loader/i386/pc/linux.c
index 2a2995201..bf4dc0488 100644
--- a/grub-core/loader/i386/pc/linux.c
+++ b/grub-core/loader/i386/pc/linux.c
@@ -55,7 +55,7 @@ static grub_err_t
 grub_linux16_boot (void)
 {
   grub_uint16_t segment;
-  struct grub_relocator16_state state;
+  struct grub_relocator16_state state = {0};
 
   segment = grub_linux_real_target >> 4;
   state.gs = state.fs = state.es = state.ds = state.ss = segment;
-- 
2.27.0

[PATCH v2 3/7] grub-core/net/arp.c: Fix uninitialized scalar variable

[Alec Brown]

In the function grub_net_arp_receive(), grub_net_network_level_address_t
sender_addr and target_addr are being called but aren't being initialized. In
both of these structs, each member is being set to a value except for
grub_dns_option_t option. This results in this member being filled with junk
data from the stack. To prevent this, we can set the option member in both
structs to 0.

Fixes: CID 375030

Signed-off-by: Alec Brown <alec.r.brown@oracle.com>
---
 grub-core/net/arp.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/grub-core/net/arp.c b/grub-core/net/arp.c
index 54306e3b1..1d367436c 100644
--- a/grub-core/net/arp.c
+++ b/grub-core/net/arp.c
@@ -128,6 +128,8 @@ grub_net_arp_receive (struct grub_net_buff *nb, struct grub_net_card *card,
   target_addr.type = GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV4;
   sender_addr.ipv4 = arp_packet->sender_ip;
   target_addr.ipv4 = arp_packet->recv_ip;
+  sender_addr.option = 0;
+  target_addr.option = 0;
   if (arp_packet->sender_ip == pending_req)
     have_pending = 1;
 
-- 
2.27.0

[PATCH v2 4/7] grub-core/loader/i386/xnu.c: Fix uninitialized scalar variable

[Alec Brown]

In the function grub_xnu_boot_resume(), struct grub_relocator32_state state is
called but isn't being initialized. This results in the members grub_uint32_t
ebx, grub_uint32_t ecx, grub_uint32_t edx, grub_uint32_t esi, and grub_uint32_t
edi being filled with junk data from the stack since none of them are being set
to any values. We can prevent this by setting state to {0}.

Fixes: CID 375031

Signed-off-by: Alec Brown <alec.r.brown@oracle.com>
---
 grub-core/loader/i386/xnu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/grub-core/loader/i386/xnu.c b/grub-core/loader/i386/xnu.c
index a70093607..2bc118fc0 100644
--- a/grub-core/loader/i386/xnu.c
+++ b/grub-core/loader/i386/xnu.c
@@ -805,7 +805,7 @@ grub_cpu_xnu_fill_devicetree (grub_uint64_t *fsbfreq_out)
 grub_err_t
 grub_xnu_boot_resume (void)
 {
-  struct grub_relocator32_state state;
+  struct grub_relocator32_state state = {0};
 
   state.esp = grub_xnu_stack;
   state.ebp = grub_xnu_stack;
-- 
2.27.0

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

[Alec Brown]

In the function grub_net_ipv6_get_link_local(), grub_net_network_level_address_t
addr is called but isn't being initialized. This results in the member
grub_dns_option_t option being filled with junk data from the stack. We can
prevent this by setting the option member in addr to 0.

Fixes: CID 375033

Signed-off-by: Alec Brown <alec.r.brown@oracle.com>
---
 grub-core/net/net.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/grub-core/net/net.c b/grub-core/net/net.c
index 4d3eb5c1a..b6eb1f951 100644
--- a/grub-core/net/net.c
+++ b/grub-core/net/net.c
@@ -292,6 +292,7 @@ grub_net_ipv6_get_link_local (struct grub_net_card *card,
   addr.type = GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV6;
   addr.ipv6[0] = grub_cpu_to_be64_compile_time (0xfe80ULL << 48);
   addr.ipv6[1] = grub_net_ipv6_get_id (hwaddr);
+  addr.option = 0;
 
   FOR_NET_NETWORK_LEVEL_INTERFACES (inf)
   {
-- 
2.27.0

[PATCH v2 6/7] grub-core/loader/i386/xnu.c: Fix uninitialized scalar variable

[Alec Brown]

In the function grub_xnu_boot(), struct grub_relocator32_state state is called
but isn't being initialized. This results in the members grub_uint32_t ebx,
grub_uint32_t ecx, grub_uint32_t edx, grub_uint32_t edi, and grub_uint32_t esi
being filled with junk data from the stack since none of them are being set to
any values. We can prevent this by setting state to {0}.

Fixes: CID 375035

Signed-off-by: Alec Brown <alec.r.brown@oracle.com>
---
 grub-core/loader/i386/xnu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/grub-core/loader/i386/xnu.c b/grub-core/loader/i386/xnu.c
index 2bc118fc0..c0fb76df4 100644
--- a/grub-core/loader/i386/xnu.c
+++ b/grub-core/loader/i386/xnu.c
@@ -960,7 +960,7 @@ grub_xnu_boot (void)
   grub_addr_t devtree_target;
   grub_size_t devtreelen;
   int i;
-  struct grub_relocator32_state state;
+  struct grub_relocator32_state state = {0};
   grub_uint64_t fsbfreq = 100000000;
   int v2 = (grub_xnu_darwin_version >= 11);
   grub_uint32_t efi_system_table = 0;
-- 
2.27.0

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

[Alec Brown]

In the function grub_net_configure_by_dhcp_ack(),
grub_net_network_level_address_t addr is called but isn't being initialized.
This results in the member grub_dns_option_t option being filled with junk data
from the stack. To prevent this, we can set the option member in addr to 0.

Fixes: CID 375036

Signed-off-by: Alec Brown <alec.r.brown@oracle.com>
---
 grub-core/net/bootp.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/grub-core/net/bootp.c b/grub-core/net/bootp.c
index 6fb562702..8dbd1b232 100644
--- a/grub-core/net/bootp.c
+++ b/grub-core/net/bootp.c
@@ -244,6 +244,7 @@ grub_net_configure_by_dhcp_ack (const char *name,
 
   addr.type = GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV4;
   addr.ipv4 = bp->your_ip;
+  addr.option = 0;
 
   if (device)
     *device = 0;
-- 
2.27.0

Re: [PATCH v2 0/7] Fix coverity uninitialized scalar variable bugs in grub-core

[Darren Kenny]

Hi Alec,

These changes look good. For the series:

Reviewed-by: Darren Kenny <darren.kenny@oracle.com>

Thanks,

Darren.

On Monday, 2022-03-21 at 02:28:55 -04, Alec Brown wrote:
> v2: Set structs with multiple uninitialized members to {0} and set single
> uninitialized members to 0.
>
> Coverity identified multiple uninitialized scalar variable bugs in multiple
> components of the grub-core. These patches address these issues.
>
> The Coverity bugs being addressed are:
> CID 375026
> CID 375028
> CID 375030
> CID 375031
> CID 375033
> CID 375035
> CID 375036
>
> Alec Brown (7):
>       grub-core/loader/i386/bsd.c: Fix uninitialized scalar variable
>       grub-core/loader/i386/pc/linux.c: Fix uninitialized scalar variable
>       grub-core/net/arp.c: Fix uninitialized scalar variable
>       grub-core/loader/i386/xnu.c: Fix uninitialized scalar variable
>       grub-core/net/net.c: Fix uninitialized scalar variable
>       grub-core/loader/i386/xnu.c: Fix uninitialized scalar variable
>       grub-core/net/bootp.c: Fix uninitialized scalar variable
>
>  grub-core/loader/i386/bsd.c      | 2 +-
>  grub-core/loader/i386/pc/linux.c | 2 +-
>  grub-core/loader/i386/xnu.c      | 4 ++--
>  grub-core/net/arp.c              | 2 ++
>  grub-core/net/bootp.c            | 1 +
>  grub-core/net/net.c              | 1 +
>  6 files changed, 8 insertions(+), 4 deletions(-)

Re: [PATCH v2 0/7] Fix coverity uninitialized scalar variable bugs in grub-core

[Daniel Kiper]

On Mon, Mar 21, 2022 at 12:18:33PM +0000, Darren Kenny wrote:
> Hi Alec,
>
> These changes look good. For the series:
>
> Reviewed-by: Darren Kenny <darren.kenny@oracle.com>

Same for me, Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>...

Daniel