[PATCH] btrfs: avoid "used uninitialized" error with GCC7

[PATCH] btrfs: avoid "used uninitialized" error with GCC7

[Andrei Borzenkov]

sblock was local and so considered new variable on every loop
iteration.

While on it, dynamically allocate buffer to reduce stack usage.

Closes: 50597

---
 grub-core/fs/btrfs.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c
index 9cffa91..99e81f9 100644
--- a/grub-core/fs/btrfs.c
+++ b/grub-core/fs/btrfs.c
@@ -229,24 +229,29 @@ read_sblock (grub_disk_t disk, struct grub_btrfs_superblock *sb)
 {
   unsigned i;
   grub_err_t err = GRUB_ERR_NONE;
+  struct grub_btrfs_superblock *sblock;
+
+  sblock = grub_malloc (sizeof (*sblock));
+  if (sblock == NULL)
+    return grub_errno;
+
   for (i = 0; i < ARRAY_SIZE (superblock_sectors); i++)
     {
-      struct grub_btrfs_superblock sblock;
       /* Don't try additional superblocks beyond device size.  */
-      if (i && (grub_le_to_cpu64 (sblock.this_device.size)
+      if (i && (grub_le_to_cpu64 (sblock->this_device.size)
 		>> GRUB_DISK_SECTOR_BITS) <= superblock_sectors[i])
 	break;
       err = grub_disk_read (disk, superblock_sectors[i], 0,
-			    sizeof (sblock), &sblock);
+			    sizeof (*sblock), sblock);
       if (err == GRUB_ERR_OUT_OF_RANGE)
 	break;
 
-      if (grub_memcmp ((char *) sblock.signature, GRUB_BTRFS_SIGNATURE,
+      if (grub_memcmp ((char *) sblock->signature, GRUB_BTRFS_SIGNATURE,
 		       sizeof (GRUB_BTRFS_SIGNATURE) - 1) != 0)
 	break;
-      if (i == 0 || grub_le_to_cpu64 (sblock.generation)
+      if (i == 0 || grub_le_to_cpu64 (sblock->generation)
 	  > grub_le_to_cpu64 (sb->generation))
-	grub_memcpy (sb, &sblock, sizeof (sblock));
+	grub_memcpy (sb, sblock, sizeof (*sblock));
     }
 
   if ((err == GRUB_ERR_OUT_OF_RANGE || !err) && i == 0)
@@ -255,6 +260,7 @@ read_sblock (grub_disk_t disk, struct grub_btrfs_superblock *sb)
   if (err == GRUB_ERR_OUT_OF_RANGE)
     grub_errno = err = GRUB_ERR_NONE;
 
+  grub_free (sblock);
   return err;
 }
 
-- 
tg: (8014b7b..) bug/50597 (depends on: master)

Re: [PATCH] btrfs: avoid "used uninitialized" error with GCC7

[Vladimir 'phcoder' Serbinenko]

[-- Attachment #1: Type: text/plain, Size: 2583 bytes --]

On Fri, Mar 31, 2017, 06:43 Andrei Borzenkov <arvidjaar@gmail.com> wrote:

> sblock was local and so considered new variable on every loop
> iteration.
>
> While on it, dynamically allocate buffer to reduce stack usage.
>
Looks good. Did you check all instances sizeof, so we don't have sizeof of
pointer?
Did you check return paths for free'ing on all of them?

>
>
> Closes: 50597
>
> ---
>  grub-core/fs/btrfs.c | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c
> index 9cffa91..99e81f9 100644
> --- a/grub-core/fs/btrfs.c
> +++ b/grub-core/fs/btrfs.c
> @@ -229,24 +229,29 @@ read_sblock (grub_disk_t disk, struct
> grub_btrfs_superblock *sb)
>  {
>    unsigned i;
>    grub_err_t err = GRUB_ERR_NONE;
> +  struct grub_btrfs_superblock *sblock;
> +
> +  sblock = grub_malloc (sizeof (*sblock));
> +  if (sblock == NULL)
> +    return grub_errno;
> +
>    for (i = 0; i < ARRAY_SIZE (superblock_sectors); i++)
>      {
> -      struct grub_btrfs_superblock sblock;
>        /* Don't try additional superblocks beyond device size.  */
> -      if (i && (grub_le_to_cpu64 (sblock.this_device.size)
> +      if (i && (grub_le_to_cpu64 (sblock->this_device.size)
>                 >> GRUB_DISK_SECTOR_BITS) <= superblock_sectors[i])
>         break;
>        err = grub_disk_read (disk, superblock_sectors[i], 0,
> -                           sizeof (sblock), &sblock);
> +                           sizeof (*sblock), sblock);
>        if (err == GRUB_ERR_OUT_OF_RANGE)
>         break;
>
> -      if (grub_memcmp ((char *) sblock.signature, GRUB_BTRFS_SIGNATURE,
> +      if (grub_memcmp ((char *) sblock->signature, GRUB_BTRFS_SIGNATURE,
>                        sizeof (GRUB_BTRFS_SIGNATURE) - 1) != 0)
>         break;
> -      if (i == 0 || grub_le_to_cpu64 (sblock.generation)
> +      if (i == 0 || grub_le_to_cpu64 (sblock->generation)
>           > grub_le_to_cpu64 (sb->generation))
> -       grub_memcpy (sb, &sblock, sizeof (sblock));
> +       grub_memcpy (sb, sblock, sizeof (*sblock));
>      }
>
>    if ((err == GRUB_ERR_OUT_OF_RANGE || !err) && i == 0)
> @@ -255,6 +260,7 @@ read_sblock (grub_disk_t disk, struct
> grub_btrfs_superblock *sb)
>    if (err == GRUB_ERR_OUT_OF_RANGE)
>      grub_errno = err = GRUB_ERR_NONE;
>
> +  grub_free (sblock);
>    return err;
>  }
>
> --
> tg: (8014b7b..) bug/50597 (depends on: master)
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>

[-- Attachment #2: Type: text/html, Size: 4761 bytes --]

Re: [PATCH] btrfs: avoid "used uninitialized" error with GCC7

[Andrei Borzenkov]

03.04.2017 11:59, Vladimir 'phcoder' Serbinenko пишет:
> On Fri, Mar 31, 2017, 06:43 Andrei Borzenkov <arvidjaar@gmail.com> wrote:
> 
>> sblock was local and so considered new variable on every loop
>> iteration.
>>
>> While on it, dynamically allocate buffer to reduce stack usage.
>>
> Looks good. Did you check all instances sizeof, so we don't have sizeof of
> pointer?
> Did you check return paths for free'ing on all of them?
> 

There is one common return path, but I committed light version that just
moves variable out of loop; will commit full version after release.