On Fri, Jan 16, 2009 at 08:07:32PM +0100, Jörn Engel wrote:
> On Fri, 16 January 2009 18:45:25 +0100, Eric Sesterhenn wrote:
> >
> > 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 images.
> >
> > Signed-off-by: Eric Sesterhenn <snakebyte@xxxxxx>
> >
> > --- linux/lib/zlib_inflate/inflate.c.orig 2009-01-16 15:40:04.000000000 +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] = /* 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 */
> > + /* 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
> >
> > if (strm == NULL || strm->state == NULL ||
> > (strm->next_in == NULL && strm->avail_in != 0))
>
> Unzipping to NULL is not an attribute of PPC, but rather of being called
> 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 was about to say... so thanks! :)
> 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 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?
--
Tom Rini
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
