2009-03-05 01:55:06

by Phillip Lougher

[permalink] [raw]
Subject: [GIT PULL] Squashfs fixes for 2.6.29?

Hi Linus,

Please consider pulling the following Squashfs fixes. They fix some
potential oopses when dealing with corrupted filesystems (plus a trivial
documentation typo fix).

Please pull from

git://git.kernel.org/pub/scm/linux/kernel/git/pkl/squashfs-linus.git

Phillip Lougher (2):
Squashfs: Fix oops when reading fsfuzzer corrupted filesystems
Squashfs: fix documentation typo, Cramfs filesystem limit is 256 MiB

Roel Kluin (1):
Squashfs: frag_size should be signed, as it can hold an error result

Documentation/filesystems/squashfs.txt | 2 +-
fs/squashfs/block.c | 13 +++++++++++--
fs/squashfs/cache.c | 4 ++--
fs/squashfs/inode.c | 6 ++++--
fs/squashfs/squashfs.h | 2 +-
fs/squashfs/super.c | 2 +-
6 files changed, 20 insertions(+), 9 deletions(-)

Thanks

Phillip


2009-03-09 21:39:31

by Stefan Lippers-Hollmann

[permalink] [raw]
Subject: Re: [GIT PULL] Squashfs fixes for 2.6.29?

Hi

On Donnerstag, 5. M?rz 2009, Phillip Lougher wrote:
> Hi Linus,
>
> Please consider pulling the following Squashfs fixes. They fix some
> potential oopses when dealing with corrupted filesystems (plus a trivial
> documentation typo fix).
>
> Please pull from
>
> git://git.kernel.org/pub/scm/linux/kernel/git/pkl/squashfs-linus.git
>
> Phillip Lougher (2):
> Squashfs: Fix oops when reading fsfuzzer corrupted filesystems
[...]

This patch seems to break squashfs for me on i386 and amd64.

Test environment is a squashed filesystem image (live CD image, but also
tested manually with a loop mounted iso9660 and loop mounted squashfs;
kernel 2.6.29-rc7-git2). The squashfs image has been created with
squashfs-tools CVS[1] as of today (latest commit 2009-03-03).

# mkdir /tmp/pkg
# mount -o loop /var/cache/pyfll/sidux-snapshot-xfce-lite-i386-200903091945.iso /mnt
# mount -t squashfs -o loop /mnt/sidux/sidux.686 /mnt/
# LANG= cp -a /mnt/ /tmp/pkg/
cp: reading `/mnt/lib/modules/2.6.28-7.slh.3-sidux-686/kernel/drivers/media/dvb/ttpci/dvb-ttpci.ko': Input/output error
cp: cannot stat `/mnt/lib/modules/2.6.29-rc7-sidux-686/kernel/drivers/hwmon': Input/output error
cp: cannot stat `/mnt/lib/modules/2.6.29-rc7-sidux-686/kernel/drivers/i2c': Input/output error
cp: cannot stat `/mnt/lib/modules/2.6.29-rc7-sidux-686/kernel/drivers/infiniband': Input/output error
[...]

dmesg shows the following:

ISO 9660 Extensions: Microsoft Joliet Level 3
ISO 9660 Extensions: RRIP_1991A
squashfs: version 4.0 (2009/01/31) Phillip Lougher
SQUASHFS error: zlib_inflate tried to decompress too much data, expected 131072 bytes. Zlib data probably corrupt
SQUASHFS error: squashfs_read_data failed to read block 0xb7781f
SQUASHFS error: Unable to read data cache entry [b7781f]
SQUASHFS error: Unable to read page, block b7781f, size c7e5
SQUASHFS error: Unable to read data cache entry [b7781f]
SQUASHFS error: Unable to read page, block b7781f, size c7e5
SQUASHFS error: Unable to read data cache entry [b7781f]
SQUASHFS error: Unable to read page, block b7781f, size c7e5
SQUASHFS error: Unable to read data cache entry [b7781f]
SQUASHFS error: Unable to read page, block b7781f, size c7e5
SQUASHFS error: Unable to read data cache entry [b7781f]
SQUASHFS error: Unable to read page, block b7781f, size c7e5
SQUASHFS error: zlib_inflate tried to decompress too much data, expected 8192 bytes. Zlib data probably corrupt
SQUASHFS error: squashfs_read_data failed to read block 0x17135aab
SQUASHFS error: Unable to read metadata cache entry [17135aab]
SQUASHFS error: Unable to read inode 0xc37f06f2
[...]

Reverting just this patch[2] results in flawless operations (I have the
same problems [and workaround, by reverting this patch] with the squashfs4
patches applied to 2.6.28.7, so that seems to rule out unrelated breakage).

Regards
Stefan Lippers-Hollmann

[1] cvs -d:pserver:[email protected]:/cvsroot/squashfs login
cvs -z3 -d:pserver:[email protected]:/cvsroot/squashfs co -P squashfs/squashfs-tools
[2] From 118e1ef6fabfc023126e6075f6ac0fc729cb5285 Mon Sep 17 00:00:00 2001
From: Phillip Lougher <[email protected]>
Date: Thu, 5 Mar 2009 00:31:12 +0000
Subject: [PATCH 16/17] Squashfs: Fix oops when reading fsfuzzer corrupted filesystems


Attachments:
(No filename) (3.24 kB)
signature.asc (197.00 B)
This is a digitally signed message part.
Download all attachments

2009-03-10 02:13:24

by Phillip Lougher

[permalink] [raw]
Subject: Re: [GIT PULL] Squashfs fixes for 2.6.29?

Stefan Lippers-Hollmann wrote:

>
> This patch seems to break squashfs for me on i386 and amd64.
>
> Test environment is a squashed filesystem image (live CD image, but also
> tested manually with a loop mounted iso9660 and loop mounted squashfs;
> kernel 2.6.29-rc7-git2). The squashfs image has been created with
> squashfs-tools CVS[1] as of today (latest commit 2009-03-03).
>

Hi,

Can you send me a filesystem (or link to one) which exhibits this? Zlib
is obviously showing unexpected behaviour...

Thanks

Phillip

2009-03-10 11:03:17

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [GIT PULL] Squashfs fixes for 2.6.29?

On Tue, 10 Mar 2009, Phillip Lougher wrote:
> Stefan Lippers-Hollmann wrote:
> > This patch seems to break squashfs for me on i386 and amd64.
> > Test environment is a squashed filesystem image (live CD image, but also
> > tested manually with a loop mounted iso9660 and loop mounted squashfs;
> > kernel 2.6.29-rc7-git2). The squashfs image has been created with
> > squashfs-tools CVS[1] as of today (latest commit 2009-03-03).
> >
>
> Can you send me a filesystem (or link to one) which exhibits this? Zlib is
> obviously showing unexpected behaviour...

I see the same thing here. I'll send you a test file system by private email.

The patch below fixes it. It seems zlib sometimes does need an additional
loop ;-)

Note that I expect it may now loop forever in case of file system corruption.

Signed-off-by: Geert Uytterhoeven <[email protected]>

diff --git a/fs/squashfs/block.c b/fs/squashfs/block.c
index 321728f..46358dd 100644
--- a/fs/squashfs/block.c
+++ b/fs/squashfs/block.c
@@ -184,15 +184,7 @@ int squashfs_read_data(struct super_block *sb, void **buffer, u64 index,
offset = 0;
}

- if (msblk->stream.avail_out == 0) {
- if (page == pages) {
- ERROR("zlib_inflate tried to "
- "decompress too much data, "
- "expected %d bytes. Zlib "
- "data probably corrupt\n",
- srclength);
- goto release_mutex;
- }
+ if (msblk->stream.avail_out == 0 && page < pages) {
msblk->stream.next_out = buffer[page++];
msblk->stream.avail_out = PAGE_CACHE_SIZE;
}

With kind regards,

Geert Uytterhoeven
Software Architect

Sony Techsoft Centre Europe
The Corporate Village ? Da Vincilaan 7-D1 ? B-1935 Zaventem ? Belgium

Phone: +32 (0)2 700 8453
Fax: +32 (0)2 700 8622
E-mail: [email protected]
Internet: http://www.sony-europe.com/

A division of Sony Europe (Belgium) N.V.
VAT BE 0413.825.160 ? RPR Brussels
Fortis ? BIC GEBABEBB ? IBAN BE41293037680010

2009-03-10 13:30:52

by Woody Suwalski

[permalink] [raw]
Subject: Re: [GIT PULL] Squashfs fixes for 2.6.29?

Geert Uytterhoeven wrote:
> On Tue, 10 Mar 2009, Phillip Lougher wrote:
>
>> Stefan Lippers-Hollmann wrote:
>>
>>> This patch seems to break squashfs for me on i386 and amd64.
>>> Test environment is a squashed filesystem image (live CD image, but also
>>> tested manually with a loop mounted iso9660 and loop mounted squashfs;
>>> kernel 2.6.29-rc7-git2). The squashfs image has been created with
>>> squashfs-tools CVS[1] as of today (latest commit 2009-03-03).
>>>
>>>
>> Can you send me a filesystem (or link to one) which exhibits this? Zlib is
>> obviously showing unexpected behaviour...
>>
>
> I see the same thing here. I'll send you a test file system by private email.
>
> The patch below fixes it. It seems zlib sometimes does need an additional
> loop ;-)
>
> Note that I expect it may now loop forever in case of file system corruption.
>
> Signed-off-by: Geert Uytterhoeven <[email protected]>
>
> diff --git a/fs/squashfs/block.c b/fs/squashfs/block.c
> index 321728f..46358dd 100644
> --- a/fs/squashfs/block.c
> +++ b/fs/squashfs/block.c
> @@ -184,15 +184,7 @@ int squashfs_read_data(struct super_block *sb, void **buffer, u64 index,
> offset = 0;
> }
>
> - if (msblk->stream.avail_out == 0) {
> - if (page == pages) {
> - ERROR("zlib_inflate tried to "
> - "decompress too much data, "
> - "expected %d bytes. Zlib "
> - "data probably corrupt\n",
> - srclength);
> - goto release_mutex;
> - }
> + if (msblk->stream.avail_out == 0 && page < pages) {
> msblk->stream.next_out = buffer[page++];
> msblk->stream.avail_out = PAGE_CACHE_SIZE;
> }
>
> With kind regards,
>
> Geert Uytterhoeven
> Software Architect
>
>
Same here. I have experimentally commented out the "goto release_mutex"
and I see that the error message is printed once or twice during boot.
(and we are banging on it heavily - no real file system problems seen).
In my case squash4 is a 600M+ all the system partition to be aufs'd with
a read-write portion.
Leaving the patch as it is renders the system unbootable - weird errors
resulting from aborted reads....

Woody

--
Woody Suwalski, Xandros, Ottawa, Canada, 1-613-842-3498 x414

2009-03-10 20:03:17

by Stefan Lippers-Hollmann

[permalink] [raw]
Subject: Re: [GIT PULL] Squashfs fixes for 2.6.29?

Hi

On Dienstag, 10. M?rz 2009, Phillip Lougher wrote:
> Stefan Lippers-Hollmann wrote:
>
> >
> > This patch seems to break squashfs for me on i386 and amd64.
> >
> > Test environment is a squashed filesystem image (live CD image, but also
> > tested manually with a loop mounted iso9660 and loop mounted squashfs;
> > kernel 2.6.29-rc7-git2). The squashfs image has been created with
> > squashfs-tools CVS[1] as of today (latest commit 2009-03-03).
> >
>
> Hi,
>
> Can you send me a filesystem (or link to one) which exhibits this? Zlib
> is obviously showing unexpected behaviour...

http://sidux.com/slh/squashfs/sidux-snapshot-minimal-i386-200903101824.iso [104 MB]

MD5:
03475b9793134f48306fe2ba7f26ce7c *sidux-snapshot-minimal-i386-200903101824.iso

SHA256:
6ff9ec65102af8e2277a0ec30e8db13be2313ef64c9a3a3b5febaafab37cf00f *sidux-snapshot-minimal-i386-200903101824.iso

The squashfs image is stored inside the ISO9660 container as
"sidux/sidux.686", it is bootable and the installed kernel (2.6.29-rc7-git3
+ aufs2, it's required for the live system, but I have checked that it is
also reproducable with plain linux-2.6.git and without any patches, outside
of a live environment) exposes this problem.

Sources for all software installed inside the squashfs image is available
at:
http://sidux.com/slh/squashfs/sidux-snapshot-minimal-i386-200903101824-source/

The (Debian-) packaged kernel source as:
dget http://sidux.com/slh/squashfs/sidux-snapshot-minimal-i386-200903101824-source/linux-sidux-2.6_2.6.29~rc7-1~git3.slh.0.dsc [67 MB]
and as plain tarball with all patches pre-applied:
http://sidux.com/slh/squashfs/linux-source-sidux-2.6.29-rc7.tar.bz2 [51 MB]

which consists of the following sources/ patches:
http://eu.kernel.org/pub/linux/kernel/v2.6/linux-2.6.28.tar.bz2 [64 MB]
http://eu.kernel.org/pub/linux/kernel/v2.6/testing/patch-2.6.29-rc7.bz2 [12 MB]
http://eu.kernel.org/pub/linux/kernel/v2.6/snapshots/patch-2.6.29-rc7-git3.bz2 [60 KB]

and required for the live CD, but reproducable without these:
http://svn.berlios.de/svnroot/repos/fullstory/linux-sidux-2.6/trunk/debian/patches/debian/version.patch [4 KB]
http://svn.berlios.de/svnroot/repos/fullstory/linux-sidux-2.6/trunk/debian/patches/debian/kernelvariables.patch [4 KB]
http://svn.berlios.de/svnroot/repos/fullstory/linux-sidux-2.6/trunk/debian/patches/debian/doc-build-parallel.patch [4 KB]
http://svn.berlios.de/svnroot/repos/fullstory/linux-sidux-2.6/trunk/debian/patches/debian/scripts-kconfig-reportoldconfig.patch [12 KB]
http://svn.berlios.de/svnroot/repos/fullstory/linux-sidux-2.6/trunk/debian/patches/features/aufs2-20090309.diff [632 KB]

used Kernel config:
http://sidux.com/slh/squashfs/config-2.6.29-rc7-sidux-686 [96 KB]

> Thanks
>
> Phillip

Thank you a lot for your efforts, I'm now going to test Geert
Uytterhoeven's proposed patch and will report back.

Regards
Stefan Lippers-Hollmann

2009-03-10 21:25:27

by Stefan Lippers-Hollmann

[permalink] [raw]
Subject: Re: [GIT PULL] Squashfs fixes for 2.6.29?

Hi

On Dienstag, 10. M?rz 2009, Geert Uytterhoeven wrote:
> On Tue, 10 Mar 2009, Phillip Lougher wrote:
> > Stefan Lippers-Hollmann wrote:
> > > This patch seems to break squashfs for me on i386 and amd64.
> > > Test environment is a squashed filesystem image (live CD image, but also
> > > tested manually with a loop mounted iso9660 and loop mounted squashfs;
> > > kernel 2.6.29-rc7-git2). The squashfs image has been created with
> > > squashfs-tools CVS[1] as of today (latest commit 2009-03-03).
> > >
> >
> > Can you send me a filesystem (or link to one) which exhibits this? Zlib is
> > obviously showing unexpected behaviour...
>
> I see the same thing here. I'll send you a test file system by private email.
>
> The patch below fixes it. It seems zlib sometimes does need an additional
> loop ;-)
>
> Note that I expect it may now loop forever in case of file system corruption.

This patch seems to work for me, no further errors while copying files and
no syslog messages.

Regards
Stefan Lippers-Hollmann
--

> Signed-off-by: Geert Uytterhoeven <[email protected]>
>
> diff --git a/fs/squashfs/block.c b/fs/squashfs/block.c
> index 321728f..46358dd 100644
> --- a/fs/squashfs/block.c
> +++ b/fs/squashfs/block.c
> @@ -184,15 +184,7 @@ int squashfs_read_data(struct super_block *sb, void **buffer, u64 index,
> offset = 0;
> }
>
> - if (msblk->stream.avail_out == 0) {
> - if (page == pages) {
> - ERROR("zlib_inflate tried to "
> - "decompress too much data, "
> - "expected %d bytes. Zlib "
> - "data probably corrupt\n",
> - srclength);
> - goto release_mutex;
> - }
> + if (msblk->stream.avail_out == 0 && page < pages) {
> msblk->stream.next_out = buffer[page++];
> msblk->stream.avail_out = PAGE_CACHE_SIZE;
> }
>
> With kind regards,
>
> Geert Uytterhoeven
> Software Architect
>
> Sony Techsoft Centre Europe
> The Corporate Village ? Da Vincilaan 7-D1 ? B-1935 Zaventem ? Belgium
>
> Phone: +32 (0)2 700 8453
> Fax: +32 (0)2 700 8622
> E-mail: [email protected]
> Internet: http://www.sony-europe.com/
>
> A division of Sony Europe (Belgium) N.V.
> VAT BE 0413.825.160 ? RPR Brussels
> Fortis ? BIC GEBABEBB ? IBAN BE41293037680010

2009-03-10 21:33:20

by Phillip Lougher

[permalink] [raw]
Subject: Re: [GIT PULL] Squashfs fixes for 2.6.29?

Geert Uytterhoeven wrote:
> On Tue, 10 Mar 2009, Phillip Lougher wrote:
>> Stefan Lippers-Hollmann wrote:
>>> This patch seems to break squashfs for me on i386 and amd64.
>>> Test environment is a squashed filesystem image (live CD image, but also
>>> tested manually with a loop mounted iso9660 and loop mounted squashfs;
>>> kernel 2.6.29-rc7-git2). The squashfs image has been created with
>>> squashfs-tools CVS[1] as of today (latest commit 2009-03-03).
>>>
>> Can you send me a filesystem (or link to one) which exhibits this? Zlib is
>> obviously showing unexpected behaviour...
>
> I see the same thing here. I'll send you a test file system by private email.
>

OK thanks, I've received it, and reproduced the problem :-)

> The patch below fixes it. It seems zlib sometimes does need an additional
> loop ;-)
>

Yes thanks.

> Note that I expect it may now loop forever in case of file system corruption.

This is the next thing, to redo the file system corruption patch.

Thanks

Phillip

2009-03-11 06:13:11

by Phillip Lougher

[permalink] [raw]
Subject: Re: [GIT PULL] Squashfs fixes for 2.6.29?

Phillip Lougher wrote:
> Geert Uytterhoeven wrote:
>> On Tue, 10 Mar 2009, Phillip Lougher wrote:
>>> Stefan Lippers-Hollmann wrote:
>>>> This patch seems to break squashfs for me on i386 and amd64.
>>>> Test environment is a squashed filesystem image (live CD image, but also
>>>> tested manually with a loop mounted iso9660 and loop mounted squashfs;
>>>> kernel 2.6.29-rc7-git2). The squashfs image has been created with
>>>> squashfs-tools CVS[1] as of today (latest commit 2009-03-03).
>>>>
>>> Can you send me a filesystem (or link to one) which exhibits this? Zlib is
>>> obviously showing unexpected behaviour...
>> I see the same thing here. I'll send you a test file system by private email.
>>
>
> OK thanks, I've received it, and reproduced the problem :-)
>
>> The patch below fixes it. It seems zlib sometimes does need an additional
>> loop ;-)
>>
>
> Yes thanks.
>
>> Note that I expect it may now loop forever in case of file system corruption.
>
> This is the next thing, to redo the file system corruption patch.
>


Hi,

The following patch fixes the problems you've experienced with your test
filesystems (thanks for making them available), and also deals with the corrupted
filesystems issue. It correctly works with all the corrupted filesystems sent
earlier this year.

The problem arises due to an unexpected corner-case with zlib which the
corrupted filesystems patch didn't address. Very occasionally zlib exits
needing a couple of extra bytes of input (1 and 2 bytes in the test filesystems),
but with avail_out == 0 and no more output buffers available. This situation
was incorrectly flagged as an error by the corrupted filesystems patch. I
unfortunately didn't anticipate this scenario because it seems contrary to
expectation that zlib will be still reading input having produced all the
expected output. The fix is to not print an error if zlib wants more input
bytes and there's more input bytes to be read.

Geert I can put a signed off line from you on the patch if you like (as you
sent the original fix). I could add reported and tested lines from both of you?
They seem to be being used more often and sound a good idea.

Thanks

Phillip

diff --git a/fs/squashfs/block.c b/fs/squashfs/block.c
index 321728f..b85173f 100644
--- a/fs/squashfs/block.c
+++ b/fs/squashfs/block.c
@@ -166,6 +166,22 @@ int squashfs_read_data(struct super_block *sb, void **buffer, u64 index,

bytes = length;
do {
+ if (msblk->stream.avail_out == 0) {
+ if (page < pages) {
+ msblk->stream.next_out = buffer[page++];
+ msblk->stream.avail_out =
+ PAGE_CACHE_SIZE;
+ } else if (msblk->stream.avail_in > 0
+ || bytes == 0) {
+ ERROR("zlib_inflate tried to "
+ "decompress too much data, "
+ "expected %d bytes. Zlib "
+ "data probably corrupt\n",
+ srclength);
+ goto release_mutex;
+ }
+ }
+
if (msblk->stream.avail_in == 0 && k < b) {
avail = min(bytes, msblk->devblksize - offset);
bytes -= avail;
@@ -184,19 +200,6 @@ int squashfs_read_data(struct super_block *sb, void **buffer, u64 index,
offset = 0;
}

- if (msblk->stream.avail_out == 0) {
- if (page == pages) {
- ERROR("zlib_inflate tried to "
- "decompress too much data, "
- "expected %d bytes. Zlib "
- "data probably corrupt\n",
- srclength);
- goto release_mutex;
- }
- msblk->stream.next_out = buffer[page++];
- msblk->stream.avail_out = PAGE_CACHE_SIZE;
- }
-
if (!zlib_init) {
zlib_err = zlib_inflateInit(&msblk->stream);
if (zlib_err != Z_OK) {



2009-03-11 14:19:26

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [GIT PULL] Squashfs fixes for 2.6.29?

On Wed, 11 Mar 2009, Phillip Lougher wrote:
> Phillip Lougher wrote:
> > Geert Uytterhoeven wrote:
> > > On Tue, 10 Mar 2009, Phillip Lougher wrote:
> > > > Stefan Lippers-Hollmann wrote:
> > > > > This patch seems to break squashfs for me on i386 and amd64.
> > > > > Test environment is a squashed filesystem image (live CD image, but
> > > > > also
> > > > > tested manually with a loop mounted iso9660 and loop mounted squashfs;
> > > > > kernel 2.6.29-rc7-git2). The squashfs image has been created with
> > > > > squashfs-tools CVS[1] as of today (latest commit 2009-03-03).
> > > > >
> > > > Can you send me a filesystem (or link to one) which exhibits this? Zlib
> > > > is
> > > > obviously showing unexpected behaviour...
> > > I see the same thing here. I'll send you a test file system by private
> > > email.
> > >
> >
> > OK thanks, I've received it, and reproduced the problem :-)
> >
> > > The patch below fixes it. It seems zlib sometimes does need an additional
> > > loop ;-)
> > >
> >
> > Yes thanks.
> >
> > > Note that I expect it may now loop forever in case of file system
> > > corruption.
> >
> > This is the next thing, to redo the file system corruption patch.
>
> The following patch fixes the problems you've experienced with your test
> filesystems (thanks for making them available), and also deals with the
> corrupted
> filesystems issue. It correctly works with all the corrupted filesystems sent
> earlier this year.
>
> The problem arises due to an unexpected corner-case with zlib which the
> corrupted filesystems patch didn't address. Very occasionally zlib exits
> needing a couple of extra bytes of input (1 and 2 bytes in the test
> filesystems),

I saw up to 6 extra bytes of input in another file system.

I tried to corrupt the extra bytes and see what happens. So far zlib detected
the corruption in all cases and returned Z_DATA_ERROR, hence there was no
infinite looping.

> but with avail_out == 0 and no more output buffers available. This situation
> was incorrectly flagged as an error by the corrupted filesystems patch. I
> unfortunately didn't anticipate this scenario because it seems contrary to
> expectation that zlib will be still reading input having produced all the
> expected output. The fix is to not print an error if zlib wants more input
> bytes and there's more input bytes to be read.
>
> Geert I can put a signed off line from you on the patch if you like (as you
> sent the original fix). I could add reported and tested lines from both of
> you?
> They seem to be being used more often and sound a good idea.

Tested-by: Geert Uytterhoeven <[email protected]>

Please note that I had to apply your patch manually, as it's
whitespace-damaged. Below is a version that does apply.

diff --git a/fs/squashfs/block.c b/fs/squashfs/block.c
index 321728f..b85173f 100644
--- a/fs/squashfs/block.c
+++ b/fs/squashfs/block.c
@@ -166,6 +166,22 @@ int squashfs_read_data(struct super_block *sb, void **buffer, u64 index,

bytes = length;
do {
+ if (msblk->stream.avail_out == 0) {
+ if (page < pages) {
+ msblk->stream.next_out = buffer[page++];
+ msblk->stream.avail_out =
+ PAGE_CACHE_SIZE;
+ } else if (msblk->stream.avail_in > 0
+ || bytes == 0) {
+ ERROR("zlib_inflate tried to "
+ "decompress too much data, "
+ "expected %d bytes. Zlib "
+ "data probably corrupt\n",
+ srclength);
+ goto release_mutex;
+ }
+ }
+
if (msblk->stream.avail_in == 0 && k < b) {
avail = min(bytes, msblk->devblksize - offset);
bytes -= avail;
@@ -184,19 +200,6 @@ int squashfs_read_data(struct super_block *sb, void **buffer, u64 index,
offset = 0;
}

- if (msblk->stream.avail_out == 0) {
- if (page == pages) {
- ERROR("zlib_inflate tried to "
- "decompress too much data, "
- "expected %d bytes. Zlib "
- "data probably corrupt\n",
- srclength);
- goto release_mutex;
- }
- msblk->stream.next_out = buffer[page++];
- msblk->stream.avail_out = PAGE_CACHE_SIZE;
- }
-
if (!zlib_init) {
zlib_err = zlib_inflateInit(&msblk->stream);
if (zlib_err != Z_OK) {

With kind regards,

Geert Uytterhoeven
Software Architect

Sony Techsoft Centre Europe
The Corporate Village ? Da Vincilaan 7-D1 ? B-1935 Zaventem ? Belgium

Phone: +32 (0)2 700 8453
Fax: +32 (0)2 700 8622
E-mail: [email protected]
Internet: http://www.sony-europe.com/

A division of Sony Europe (Belgium) N.V.
VAT BE 0413.825.160 ? RPR Brussels
Fortis ? BIC GEBABEBB ? IBAN BE41293037680010

2009-03-11 20:47:17

by Stefan Lippers-Hollmann

[permalink] [raw]
Subject: Re: [GIT PULL] Squashfs fixes for 2.6.29?

Hi

On Mittwoch, 11. M?rz 2009, Phillip Lougher wrote:
> Phillip Lougher wrote:
[...]
> The following patch fixes the problems you've experienced with your test
> filesystems (thanks for making them available), and also deals with the corrupted
> filesystems issue. It correctly works with all the corrupted filesystems sent
> earlier this year.
>
> The problem arises due to an unexpected corner-case with zlib which the
> corrupted filesystems patch didn't address. Very occasionally zlib exits
> needing a couple of extra bytes of input (1 and 2 bytes in the test filesystems),
> but with avail_out == 0 and no more output buffers available. This situation
> was incorrectly flagged as an error by the corrupted filesystems patch. I
> unfortunately didn't anticipate this scenario because it seems contrary to
> expectation that zlib will be still reading input having produced all the
> expected output. The fix is to not print an error if zlib wants more input
> bytes and there's more input bytes to be read.

I just got home and around to test it with 2.6.29-rc7-git4, no further
issues with large filesystems (copying + live usage) and it's working
reliably (after whitespace fixing), thank you a lot.

> Geert I can put a signed off line from you on the patch if you like (as you
> sent the original fix). I could add reported and tested lines from both of you?
> They seem to be being used more often and sound a good idea.

Feel free to add the tags as needed, thanks.
Reported-by: Stefan Lippers-Hollmann <[email protected]>
Tested-by: Stefan Lippers-Hollmann <[email protected]>

> Thanks
>
> Phillip

Regards
Stefan Lippers-Hollmann

--
> diff --git a/fs/squashfs/block.c b/fs/squashfs/block.c
> index 321728f..b85173f 100644
> --- a/fs/squashfs/block.c
> +++ b/fs/squashfs/block.c
> @@ -166,6 +166,22 @@ int squashfs_read_data(struct super_block *sb, void **buffer, u64 index,
>
> bytes = length;
> do {
> + if (msblk->stream.avail_out == 0) {
> + if (page < pages) {
> + msblk->stream.next_out = buffer[page++];
> + msblk->stream.avail_out =
> + PAGE_CACHE_SIZE;
> + } else if (msblk->stream.avail_in > 0
> + || bytes == 0) {
> + ERROR("zlib_inflate tried to "
> + "decompress too much data, "
> + "expected %d bytes. Zlib "
> + "data probably corrupt\n",
> + srclength);
> + goto release_mutex;
> + }
> + }
> +
> if (msblk->stream.avail_in == 0 && k < b) {
> avail = min(bytes, msblk->devblksize - offset);
> bytes -= avail;
> @@ -184,19 +200,6 @@ int squashfs_read_data(struct super_block *sb, void **buffer, u64 index,
> offset = 0;
> }
>
> - if (msblk->stream.avail_out == 0) {
> - if (page == pages) {
> - ERROR("zlib_inflate tried to "
> - "decompress too much data, "
> - "expected %d bytes. Zlib "
> - "data probably corrupt\n",
> - srclength);
> - goto release_mutex;
> - }
> - msblk->stream.next_out = buffer[page++];
> - msblk->stream.avail_out = PAGE_CACHE_SIZE;
> - }
> -
> if (!zlib_init) {
> zlib_err = zlib_inflateInit(&msblk->stream);
> if (zlib_err != Z_OK) {