[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] Avoid NULL pointer dereference in progress module.
From: |
Dann Frazier |
Subject: |
Re: [PATCH] Avoid NULL pointer dereference in progress module. |
Date: |
Mon, 21 Sep 2015 09:11:32 -0600 |
On Fri, Aug 21, 2015 at 8:24 AM, Dann Frazier
<address@hidden> wrote:
> On Thu, Aug 20, 2015 at 11:55 AM, Andrei Borzenkov <address@hidden> 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.
hey Andrei,
Did you need anything more from me wrt this fix?
-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 <address@hidden>
>>> ---
>>> 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;
>>>
>>
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [PATCH] Avoid NULL pointer dereference in progress module.,
Dann Frazier <=