* Phillip Lougher (phil.lougher@gmail.com) wrote:

On Tue, Jan 20, 2009 at 4:51 PM, Eric Sesterhenn [off-list ref] wrote:

quoted

I already tested squashfs. One issue is basically a problem with
the zlib-api for which i just posted a patch here
http://marc.info/?t=123212807300003&r=1&w=2

Thanks for testing Squashfs.  I've not ignored your emails, but I've
been busy job hunting, and so have not had time to look into this
until now.

no problem, made me take a closer look at the issue :-) Hope you were
successfull

I hardened Squashfs against fsfuzzer back in November 2006 (remember
the month of kernel bugs, or MOKB, which highlighted a number of
issues with Squashfs).  Your testing has thrown up a regression that I
inadvertently  put in last month!

Thats why I do the fsfuzzer tests for every kernel i test :-)

will you do the "wheter" -> "whether" change for the other patch locally
and push it or do you want me to send a changed version?

quoted

The other is an overwritten redzone (also reported in this thread
http://marc.info/?l=linux-fsdevel&m=123212794425497&w=2)
Looks like a length parameter is passed to squashfs_read_data
which is bigger than ((msblk->block_size >> msblk->devblksize_log2) +
1), so the kcalloced buffer gets overwritten later.

As part of the mainlining effort I changed Squashfs to allocate
buffers in 4K page sizes rather than use vmalloced large buffers.   As
far as zlib goes, it means zlib_inflate now decompresses into a
sequence of 4K buffers rather than one large buffer.   What this means
is zlib_inflate is called repeatedly moving to the next 4K page
whenever zlib_inflate asks for another buffer (stream.avail_out == 0).

Your testing have thrown up the case where zlib_inflate is asking for
too many output buffers, i.e. it has returned with Z_OK,
stream.avail_in !=0 (more input data to be processed), and
stream.avail_out == 0 (I'd like another output buffer).  but it has
consumed all the output buffers.  This isn't checked (the code assumes
zlib will do the right thing on corrupt data and bomb out).  My guess
is either zlib_inflate is getting confused with corrupt data, or
fsfuzzer gets lucky sometimes and corrupts the filesystem to point to
another valid but larger compressed block (i.e. in your test
filesystems the 4K datablock is being corrupted to point to an 8K
metadata block).

This ultimately leads to an oops in zlib_inflate where it has been
passed a bogus or NULL steam.next_out pointer.

I'll create a patch and send it to you if you're happy to test it.

Sure, just throw it in my direction.

Greetings, Eric