Re: 2.02-beta3 remove attempt to free stack space and initialize variable before possible use

[Andrei Borzenkov]

14.03.2016 17:37, Aaron Luft пишет:
> Please consider these improvements to 2.02-beta3.
> 1) Remove the variable "oldname" which is attempting to free stack space.
> 2) Initialize the value of mdnobj to silence the compiler warning
> 
> In function 'grub_free',
>     inlined from 'grub_iso9660_iterate_dir' at grub-core/fs/iso9660.c:764:15:
> grub-core/kern/emu/mm.c:53:3: error: attempt to free a non-heap object 'name' 
> [-Werror=free-nonheap-object]
>    free (ptr);
>    ^
> lto1: all warnings being treated as errors
> lto-wrapper: fatal error: x86_64-linux-gnu-gcc-5.3.0 returned 1 exit status
> 
> grub-core/fs/zfs/zfsinfo.c: In function 'grub_cmd_zfs_bootfs':
> grub-core/fs/zfs/zfsinfo.c:401:10: error: 'mdnobj' may be used uninitialized 
> in this function [-Werror=maybe-uninitialized]
>    bootfs = grub_xasprintf ("zfs-bootfs=%s/%llu%s%s%s%s%s%s",
>           ^
> grub-core/fs/zfs/zfsinfo.c:355:17: note: 'mdnobj' was declared here
>    grub_uint64_t mdnobj;
>                  ^
> lto1: all warnings being treated as errors
> 

I cannot apply them due to whitespace changes.

> 
> 
> diff -Naur grub-2.02-beta3.orig/grub-core/fs/iso9660.c 
> grub-2.02-beta3/grub-core/fs/iso9660.c
> --- grub-2.02-beta3.orig/grub-core/fs/iso9660.c 2016-02-28 02:07:41.000000000 
> +0000
> +++ grub-2.02-beta3/grub-core/fs/iso9660.c      2016-03-12 01:17:26.581112809 
> +0000
> @@ -750,19 +750,15 @@
> 
>          if (dir->data->joliet && !ctx.filename)
>            {
> -            char *oldname, *semicolon;
> +            char *semicolon;
> 
> -            oldname = name;
>              ctx.filename = grub_iso9660_convert_string
> -                  ((grub_uint8_t *) oldname, dirent.namelen >> 1);
> +                  ((grub_uint8_t *) name, dirent.namelen >> 1);
> 
>             semicolon = grub_strrchr (ctx.filename, ';');
>             if (semicolon)
>               *semicolon = '\0';
> 
> -            if (ctx.filename_alloc)
> -              grub_free (oldname);
> -
>              ctx.filename_alloc = 1;
>            }
> 
Yes, this is one correct. Please resend as attached patch generated by
"git format-patch" with suitable commit message. Do not expand tabs when
editing.


> diff -Naur grub-2.02-beta3.orig/grub-core/fs/zfs/zfsinfo.c 
> grub-2.02-beta3/grub-core/fs/zfs/zfsinfo.c
> --- grub-2.02-beta3.orig/grub-core/fs/zfs/zfsinfo.c     2016-02-28 
> 02:07:41.000000000 +0000
> +++ grub-2.02-beta3/grub-core/fs/zfs/zfsinfo.c  2016-03-12 01:18:00.504961950 
> +0000
> @@ -352,7 +352,7 @@
>    char *fsname;
>    char *bootfs;
>    char *poolname;
> -  grub_uint64_t mdnobj;
> +  grub_uint64_t mdnobj = 0;
> 
>    if (argc < 1)
>      return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("one argument expected"));
> 

Well ... it cannot really reach code where mdnobj is used if
grub_zfs_getmdnobj() fails but static analyzer may not know it.

How do you compile it? I cannot reproduce it using gcc 5.3.1 nor did it
fail previously. Do you use non-standard compiler flags?