From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tom Rini Subject: Re: [Patch] NULL pointer deref with corrupted squashfs image Date: Fri, 16 Jan 2009 16:07:00 -0700 Message-ID: <20090116230700.GP6710@smtp.west.cox.net> References: <20090113124027.GB16333@alice> <20090116174525.GA31869@alice> <20090116190725.GA19453@logfs.org> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: Eric Sesterhenn , phillip@lougher.demon.co.uk, linux-fsdevel@vger.kernel.org, jacmet@sunsite.dk, rpurdie@rpsys.net To: =?iso-8859-1?Q?J=F6rn?= Engel Return-path: Received: from fed1rmmtao102.cox.net ([68.230.241.44]:47636 "EHLO fed1rmmtao102.cox.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1759309AbZAPXHE (ORCPT ); Fri, 16 Jan 2009 18:07:04 -0500 Content-Disposition: inline In-Reply-To: <20090116190725.GA19453@logfs.org> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Fri, Jan 16, 2009 at 08:07:32PM +0100, J=F6rn Engel wrote: > On Fri, 16 January 2009 18:45:25 +0100, Eric Sesterhenn wrote: > >=20 > > Non-PPC targets shouldnt inflate images to memory address 0. > > check strm->next_out for NULL in case on non PPC architecture > > to prevent a NULL-pointer dereference while inflating corrupted ima= ges. > >=20 > > Signed-off-by: Eric Sesterhenn > >=20 > > --- linux/lib/zlib_inflate/inflate.c.orig 2009-01-16 15:40:04.00000= 0000 +0100 > > +++ linux/lib/zlib_inflate/inflate.c 2009-01-16 15:41:42.000000000 = +0100 > > @@ -347,8 +347,12 @@ int zlib_inflate(z_streamp strm, int flu > > static const unsigned short order[19] =3D /* permutation of co= de lengths */ > > {16, 17, 18, 0, 8, 7, 9, 6, 10, 5, 11, 4, 12, 3, 13, 2, 14= , 1, 15}; > > =20 > > - /* Do not check for strm->next_out =3D=3D NULL here as ppc zIm= age > > - inflates to strm->next_out =3D 0 */ > > + /* Since ppc zImage inflates to 0 we only check > > + strm->next_out for non-ppc targets0 */ > > +#ifndef CONFIG_PPC > > + if (!strm->next_out) > > + return Z_STREAM_ERROR; > > +#endif > > =20 > > if (strm =3D=3D NULL || strm->state =3D=3D NULL || > > (strm->next_in =3D=3D NULL && strm->avail_in !=3D 0)) >=20 > Unzipping to NULL is not an attribute of PPC, but rather of being cal= led > from a bootloader that wants to unpack a kernel to NULL. Which makes > this patch wrong on two accounts. It leaves the bug for CONFIG_PPC a= nd > it may break bootloaders on other architectures. A quick grep shows > xtensa - no clue whether it loads the kernel to NULL or elsewhere. I was about to say... so thanks! :) > I'd prefer zlib_inflate to take a flag parameter to disable the check= =2E > Then we can have two wrappers roughly like this: >=20 > int zlib_inflate(z_streamp strm, int flush) > { > return __zlib_inflate(strm, flush, 1); > } >=20 > int zlib_inflate_null_ok_for_bootloaders_only(z_streamp strm, int flu= sh) > { > return __zlib_inflate(strm, flush, 0); > } >=20 > Or we could even make the two wrappers inline functions and move them= to > zlib.h. Sounds like a plan to me, except maybe zlib_inflate_unsafe() and a comment above the wrapper saying what/why is going on? --=20 Tom Rini -- 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