From: dann frazier <dann.frazier@canonical.com>
To: Andrei Borzenkov <arvidjaar@gmail.com>
Cc: The development of GNU GRUB <grub-devel@gnu.org>
Subject: [PATCH] Avoid NULL pointer dereference in progress module.
Date: Mon, 17 Aug 2015 15:57:33 -0600 [thread overview]
Message-ID: <20150817215733.GA10858@fluid.dannf> (raw)
In-Reply-To: <55D22CAA.3040200@gmail.com>
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.
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.frazier@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;
--
2.5.0
next prev parent reply other threads:[~2015-08-17 21:57 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-08-12 15:53 [PATCH] progress: Check for NULL filename dann frazier
2015-08-13 7:52 ` Andrei Borzenkov
2015-08-13 21:04 ` dann frazier
2015-08-17 18:49 ` Andrei Borzenkov
2015-08-17 21:57 ` dann frazier [this message]
2015-08-20 17:55 ` [PATCH] Avoid NULL pointer dereference in progress module Andrei Borzenkov
2015-08-21 14:24 ` Dann Frazier
2015-09-21 15:11 ` Dann Frazier
2015-10-10 8:44 ` Andrei Borzenkov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150817215733.GA10858@fluid.dannf \
--to=dann.frazier@canonical.com \
--cc=arvidjaar@gmail.com \
--cc=grub-devel@gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.