On Thu, Aug 20, 2015 at 11:55 AM, Andrei Borzenkov <arvidj...@gmail.com> wrote:
> 18.08.2015 00:57, dann frazier пишет:
>>
>> grub_net_fs_open() saves off a copy of the file structure it gets passed
>> and
>> uses it to create a bufio structure. It then overwrites the passed in file
>> structure with this new bufio structure. Since file->name doesn't get set
>> until we return back to grub_file_open(), it means that only the bufio
>> structure gets a valid file->name. The "real" file's name is left
>> uninitialized. This leads to a crash when the progress module hook is
>> called
>> on it. Fix this by adding a copy of the filename during the switcheroo.
>
>
> Actually name was already saved off as device->net-name. What about attached
> patch? I'll commit leak fix separately.
It does seem like it pokes a hole in the fs abstraction, but it works for me.
-dann
>
>> grub_file_close() will free that string in the success case, we only need
>> to handle the free in an error. While we're at it, also fix a leaked file
>> structure in the case that grub_strdup() fails when setting
>> file->device->net->name.
>>
>> Finally, make progress more robust by checking for NULL before reading the
>> name. Andrei noticed that this could occur if grub_strdup() fails in
>> grub_file_open().
>>
>> Signed-off-by: dann frazier <dann.fraz...@canonical.com>
>> ---
>> grub-core/lib/progress.c | 3 +--
>> grub-core/net/net.c | 14 +++++++++++++-
>> 2 files changed, 14 insertions(+), 3 deletions(-)
>>
>> diff --git a/grub-core/lib/progress.c b/grub-core/lib/progress.c
>> index 63a0767..2775554 100644
>> --- a/grub-core/lib/progress.c
>> +++ b/grub-core/lib/progress.c
>> @@ -70,8 +70,7 @@ grub_file_progress_hook_real (grub_disk_addr_t sector
>> __attribute__ ((unused)),
>> percent = grub_divmod64 (100 * file->progress_offset,
>> file->size, 0);
>>
>> - partial_file_name = grub_strrchr (file->name, '/');
>> - if (partial_file_name)
>> + if (file->name && (partial_file_name = grub_strrchr (file->name,
>> '/')))
>> partial_file_name++;
>> else
>> partial_file_name = "";
>> diff --git a/grub-core/net/net.c b/grub-core/net/net.c
>> index 21a4e94..9f83765 100644
>> --- a/grub-core/net/net.c
>> +++ b/grub-core/net/net.c
>> @@ -1391,7 +1391,17 @@ grub_net_fs_open (struct grub_file *file_out, const
>> char *name)
>> file->device->net->packs.last = NULL;
>> file->device->net->name = grub_strdup (name);
>> if (!file->device->net->name)
>> - return grub_errno;
>> + {
>> + grub_free (file);
>> + return grub_errno;
>> + }
>> + file->name = grub_strdup (name);
>> + if (!file->name)
>> + {
>> + grub_free (file->device->net->name);
>> + grub_free (file);
>> + return grub_errno;
>> + }
>>
>> err = file->device->net->protocol->open (file, name);
>> if (err)
>> @@ -1401,6 +1411,7 @@ grub_net_fs_open (struct grub_file *file_out, const
>> char *name)
>> grub_netbuff_free (file->device->net->packs.first->nb);
>> grub_net_remove_packet (file->device->net->packs.first);
>> }
>> + grub_free (file->name);
>> grub_free (file->device->net->name);
>> grub_free (file);
>> return err;
>> @@ -1414,6 +1425,7 @@ grub_net_fs_open (struct grub_file *file_out, const
>> char *name)
>> grub_net_remove_packet (file->device->net->packs.first);
>> }
>> file->device->net->protocol->close (file);
>> + grub_free (file->name);
>> grub_free (file->device->net->name);
>> grub_free (file);
>> return grub_errno;
>>
>
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
