From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1ZRSPb-0000qt-8L for mharc-grub-devel@gnu.org; Mon, 17 Aug 2015 17:57:47 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41771) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZRSPU-0000k9-Q2 for grub-devel@gnu.org; Mon, 17 Aug 2015 17:57:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZRSPQ-0000yV-Nq for grub-devel@gnu.org; Mon, 17 Aug 2015 17:57:40 -0400 Received: from complete.lackof.org ([198.49.126.79]:53234) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZRSPQ-0000y2-IP for grub-devel@gnu.org; Mon, 17 Aug 2015 17:57:36 -0400 Received: from localhost (c-107-2-141-92.hsd1.co.comcast.net [107.2.141.92]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) by complete.lackof.org (Postfix) with ESMTPSA id 658A533E006C; Mon, 17 Aug 2015 15:57:33 -0600 (MDT) Date: Mon, 17 Aug 2015 15:57:33 -0600 From: dann frazier To: Andrei Borzenkov Subject: [PATCH] Avoid NULL pointer dereference in progress module. Message-ID: <20150817215733.GA10858@fluid.dannf> References: <1439394805-8573-1-git-send-email-dann.frazier@canonical.com> <20150813210409.GA26195@fluid.dannf> <55D22CAA.3040200@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <55D22CAA.3040200@gmail.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 198.49.126.79 Cc: The development of GNU GRUB X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list Reply-To: The development of GNU GRUB List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 17 Aug 2015 21:57:44 -0000 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 --- 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