grub-devel.gnu.org archive mirror
 help / color / mirror / Atom feed
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



  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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).