public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [GIT PULL] Squashfs fixes for 2.6.29?
@ 2009-03-05  1:54 Phillip Lougher
  2009-03-09 21:39 ` Stefan Lippers-Hollmann
  0 siblings, 1 reply; 11+ messages in thread
From: Phillip Lougher @ 2009-03-05  1:54 UTC (permalink / raw)
  To: Linus Torvalds, Linux-kernel, Andrew Morton

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [GIT PULL] Squashfs fixes for 2.6.29?
  2009-03-05  1:54 [GIT PULL] Squashfs fixes for 2.6.29? Phillip Lougher
@ 2009-03-09 21:39 ` Stefan Lippers-Hollmann
  2009-03-10  2:13   ` Phillip Lougher
  0 siblings, 1 reply; 11+ messages in thread
From: Stefan Lippers-Hollmann @ 2009-03-09 21:39 UTC (permalink / raw)
  To: Phillip Lougher; +Cc: Linus Torvalds, Linux-kernel, Andrew Morton

[-- Attachment #1: Type: text/plain, Size: 3390 bytes --]

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:anonymous@squashfs.cvs.sourceforge.net:/cvsroot/squashfs login 
	cvs -z3 -d:pserver:anonymous@squashfs.cvs.sourceforge.net:/cvsroot/squashfs co -P squashfs/squashfs-tools
[2]	From 118e1ef6fabfc023126e6075f6ac0fc729cb5285 Mon Sep 17 00:00:00 2001
	From: Phillip Lougher <phillip@lougher.demon.co.uk>
	Date: Thu, 5 Mar 2009 00:31:12 +0000
	Subject: [PATCH 16/17] Squashfs: Fix oops when reading fsfuzzer corrupted filesystems

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [GIT PULL] Squashfs fixes for 2.6.29?
  2009-03-09 21:39 ` Stefan Lippers-Hollmann
@ 2009-03-10  2:13   ` Phillip Lougher
  2009-03-10 11:02     ` Geert Uytterhoeven
  2009-03-10 20:02     ` Stefan Lippers-Hollmann
  0 siblings, 2 replies; 11+ messages in thread
From: Phillip Lougher @ 2009-03-10  2:13 UTC (permalink / raw)
  To: Stefan Lippers-Hollmann; +Cc: Linux-kernel, Andrew Morton

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [GIT PULL] Squashfs fixes for 2.6.29?
  2009-03-10  2:13   ` Phillip Lougher
@ 2009-03-10 11:02     ` Geert Uytterhoeven
  2009-03-10 13:19       ` Woody Suwalski
                         ` (2 more replies)
  2009-03-10 20:02     ` Stefan Lippers-Hollmann
  1 sibling, 3 replies; 11+ messages in thread
From: Geert Uytterhoeven @ 2009-03-10 11:02 UTC (permalink / raw)
  To: Phillip Lougher; +Cc: Stefan Lippers-Hollmann, Linux-kernel, Andrew Morton

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 <Geert.Uytterhoeven@sonycom.com>

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:   Geert.Uytterhoeven@sonycom.com
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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [GIT PULL] Squashfs fixes for 2.6.29?
  2009-03-10 11:02     ` Geert Uytterhoeven
@ 2009-03-10 13:19       ` Woody Suwalski
  2009-03-10 21:24       ` Stefan Lippers-Hollmann
  2009-03-10 21:33       ` Phillip Lougher
  2 siblings, 0 replies; 11+ messages in thread
From: Woody Suwalski @ 2009-03-10 13:19 UTC (permalink / raw)
  To: Phillip Lougher; +Cc: Linux-kernel, Andrew Morton

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 <Geert.Uytterhoeven@sonycom.com>
>
> 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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [GIT PULL] Squashfs fixes for 2.6.29?
  2009-03-10  2:13   ` Phillip Lougher
  2009-03-10 11:02     ` Geert Uytterhoeven
@ 2009-03-10 20:02     ` Stefan Lippers-Hollmann
  1 sibling, 0 replies; 11+ messages in thread
From: Stefan Lippers-Hollmann @ 2009-03-10 20:02 UTC (permalink / raw)
  To: Phillip Lougher; +Cc: Linux-kernel, Andrew Morton

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [GIT PULL] Squashfs fixes for 2.6.29?
  2009-03-10 11:02     ` Geert Uytterhoeven
  2009-03-10 13:19       ` Woody Suwalski
@ 2009-03-10 21:24       ` Stefan Lippers-Hollmann
  2009-03-10 21:33       ` Phillip Lougher
  2 siblings, 0 replies; 11+ messages in thread
From: Stefan Lippers-Hollmann @ 2009-03-10 21:24 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: Phillip Lougher, Linux-kernel, Andrew Morton

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 <Geert.Uytterhoeven@sonycom.com>
> 
> 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:   Geert.Uytterhoeven@sonycom.com
> 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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [GIT PULL] Squashfs fixes for 2.6.29?
  2009-03-10 11:02     ` Geert Uytterhoeven
  2009-03-10 13:19       ` Woody Suwalski
  2009-03-10 21:24       ` Stefan Lippers-Hollmann
@ 2009-03-10 21:33       ` Phillip Lougher
  2009-03-11  6:12         ` Phillip Lougher
  2 siblings, 1 reply; 11+ messages in thread
From: Phillip Lougher @ 2009-03-10 21:33 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Phillip Lougher, Stefan Lippers-Hollmann, Linux-kernel,
	Andrew Morton

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [GIT PULL] Squashfs fixes for 2.6.29?
  2009-03-10 21:33       ` Phillip Lougher
@ 2009-03-11  6:12         ` Phillip Lougher
  2009-03-11 14:18           ` Geert Uytterhoeven
  2009-03-11 20:46           ` Stefan Lippers-Hollmann
  0 siblings, 2 replies; 11+ messages in thread
From: Phillip Lougher @ 2009-03-11  6:12 UTC (permalink / raw)
  To: Phillip Lougher
  Cc: Geert Uytterhoeven, Stefan Lippers-Hollmann, Linux-kernel,
	Andrew Morton

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) {





^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [GIT PULL] Squashfs fixes for 2.6.29?
  2009-03-11  6:12         ` Phillip Lougher
@ 2009-03-11 14:18           ` Geert Uytterhoeven
  2009-03-11 20:46           ` Stefan Lippers-Hollmann
  1 sibling, 0 replies; 11+ messages in thread
From: Geert Uytterhoeven @ 2009-03-11 14:18 UTC (permalink / raw)
  To: Phillip Lougher
  Cc: Phillip Lougher, Stefan Lippers-Hollmann, Linux-kernel,
	Andrew Morton

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 <Geert.Uytterhoeven@sonycom.com>

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:   Geert.Uytterhoeven@sonycom.com
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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [GIT PULL] Squashfs fixes for 2.6.29?
  2009-03-11  6:12         ` Phillip Lougher
  2009-03-11 14:18           ` Geert Uytterhoeven
@ 2009-03-11 20:46           ` Stefan Lippers-Hollmann
  1 sibling, 0 replies; 11+ messages in thread
From: Stefan Lippers-Hollmann @ 2009-03-11 20:46 UTC (permalink / raw)
  To: Phillip Lougher
  Cc: Phillip Lougher, Geert Uytterhoeven, Linux-kernel, Andrew Morton

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 <s.L-H@gmx.de>
Tested-by: Stefan Lippers-Hollmann <s.L-H@gmx.de>

> 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) {

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2009-03-11 20:47 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-03-05  1:54 [GIT PULL] Squashfs fixes for 2.6.29? Phillip Lougher
2009-03-09 21:39 ` Stefan Lippers-Hollmann
2009-03-10  2:13   ` Phillip Lougher
2009-03-10 11:02     ` Geert Uytterhoeven
2009-03-10 13:19       ` Woody Suwalski
2009-03-10 21:24       ` Stefan Lippers-Hollmann
2009-03-10 21:33       ` Phillip Lougher
2009-03-11  6:12         ` Phillip Lougher
2009-03-11 14:18           ` Geert Uytterhoeven
2009-03-11 20:46           ` Stefan Lippers-Hollmann
2009-03-10 20:02     ` Stefan Lippers-Hollmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox