[PATCH v2] grub-file: fix segmentation fault

[PATCH v2] grub-file: fix segmentation fault

[Michael Chang]

In grub_file_open the file handle returned by file filters has no file->name
set which leads to segmentation fault later referenced by grub_elf_file. This
patch tries to fix the problem.

 gdb --args ./grub-file --is-x86_64-xen-domu /boot/vmlinux-4.1.12-1-default.gz

 (gdb) bt
 #0  0x000000000047597e in grub_strlen (s=0x0) at ../grub-core/kern/misc.c:558
 #1  0x00000000004757e2 in grub_strdup (s=0x0) at ../grub-core/kern/misc.c:463
 #2  0x0000000000406418 in grub_elf_file (file=0x6dfb50, filename=0x0) at ../grub-core/kern/elf.c:89
 #3  0x00000000004043b3 in grub_xen_file (file=0x6dfb50) at ../grub-core/loader/i386/xen_file.c:29
 #4  0x0000000000403930 in grub_cmd_file (ctxt=0x7fffffffe120, argc=1, args=0x6dfa00) at ../grub-core/commands/file.c:425
 #5  0x000000000047e178 in grub_extcmd_dispatcher (cmd=0x6df730, argc=2, args=0x6ddfb0, script=0x0) at ../grub-core/commands/extcmd.c:54
 #6  0x000000000047e1d7 in grub_extcmd_dispatch (cmd=0x6df730, argc=2, args=0x6ddfb0) at ../grub-core/commands/extcmd.c:67
 #7  0x0000000000402945 in main (argc=3, argv=0x7fffffffe2e8) at ../util/grub-file.c:102
 (gdb) frame 3
 #3  0x00000000004043b3 in grub_xen_file (file=0x6dfb50) at ../grub-core/loader/i386/xen_file.c:29
 29        elf = grub_elf_file (file, file->name);

v2: Keep the file->name in file io handle for filters as it would be fetched by
progress and used modules, according to the review by Andrei Borzenkov.

---
 grub-core/kern/file.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/grub-core/kern/file.c b/grub-core/kern/file.c
index 668f893..5506308 100644
--- a/grub-core/kern/file.c
+++ b/grub-core/kern/file.c
@@ -123,7 +123,15 @@ grub_file_open (const char *name)
       }
   if (!file)
     grub_file_close (last_file);
-    
+  else
+    {
+      if (!file->name)
+	{
+	  file->name = grub_strdup (name);
+	  grub_errno = GRUB_ERR_NONE;
+	}
+    }
+
   grub_memcpy (grub_file_filters_enabled, grub_file_filters_all,
 	       sizeof (grub_file_filters_enabled));
 
-- 
2.6.6

Re: [PATCH v2] grub-file: fix segmentation fault

[Daniel Kiper]

On Tue, Apr 12, 2016 at 02:24:12PM +0800, Michael Chang wrote:
> In grub_file_open the file handle returned by file filters has no file->name
> set which leads to segmentation fault later referenced by grub_elf_file. This
> patch tries to fix the problem.
>
>  gdb --args ./grub-file --is-x86_64-xen-domu /boot/vmlinux-4.1.12-1-default.gz
>
>  (gdb) bt
>  #0  0x000000000047597e in grub_strlen (s=0x0) at ../grub-core/kern/misc.c:558
>  #1  0x00000000004757e2 in grub_strdup (s=0x0) at ../grub-core/kern/misc.c:463
>  #2  0x0000000000406418 in grub_elf_file (file=0x6dfb50, filename=0x0) at ../grub-core/kern/elf.c:89
>  #3  0x00000000004043b3 in grub_xen_file (file=0x6dfb50) at ../grub-core/loader/i386/xen_file.c:29
>  #4  0x0000000000403930 in grub_cmd_file (ctxt=0x7fffffffe120, argc=1, args=0x6dfa00) at ../grub-core/commands/file.c:425
>  #5  0x000000000047e178 in grub_extcmd_dispatcher (cmd=0x6df730, argc=2, args=0x6ddfb0, script=0x0) at ../grub-core/commands/extcmd.c:54
>  #6  0x000000000047e1d7 in grub_extcmd_dispatch (cmd=0x6df730, argc=2, args=0x6ddfb0) at ../grub-core/commands/extcmd.c:67
>  #7  0x0000000000402945 in main (argc=3, argv=0x7fffffffe2e8) at ../util/grub-file.c:102
>  (gdb) frame 3
>  #3  0x00000000004043b3 in grub_xen_file (file=0x6dfb50) at ../grub-core/loader/i386/xen_file.c:29
>  29        elf = grub_elf_file (file, file->name);
>
> v2: Keep the file->name in file io handle for filters as it would be fetched by
> progress and used modules, according to the review by Andrei Borzenkov.

Vladimir, Andrei, could you take care of it?

Daniel

Re: [PATCH v2] grub-file: fix segmentation fault

[Andrei Borzenkov]

On Thu, Nov 17, 2016 at 10:08 PM, Daniel Kiper <dkiper@net-space.pl> wrote:
> On Tue, Apr 12, 2016 at 02:24:12PM +0800, Michael Chang wrote:
>> In grub_file_open the file handle returned by file filters has no file->name
>> set which leads to segmentation fault later referenced by grub_elf_file. This
>> patch tries to fix the problem.
>>
>>  gdb --args ./grub-file --is-x86_64-xen-domu /boot/vmlinux-4.1.12-1-default.gz
>>
>>  (gdb) bt
>>  #0  0x000000000047597e in grub_strlen (s=0x0) at ../grub-core/kern/misc.c:558
>>  #1  0x00000000004757e2 in grub_strdup (s=0x0) at ../grub-core/kern/misc.c:463
>>  #2  0x0000000000406418 in grub_elf_file (file=0x6dfb50, filename=0x0) at ../grub-core/kern/elf.c:89
>>  #3  0x00000000004043b3 in grub_xen_file (file=0x6dfb50) at ../grub-core/loader/i386/xen_file.c:29
>>  #4  0x0000000000403930 in grub_cmd_file (ctxt=0x7fffffffe120, argc=1, args=0x6dfa00) at ../grub-core/commands/file.c:425
>>  #5  0x000000000047e178 in grub_extcmd_dispatcher (cmd=0x6df730, argc=2, args=0x6ddfb0, script=0x0) at ../grub-core/commands/extcmd.c:54
>>  #6  0x000000000047e1d7 in grub_extcmd_dispatch (cmd=0x6df730, argc=2, args=0x6ddfb0) at ../grub-core/commands/extcmd.c:67
>>  #7  0x0000000000402945 in main (argc=3, argv=0x7fffffffe2e8) at ../util/grub-file.c:102
>>  (gdb) frame 3
>>  #3  0x00000000004043b3 in grub_xen_file (file=0x6dfb50) at ../grub-core/loader/i386/xen_file.c:29
>>  29        elf = grub_elf_file (file, file->name);
>>
>> v2: Keep the file->name in file io handle for filters as it would be fetched by
>> progress and used modules, according to the review by Andrei Borzenkov.
>
> Vladimir, Andrei, could you take care of it?
>

Hmm ... I must admit I am confused how we can get NULL here. Filters
are called after primary file->name is set and each filter copies
previous struct file, which means returned file will inherit pointer
to the same file name.

Now after quick glance there are filters (like bufio) that do not set
file->name to NULL on close which may lead to double free. Not sure if
it can be responsible for this. This needs fixing irrespective (as
long as we use current implementation).

Anyway, exactly because filters themselves do not free file->name this
patch means memory leak.

Michael, could you provide reproducer for it?

Re: [PATCH v2] grub-file: fix segmentation fault

[Michael Chang]

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

On Fri, Nov 18, 2016 at 11:50:25AM +0300, Andrei Borzenkov wrote:
> Hmm ... I must admit I am confused how we can get NULL here. Filters
> are called after primary file->name is set and each filter copies
> previous struct file, which means returned file will inherit pointer
> to the same file name.

No. I don't think so. Looking into gzio or xzio file filters they did not copy
original handle to new allocated one. And the new handle gets initialized
without file->name being set from original one. The new handle then returned
to upper file layer with file->name being null.

> Anyway, exactly because filters themselves do not free file->name this
> patch means memory leak.

Same reason above, as long as the filters did not allocate it, they did not
need to free.

> Michael, could you provide reproducer for it?

I can still reproduce the segfault on latest git HEAD. Here is kernel image
attached to reproduce the problem with:

 grub/build-xen # ./grub-file --is-x86_64-xen-domu /boot/vmlinux-4.8.4-1-default.gz
 Segmentation fault (core dumped)

Thanks,
Michael

[-- Attachment #2: vmlinux-4.8.4-1-default.gz --]
[-- Type: application/x-gzip, Size: 9616296 bytes --]

Re: [PATCH v2] grub-file: fix segmentation fault

[Michael Chang]

I have to resend the mail as the attached kernel image is too big that may not
reach the list eventually. Sorry if you get duplicated mail. 

If you still have problem with reproducing the problem please let me know. I
could provide download link for the kernel file.

On Fri, Nov 18, 2016 at 11:50:25AM +0300, Andrei Borzenkov wrote:
> Hmm ... I must admit I am confused how we can get NULL here. Filters
> are called after primary file->name is set and each filter copies
> previous struct file, which means returned file will inherit pointer
> to the same file name.

No. I don't think so. Looking into gzio or xzio file filters they did not copy
original handle to new allocated one. And the new handle gets initialized
without file->name being set from original one. The new handle then returned
to upper file layer with file->name being null.

> Anyway, exactly because filters themselves do not free file->name this
> patch means memory leak.

Same reason above, as long as the filters did not allocate it, they did not
need to free.

> Michael, could you provide reproducer for it?

I can still reproduce the segfault on latest git HEAD. Here is kernel image
attached to reproduce the problem with.

 grub/build-xen # ./grub-file --is-x86_64-xen-domu /boot/vmlinux-4.8.4-1-default.gz
 Segmentation fault (core dumped)

Thanks,
Michael

Re: [PATCH v2] grub-file: fix segmentation fault

[Andrei Borzenkov]

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

22.11.2016 10:10, Michael Chang пишет:
> On Fri, Nov 18, 2016 at 11:50:25AM +0300, Andrei Borzenkov wrote:
>> Hmm ... I must admit I am confused how we can get NULL here. Filters
>> are called after primary file->name is set and each filter copies
>> previous struct file, which means returned file will inherit pointer
>> to the same file name.
> 
> No. I don't think so. Looking into gzio or xzio file filters they did not copy
> original handle to new allocated one. And the new handle gets initialized
> without file->name being set from original one. The new handle then returned
> to upper file layer with file->name being null.
> 

Right, I was mistaken.

>> Anyway, exactly because filters themselves do not free file->name this
>> patch means memory leak.
> 
> Same reason above, as long as the filters did not allocate it, they did not
> need to free.
> 

Yes, but your patch allocates copy of name and (some) filters set it to
NULL so it is leaked.

What about attached patch? It does not increase size of kernel.

I am not thrilled as it does require more discipline from filter author.
OTOH we may find some way to consolidate boilerplate later.

@Dann, this also solves your concerns about layering violation in
progress module as side effect. Could you test this patch? Thank you!

>> Michael, could you provide reproducer for it?
> 
> I can still reproduce the segfault on latest git HEAD. Here is kernel image
> attached to reproduce the problem with:
> 
>  grub/build-xen # ./grub-file --is-x86_64-xen-domu /boot/vmlinux-4.8.4-1-default.gz
>  Segmentation fault (core dumped)
> 
> Thanks,
> Michael
> 


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: file-filter-filename.patch --]
[-- Type: text/x-patch; name="file-filter-filename.patch", Size: 7502 bytes --]

From: Andrei Borzenkov <arvidjaar@gmail.com>
Subject: [PATCH] file: make sure file name is set when filters are used

In grub_file_open the file handle returned by file filters has no file->name
set which leads to segmentation fault later referenced by grub_elf_file. This
patch tries to fix the problem.

 gdb --args ./grub-file --is-x86_64-xen-domu /boot/vmlinux-4.1.12-1-default.gz

 (gdb) bt
 #0  0x000000000047597e in grub_strlen (s=0x0) at ../grub-core/kern/misc.c:558
 #1  0x00000000004757e2 in grub_strdup (s=0x0) at ../grub-core/kern/misc.c:463
 #2  0x0000000000406418 in grub_elf_file (file=0x6dfb50, filename=0x0) at ../grub-core/kern/elf.c:89
 #3  0x00000000004043b3 in grub_xen_file (file=0x6dfb50) at ../grub-core/loader/i386/xen_file.c:29
 #4  0x0000000000403930 in grub_cmd_file (ctxt=0x7fffffffe120, argc=1, args=0x6dfa00) at ../grub-core/commands/file.c:425
 #5  0x000000000047e178 in grub_extcmd_dispatcher (cmd=0x6df730, argc=2, args=0x6ddfb0, script=0x0) at ../grub-core/commands/extcmd.c:54
 #6  0x000000000047e1d7 in grub_extcmd_dispatch (cmd=0x6df730, argc=2, args=0x6ddfb0) at ../grub-core/commands/extcmd.c:67
 #7  0x0000000000402945 in main (argc=3, argv=0x7fffffffe2e8) at ../util/grub-file.c:102
 (gdb) frame 3
 #3  0x00000000004043b3 in grub_xen_file (file=0x6dfb50) at ../grub-core/loader/i386/xen_file.c:29
 29        elf = grub_elf_file (file, file->name);

Initialize file->name early and make sure it is set in file structure returned
by each filter. Note that each filter must set it to NULL in its ->close method
to avoid double free.

It also makes redundant special case for net in progress module, because now file
name is set also for net files.

Reported by: Michael Chang <mchang@suse.com>

---
 grub-core/commands/verify.c | 1 +
 grub-core/io/bufio.c        | 3 +++
 grub-core/io/gzio.c         | 2 ++
 grub-core/io/lzopio.c       | 2 ++
 grub-core/io/offset.c       | 3 +++
 grub-core/io/xzio.c         | 2 ++
 grub-core/kern/file.c       | 5 ++---
 grub-core/lib/progress.c    | 7 +------
 8 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/grub-core/commands/verify.c b/grub-core/commands/verify.c
index 67cb1c7..a959929 100644
--- a/grub-core/commands/verify.c
+++ b/grub-core/commands/verify.c
@@ -845,6 +845,7 @@ verified_close (struct grub_file *file)
 
   /* device and name are freed by parent */
   file->device = 0;
+  /* Must set to NULL to prevent double free in grub_file_close */
   file->name = 0;
 
   return grub_errno;
diff --git a/grub-core/io/bufio.c b/grub-core/io/bufio.c
index 2243827..3a8a53c 100644
--- a/grub-core/io/bufio.c
+++ b/grub-core/io/bufio.c
@@ -72,6 +72,7 @@ grub_bufio_open (grub_file_t io, int size)
   bufio->block_size = size;
 
   file->device = io->device;
+  file->name = io->name;
   file->size = io->size;
   file->data = bufio;
   file->fs = &grub_bufio_fs;
@@ -191,6 +192,8 @@ grub_bufio_close (grub_file_t file)
   grub_free (bufio);
 
   file->device = 0;
+  /* Must set to NULL to prevent double free in grub_file_close */
+  file->name = 0;
 
   return grub_errno;
 }
diff --git a/grub-core/io/gzio.c b/grub-core/io/gzio.c
index 0f2ea6b..17dcbe3 100644
--- a/grub-core/io/gzio.c
+++ b/grub-core/io/gzio.c
@@ -1144,6 +1144,7 @@ grub_gzio_open (grub_file_t io, const char *name __attribute__ ((unused)))
   gzio->file = io;
 
   file->device = io->device;
+  file->name = io->name;
   file->data = gzio;
   file->fs = &grub_gzio_fs;
   file->not_easily_seekable = 1;
@@ -1291,6 +1292,7 @@ grub_gzio_close (grub_file_t file)
 
   /* No need to close the same device twice.  */
   file->device = 0;
+  /* Must set to NULL to prevent double free in grub_file_close */
   file->name = 0;
 
   return grub_errno;
diff --git a/grub-core/io/lzopio.c b/grub-core/io/lzopio.c
index 7559c6c..52acde4 100644
--- a/grub-core/io/lzopio.c
+++ b/grub-core/io/lzopio.c
@@ -427,6 +427,7 @@ grub_lzopio_open (grub_file_t io,
   lzopio->file = io;
 
   file->device = io->device;
+  file->name = io->name;
   file->data = lzopio;
   file->fs = &grub_lzopio_fs;
   file->size = GRUB_FILE_SIZE_UNKNOWN;
@@ -523,6 +524,7 @@ grub_lzopio_close (grub_file_t file)
 
   /* Device must not be closed twice.  */
   file->device = 0;
+  /* Must set to NULL to prevent double free in grub_file_close */
   file->name = 0;
   return grub_errno;
 }
diff --git a/grub-core/io/offset.c b/grub-core/io/offset.c
index ebed0eb..1de192e 100644
--- a/grub-core/io/offset.c
+++ b/grub-core/io/offset.c
@@ -46,6 +46,8 @@ grub_offset_close (grub_file_t file)
 
   /* No need to close the same device twice.  */
   file->device = 0;
+  /* Must set to NULL to prevent double free in grub_file_close */
+  file->name = 0;
 
   return 0;
 }
@@ -88,6 +90,7 @@ grub_file_offset_open (grub_file_t parent, grub_off_t start, grub_off_t size)
   off_data->parent = parent;
 
   off_file->device = parent->device;
+  off_file->name = parent->name;
   off_file->data = off_data;
   off_file->fs = &grub_offset_fs;
   off_file->size = size;
diff --git a/grub-core/io/xzio.c b/grub-core/io/xzio.c
index a3536ad..cdf6cef 100644
--- a/grub-core/io/xzio.c
+++ b/grub-core/io/xzio.c
@@ -189,6 +189,7 @@ grub_xzio_open (grub_file_t io,
   xzio->file = io;
 
   file->device = io->device;
+  file->name = io->name;
   file->data = xzio;
   file->fs = &grub_xzio_fs;
   file->size = GRUB_FILE_SIZE_UNKNOWN;
@@ -319,6 +320,7 @@ grub_xzio_close (grub_file_t file)
 
   /* Device must not be closed twice.  */
   file->device = 0;
+  /* Must set to NULL to prevent double free in grub_file_close */
   file->name = 0;
   return grub_errno;
 }
diff --git a/grub-core/kern/file.c b/grub-core/kern/file.c
index 668f893..dd6c28a 100644
--- a/grub-core/kern/file.c
+++ b/grub-core/kern/file.c
@@ -88,6 +88,8 @@ grub_file_open (const char *name)
     goto fail;
 
   file->device = device;
+  file->name = grub_strdup (name);
+  grub_errno = GRUB_ERR_NONE;
 
   /* In case of relative pathnames and non-Unix systems (like Windows)
    * name of host files may not start with `/'. Blocklists for host files
@@ -111,9 +113,6 @@ grub_file_open (const char *name)
   if ((file->fs->open) (file, file_name) != GRUB_ERR_NONE)
     goto fail;
 
-  file->name = grub_strdup (name);
-  grub_errno = GRUB_ERR_NONE;
-
   for (filter = 0; file && filter < ARRAY_SIZE (grub_file_filters_enabled);
        filter++)
     if (grub_file_filters_enabled[filter])
diff --git a/grub-core/lib/progress.c b/grub-core/lib/progress.c
index 4b7cbbc..c8efdb1 100644
--- a/grub-core/lib/progress.c
+++ b/grub-core/lib/progress.c
@@ -77,12 +77,7 @@ grub_file_progress_hook_real (grub_disk_addr_t sector __attribute__ ((unused)),
 	percent = grub_divmod64 (100 * file->progress_offset,
 				 file->size, 0);
 
-      /* grub_net_fs_open() saves off partial file structure before name is initialized.
-         It already saves passed file name in net structure so just use it in this case.
-       */
-      if (file->device->net)
-	partial_file_name = grub_strrchr (file->device->net->name, '/');
-      else if (file->name) /* grub_file_open() may leave it as NULL */
+      if (file->name) /* grub_file_open() may leave it as NULL */
 	partial_file_name = grub_strrchr (file->name, '/');
       else
 	partial_file_name = NULL;
-- 
tg: (c9a8d03..) u/file-filter-filename (depends on: master)

Re: [PATCH v2] grub-file: fix segmentation fault

[Michael Chang]

On Tue, Nov 22, 2016 at 09:39:43PM +0300, Andrei Borzenkov wrote:
> 22.11.2016 10:10, Michael Chang пишет:
> 
> What about attached patch? It does not increase size of kernel.

Just some nitpicks in comments and a potential leak in fail path (See below),
otherwise LGTM.

> 
> I am not thrilled as it does require more discipline from filter author.
> OTOH we may find some way to consolidate boilerplate later.

Yes, that's also why I tend to not touch the filter but trying to paper over it
on the parent's side, to be less frustrated to the filter authors.

> 
> @Dann, this also solves your concerns about layering violation in
> progress module as side effect. Could you test this patch? Thank you!

I think this may deserve a separate patch (in file.c and progress.c) as it did
not fixed by the segfault problem here. But that's all up to you, as
consolidating all file->name related fix in one patch is nothing wrong.

> From: Andrei Borzenkov <arvidjaar@gmail.com>
> Subject: [PATCH] file: make sure file name is set when filters are used
> 
> In grub_file_open the file handle returned by file filters has no file->name
> set which leads to segmentation fault later referenced by grub_elf_file. This
> patch tries to fix the problem.
> 
>  gdb --args ./grub-file --is-x86_64-xen-domu /boot/vmlinux-4.1.12-1-default.gz
> 
>  (gdb) bt
>  #0  0x000000000047597e in grub_strlen (s=0x0) at ../grub-core/kern/misc.c:558
>  #1  0x00000000004757e2 in grub_strdup (s=0x0) at ../grub-core/kern/misc.c:463
>  #2  0x0000000000406418 in grub_elf_file (file=0x6dfb50, filename=0x0) at ../grub-core/kern/elf.c:89
>  #3  0x00000000004043b3 in grub_xen_file (file=0x6dfb50) at ../grub-core/loader/i386/xen_file.c:29
>  #4  0x0000000000403930 in grub_cmd_file (ctxt=0x7fffffffe120, argc=1, args=0x6dfa00) at ../grub-core/commands/file.c:425
>  #5  0x000000000047e178 in grub_extcmd_dispatcher (cmd=0x6df730, argc=2, args=0x6ddfb0, script=0x0) at ../grub-core/commands/extcmd.c:54
>  #6  0x000000000047e1d7 in grub_extcmd_dispatch (cmd=0x6df730, argc=2, args=0x6ddfb0) at ../grub-core/commands/extcmd.c:67
>  #7  0x0000000000402945 in main (argc=3, argv=0x7fffffffe2e8) at ../util/grub-file.c:102
>  (gdb) frame 3
>  #3  0x00000000004043b3 in grub_xen_file (file=0x6dfb50) at ../grub-core/loader/i386/xen_file.c:29
>  29        elf = grub_elf_file (file, file->name);
> 
> Initialize file->name early and make sure it is set in file structure returned
> by each filter. Note that each filter must set it to NULL in its ->close method
> to avoid double free.
> 
> It also makes redundant special case for net in progress module, because now file
> name is set also for net files.
> 
> Reported by: Michael Chang <mchang@suse.com>
> 
> ---
>  grub-core/commands/verify.c | 1 +
>  grub-core/io/bufio.c        | 3 +++
>  grub-core/io/gzio.c         | 2 ++
>  grub-core/io/lzopio.c       | 2 ++
>  grub-core/io/offset.c       | 3 +++
>  grub-core/io/xzio.c         | 2 ++
>  grub-core/kern/file.c       | 5 ++---
>  grub-core/lib/progress.c    | 7 +------
>  8 files changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/grub-core/commands/verify.c b/grub-core/commands/verify.c
> index 67cb1c7..a959929 100644
> --- a/grub-core/commands/verify.c
> +++ b/grub-core/commands/verify.c
> @@ -845,6 +845,7 @@ verified_close (struct grub_file *file)
>  
>    /* device and name are freed by parent */
>    file->device = 0;
> +  /* Must set to NULL to prevent double free in grub_file_close */

As long as the comment is intended to make things more obvious, I'd suggest to
address again it's for "parent's" grub_file_close and be more in line with
previous comment has claimed.

Same applies for all similar comments below.

>    file->name = 0;
>  
>    return grub_errno;
> diff --git a/grub-core/io/bufio.c b/grub-core/io/bufio.c
> index 2243827..3a8a53c 100644
> --- a/grub-core/io/bufio.c
> +++ b/grub-core/io/bufio.c
> @@ -72,6 +72,7 @@ grub_bufio_open (grub_file_t io, int size)
>    bufio->block_size = size;
>  
>    file->device = io->device;
> +  file->name = io->name;
>    file->size = io->size;
>    file->data = bufio;
>    file->fs = &grub_bufio_fs;
> @@ -191,6 +192,8 @@ grub_bufio_close (grub_file_t file)
>    grub_free (bufio);
>  
>    file->device = 0;
> +  /* Must set to NULL to prevent double free in grub_file_close */
> +  file->name = 0;
>  
>    return grub_errno;
>  }
> diff --git a/grub-core/io/gzio.c b/grub-core/io/gzio.c
> index 0f2ea6b..17dcbe3 100644
> --- a/grub-core/io/gzio.c
> +++ b/grub-core/io/gzio.c
> @@ -1144,6 +1144,7 @@ grub_gzio_open (grub_file_t io, const char *name __attribute__ ((unused)))
>    gzio->file = io;
>  
>    file->device = io->device;
> +  file->name = io->name;
>    file->data = gzio;
>    file->fs = &grub_gzio_fs;
>    file->not_easily_seekable = 1;
> @@ -1291,6 +1292,7 @@ grub_gzio_close (grub_file_t file)
>  
>    /* No need to close the same device twice.  */
>    file->device = 0;
> +  /* Must set to NULL to prevent double free in grub_file_close */
>    file->name = 0;
>  
>    return grub_errno;
> diff --git a/grub-core/io/lzopio.c b/grub-core/io/lzopio.c
> index 7559c6c..52acde4 100644
> --- a/grub-core/io/lzopio.c
> +++ b/grub-core/io/lzopio.c
> @@ -427,6 +427,7 @@ grub_lzopio_open (grub_file_t io,
>    lzopio->file = io;
>  
>    file->device = io->device;
> +  file->name = io->name;
>    file->data = lzopio;
>    file->fs = &grub_lzopio_fs;
>    file->size = GRUB_FILE_SIZE_UNKNOWN;
> @@ -523,6 +524,7 @@ grub_lzopio_close (grub_file_t file)
>  
>    /* Device must not be closed twice.  */
>    file->device = 0;
> +  /* Must set to NULL to prevent double free in grub_file_close */
>    file->name = 0;
>    return grub_errno;
>  }
> diff --git a/grub-core/io/offset.c b/grub-core/io/offset.c
> index ebed0eb..1de192e 100644
> --- a/grub-core/io/offset.c
> +++ b/grub-core/io/offset.c
> @@ -46,6 +46,8 @@ grub_offset_close (grub_file_t file)
>  
>    /* No need to close the same device twice.  */
>    file->device = 0;
> +  /* Must set to NULL to prevent double free in grub_file_close */
> +  file->name = 0;
>  
>    return 0;
>  }
> @@ -88,6 +90,7 @@ grub_file_offset_open (grub_file_t parent, grub_off_t start, grub_off_t size)
>    off_data->parent = parent;
>  
>    off_file->device = parent->device;
> +  off_file->name = parent->name;
>    off_file->data = off_data;
>    off_file->fs = &grub_offset_fs;
>    off_file->size = size;
> diff --git a/grub-core/io/xzio.c b/grub-core/io/xzio.c
> index a3536ad..cdf6cef 100644
> --- a/grub-core/io/xzio.c
> +++ b/grub-core/io/xzio.c
> @@ -189,6 +189,7 @@ grub_xzio_open (grub_file_t io,
>    xzio->file = io;
>  
>    file->device = io->device;
> +  file->name = io->name;
>    file->data = xzio;
>    file->fs = &grub_xzio_fs;
>    file->size = GRUB_FILE_SIZE_UNKNOWN;
> @@ -319,6 +320,7 @@ grub_xzio_close (grub_file_t file)
>  
>    /* Device must not be closed twice.  */
>    file->device = 0;
> +  /* Must set to NULL to prevent double free in grub_file_close */
>    file->name = 0;
>    return grub_errno;
>  }
> diff --git a/grub-core/kern/file.c b/grub-core/kern/file.c
> index 668f893..dd6c28a 100644
> --- a/grub-core/kern/file.c
> +++ b/grub-core/kern/file.c
> @@ -88,6 +88,8 @@ grub_file_open (const char *name)
>      goto fail;
>  
>    file->device = device;
> +  file->name = grub_strdup (name);
> +  grub_errno = GRUB_ERR_NONE;
>  
>    /* In case of relative pathnames and non-Unix systems (like Windows)
>     * name of host files may not start with `/'. Blocklists for host files
> @@ -111,9 +113,6 @@ grub_file_open (const char *name)
>    if ((file->fs->open) (file, file_name) != GRUB_ERR_NONE)
>      goto fail;
>  
> -  file->name = grub_strdup (name);
> -  grub_errno = GRUB_ERR_NONE;
> -

Well.. I think file->name now has to be free also in the "goto fail" path.

Thanks,
Michael

>    for (filter = 0; file && filter < ARRAY_SIZE (grub_file_filters_enabled);
>         filter++)
>      if (grub_file_filters_enabled[filter])
> diff --git a/grub-core/lib/progress.c b/grub-core/lib/progress.c
> index 4b7cbbc..c8efdb1 100644
> --- a/grub-core/lib/progress.c
> +++ b/grub-core/lib/progress.c
> @@ -77,12 +77,7 @@ grub_file_progress_hook_real (grub_disk_addr_t sector __attribute__ ((unused)),
>  	percent = grub_divmod64 (100 * file->progress_offset,
>  				 file->size, 0);
>  
> -      /* grub_net_fs_open() saves off partial file structure before name is initialized.
> -         It already saves passed file name in net structure so just use it in this case.
> -       */
> -      if (file->device->net)
> -	partial_file_name = grub_strrchr (file->device->net->name, '/');
> -      else if (file->name) /* grub_file_open() may leave it as NULL */
> +      if (file->name) /* grub_file_open() may leave it as NULL */
>  	partial_file_name = grub_strrchr (file->name, '/');
>        else
>  	partial_file_name = NULL;



> -- 
> tg: (c9a8d03..) u/file-filter-filename (depends on: master)

> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel