From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?utf-8?B?SsO2cm4=?= Engel Subject: Re: [Patch] NULL pointer deref with corrupted squashfs image Date: Fri, 16 Jan 2009 20:07:32 +0100 Message-ID: <20090116190725.GA19453@logfs.org> References: <20090113124027.GB16333@alice> <20090116174525.GA31869@alice> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: phillip@lougher.demon.co.uk, linux-fsdevel@vger.kernel.org, jacmet@sunsite.dk, trini@kernel.crashing.org, rpurdie@rpsys.net To: Eric Sesterhenn Return-path: Received: from lazybastard.de ([212.112.238.170]:57872 "EHLO longford.logfs.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1763992AbZAPTHs (ORCPT ); Fri, 16 Jan 2009 14:07:48 -0500 Content-Disposition: inline In-Reply-To: <20090116174525.GA31869@alice> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: 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 image= s. >=20 > Signed-off-by: Eric Sesterhenn >=20 > --- linux/lib/zlib_inflate/inflate.c.orig 2009-01-16 15:40:04.0000000= 00 +0100 > +++ linux/lib/zlib_inflate/inflate.c 2009-01-16 15:41:42.000000000 +0= 100 > @@ -347,8 +347,12 @@ int zlib_inflate(z_streamp strm, int flu > static const unsigned short order[19] =3D /* permutation of code= 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 zImag= e > - 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)) Unzipping to NULL is not an attribute of PPC, but rather of being calle= d 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 and it may break bootloaders on other architectures. A quick grep shows xtensa - no clue whether it loads the kernel to NULL or elsewhere. I'd prefer zlib_inflate to take a flag parameter to disable the check. Then we can have two wrappers roughly like this: int zlib_inflate(z_streamp strm, int flush) { return __zlib_inflate(strm, flush, 1); } int zlib_inflate_null_ok_for_bootloaders_only(z_streamp strm, int flush= ) { return __zlib_inflate(strm, flush, 0); } Or we could even make the two wrappers inline functions and move them t= o zlib.h. J=C3=B6rn --=20 He that composes himself is wiser than he that composes a book. -- B. Franklin -- 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