From: Eric Sesterhenn <snakebyte@gmx.de>
To: "Jörn Engel" <joern@logfs.org>
Cc: Tom Rini <trini@kernel.crashing.org>,
phillip@lougher.demon.co.uk, linux-fsdevel@vger.kernel.org,
jacmet@sunsite.dk, rpurdie@rpsys.net, linuxppc-dev@ozlabs.org,
chris@zankel.net
Subject: Re: [Patch] NULL pointer deref with corrupted squashfs image
Date: Tue, 20 Jan 2009 17:47:14 +0100 [thread overview]
Message-ID: <20090120163957.GA21339@alice> (raw)
In-Reply-To: <20090117134916.GA29641@logfs.org>
* Jörn Engel (joern@logfs.org) wrote:
> On Fri, 16 January 2009 16:07:00 -0700, Tom Rini wrote:
> >
> > Sounds like a plan to me, except maybe zlib_inflate_unsafe() and a
> > comment above the wrapper saying what/why is going on?
>
> Eric, will you do the honors? Since you did all the hard work before,
> you derserve the fame as well. :)
Since I am not sure either about xtensa I added chris to the cc list.
Some callees of zlib_inflate() might accidentally pass a NULL
pointer in strm->next_out which zlib_inflate() should catch.
Others like the powerpc gunzip_partial expect to be able
to extract a zImage to memory location 0. This introduces
zlib_inflate_usafe() for those and adds a check for the rest
Signed-off-by: Eric Sesterhenn <snakebyte@gmx.de>
--- linux/arch/powerpc/boot/gunzip_util.c.orig 2009-01-20 10:23:11.000000000 +0100
+++ linux/arch/powerpc/boot/gunzip_util.c 2009-01-20 10:23:31.000000000 +0100
@@ -109,7 +109,7 @@ int gunzip_partial(struct gunzip_state *
state->s.next_out = dst;
state->s.avail_out = dstlen;
- r = zlib_inflate(&state->s, Z_FULL_FLUSH);
+ r = zlib_inflate_unsafe(&state->s, Z_FULL_FLUSH);
if (r != Z_OK && r != Z_STREAM_END)
fatal("inflate returned %d msg: %s\n\r", r, state->s.msg);
len = state->s.next_out - (unsigned char *)dst;
--- linux/include/linux/zlib.h.orig 2009-01-20 10:11:33.000000000 +0100
+++ linux/include/linux/zlib.h 2009-01-20 10:19:56.000000000 +0100
@@ -329,7 +329,23 @@ extern int zlib_inflateInit (z_streamp s
*/
-extern int zlib_inflate (z_streamp strm, int flush);
+extern int __zlib_inflate (z_streamp strm, int flush, int check_out);
+/*
+ These two wrappers decide wheter strm->next_out gets checked for NULL.
+ The zlib_inflate_unsafe() version got added because the PPC zImage
+ gets extracted to memory address 0 and therefore
+ we avoid this check for zlib_inflate_unsafe()
+*/
+static inline int zlib_inflate(z_streamp strm, int flush)
+{
+ return __zlib_inflate(strm, flush, 1);
+}
+
+static inline int zlib_inflate_unsafe(z_streamp strm, int flush)
+{
+ return __zlib_inflate(strm, flush, 0);
+}
+
/*
inflate decompresses as much data as possible, and stops when the input
buffer becomes empty or the output buffer becomes full. It may introduce
--- linux/lib/zlib_inflate/inflate_syms.c.orig 2009-01-20 10:22:21.000000000 +0100
+++ linux/lib/zlib_inflate/inflate_syms.c 2009-01-20 10:22:32.000000000 +0100
@@ -11,7 +11,7 @@
#include <linux/zlib.h>
EXPORT_SYMBOL(zlib_inflate_workspacesize);
-EXPORT_SYMBOL(zlib_inflate);
+EXPORT_SYMBOL(__zlib_inflate);
EXPORT_SYMBOL(zlib_inflateInit2);
EXPORT_SYMBOL(zlib_inflateEnd);
EXPORT_SYMBOL(zlib_inflateReset);
--- linux/lib/zlib_inflate/inflate.c.orig 2009-01-20 10:20:14.000000000 +0100
+++ linux/lib/zlib_inflate/inflate.c 2009-01-20 10:22:03.000000000 +0100
@@ -329,7 +329,7 @@ static int zlib_inflateSyncPacket(z_stre
will return Z_BUF_ERROR if it has not reached the end of the stream.
*/
-int zlib_inflate(z_streamp strm, int flush)
+int __zlib_inflate(z_streamp strm, int flush, int check_out)
{
struct inflate_state *state;
const unsigned char *next; /* next input */
@@ -347,8 +347,10 @@ int zlib_inflate(z_streamp strm, int flu
static const unsigned short order[19] = /* permutation of code lengths */
{16, 17, 18, 0, 8, 7, 9, 6, 10, 5, 11, 4, 12, 3, 13, 2, 14, 1, 15};
- /* Do not check for strm->next_out == NULL here as ppc zImage
- inflates to strm->next_out = 0 */
+ /* We only check strm->next_out if the callee requests it,
+ since ppc extracts the ppc zImage to 0 */
+ if (check_out && !strm->next_out)
+ return Z_STREAM_ERROR;
if (strm == NULL || strm->state == NULL ||
(strm->next_in == NULL && strm->avail_in != 0))
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2009-01-20 16:47 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-01-13 12:40 Bug with corrupted squashfs image Eric Sesterhenn
2009-01-16 17:45 ` [Patch] NULL pointer deref " Eric Sesterhenn
2009-01-16 19:07 ` Jörn Engel
2009-01-16 23:07 ` Tom Rini
2009-01-17 13:49 ` Jörn Engel
2009-01-17 19:38 ` Eric Sesterhenn
2009-01-20 16:47 ` Eric Sesterhenn [this message]
2009-01-20 17:57 ` Jörn Engel
2009-01-20 18:47 ` Tom Rini
2009-01-21 8:34 ` Eric Sesterhenn
2009-01-21 12:31 ` Phillip Lougher
2009-01-22 2:48 ` Phillip Lougher
2009-01-22 9:46 ` Jörn Engel
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=20090120163957.GA21339@alice \
--to=snakebyte@gmx.de \
--cc=chris@zankel.net \
--cc=jacmet@sunsite.dk \
--cc=joern@logfs.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linuxppc-dev@ozlabs.org \
--cc=phillip@lougher.demon.co.uk \
--cc=rpurdie@rpsys.net \
--cc=trini@kernel.crashing.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).