Re: Regression: squashfs issues since change "squashfs: migrate from ll_rw_block usage to BIO"

2020-07-16 Thread Bernd Amend
On Fri, Jul 17, 2020 at 3:22 AM Phillip Lougher
 wrote:
>
> On Fri, Jul 17, 2020 at 12:07 AM Andrew Morton
>  wrote:
> >
> > On Tue, 14 Jul 2020 21:41:07 +0200 Bernd Amend  
> > wrote:
> >
> > > Hi,
> > >
> > > With the Linux Kernel version 5.8-rc5/master I am unable to mount some
> > > squashfs filesystems compressed with "-comp lz4".
> > > If I try to mount them I get the following error:
> > > [1.084246] SQUASHFS error: lz4 decompression failed, data probably 
> > > corrupt
> > > [1.084545] SQUASHFS error: Failed to read block 0x873e1001: -5
> > > [1.084761] SQUASHFS error: Unable to read metadata cache entry 
> > > [873e0fff]
> > > [1.084983] SQUASHFS error: Unable to read directory block 
> > > [873e0fff:1586]
> > > [1.122564] SQUASHFS error: Unable to read metadata cache entry 
> > > [873e0fff]
> > > [1.122708] SQUASHFS error: Unable to read directory block 
> > > [873e0fff:1586]
> > > [1.122862] Starting init: /sbin/init exists but couldn't execute
> > > it (error -5)
> > > [1.123027] SQUASHFS error: Unable to read metadata cache entry 
> > > [873e0fff]
> > > [1.123152] SQUASHFS error: Unable to read directory block 
> > > [873e0fff:1586]
> > > [1.123279] Starting init: /etc/init exists but couldn't execute it
> > > (error -5)
> > > [1.123444] SQUASHFS error: Unable to read metadata cache entry 
> > > [873e0fff]
> > > [1.123573] SQUASHFS error: Unable to read directory block 
> > > [873e0fff:1586]
> > > [1.123713] Starting init: /bin/init exists but couldn't execute it
> > > (error -5)
> > > [1.123900] SQUASHFS error: Unable to read metadata cache entry 
> > > [873e0fff]
> > >
> > > or
> > >
> > > [ 4960.910693] attempt to access beyond end of device
> > > [ 4960.910695] loop0: rw=2048, want=46, limit=40
> > > [ 4960.910696] SQUASHFS error: Failed to read block 0x4001: -5
> > > [ 4960.910697] SQUASHFS error: Unable to read metadata cache entry [3fff]
> > > [ 4960.910698] SQUASHFS error: Unable to read inode 0x20c5000c
> > >
> > > I bisected the issue to the commit "squashfs: migrate from ll_rw_block
> > > usage to BIO"
> > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/fs/squashfs?id=93e72b3c612adcaca13d874fcc86c53e6c8da541
> > >
> > > The issue can be reproduced by downloading
> > > https://theworldsend.eu/demo.squashfs (20K) and the following command
> > > line.
> > > # mount demo.squashfs mnt && ls mnt && umount mnt
> > >
> > > The same squashfs can be mounted using Linux <=5.7.8.
> > > The kernel config is identical to the Arch Linux Kernel configuration,
> > > build using gcc 9 and 10 on x86_64.
> >
> > Thanks.  I queued a reversion patch.  I'll go ahead with this if we are
> > unable to get this fixed in the next week or so.
> >
>
> Yes, there is a bug in the patch.  I have tracked it down today, and I
> will send out a fix patch tomorrow.
>
> The bug is here:
>
> +   /* Extract the length of the metadata block */
> +   data = page_address(bvec->bv_page) + bvec->bv_offset;
> +   length = data[offset];
> +   if (offset <= bvec->bv_len - 1) {
>
> This check is wrong, it should be
>
> +   if (offset < bvec->bv_len - 1) {
>
>
> Phillip

Thanks, this change resolves the issue for me.

Bernd

>
> > Are you able to check that the below fixes things up?
> >
> > Thanks.
> >
> >
> > From: Andrew Morton 
> > Subject: revert "squashfs: migrate from ll_rw_block usage to BIO"
> >
> > Revert 93e72b3c612adc ("squashfs: migrate from ll_rw_block usage to BIO")
> > due to a regression reported by Bernd Amend.
> >
> > Link: 
> > http://lkml.kernel.org/r/caf31+h5zb7zn73obrc5svlzgfstnyye5tkvr7-6atuoqrry...@mail.gmail.com
> > Reported-by: Bernd Amend 
> > Cc: Philippe Liard 
> > Cc: Christoph Hellwig 
> > Cc: Adrien Schildknecht 
> > Cc: Phillip Lougher 
> > Cc: Guenter Roeck 
> > Cc: Daniel Rosenberg 
> > Signed-off-by: Andrew Morton 
> > ---
> >
> >  fs/squashfs/block.c |  273 ++
> >  fs/squashfs/decompressor.h  |5
> >  fs/squashfs/decompressor_multi.c|9
> >  fs/squashfs/decompressor_multi_percpu.c |6
> >  fs/squashfs/decompressor_single.c   |9
> >  fs/squashfs/lz4_wrapper.c   |   17 -
> >  fs/squashfs/lzo_wrapper.c   |   17 -
> >  fs/squashfs/squashfs.h  |4
> >  fs/squashfs/xz_wrapper.c|   51 +---
> >  fs/squashfs/zlib_wrapper.c  |   63 ++---
> >  fs/squashfs/zstd_wrapper.c  |   62 ++--
> >  11 files changed, 237 insertions(+), 279 deletions(-)
> >
> > --- 
> > a/fs/squashfs/block.c~revert-squashfs-migrate-from-ll_rw_block-usage-to-bio
> > +++ a/fs/squashfs/block.c
> > @@ -13,7 +13,6 @@
> >   * datablocks and metadata blocks.
> >   */
> >
> > -#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -28,104 +27,45 @@
> >  #include "page_actor.h"
> >
> >  /*
> > - * Returns the amount of bytes copied to 

Re: Regression: squashfs issues since change "squashfs: migrate from ll_rw_block usage to BIO"

2020-07-16 Thread Phillip Lougher
On Fri, Jul 17, 2020 at 12:07 AM Andrew Morton
 wrote:
>
> On Tue, 14 Jul 2020 21:41:07 +0200 Bernd Amend  wrote:
>
> > Hi,
> >
> > With the Linux Kernel version 5.8-rc5/master I am unable to mount some
> > squashfs filesystems compressed with "-comp lz4".
> > If I try to mount them I get the following error:
> > [1.084246] SQUASHFS error: lz4 decompression failed, data probably 
> > corrupt
> > [1.084545] SQUASHFS error: Failed to read block 0x873e1001: -5
> > [1.084761] SQUASHFS error: Unable to read metadata cache entry 
> > [873e0fff]
> > [1.084983] SQUASHFS error: Unable to read directory block 
> > [873e0fff:1586]
> > [1.122564] SQUASHFS error: Unable to read metadata cache entry 
> > [873e0fff]
> > [1.122708] SQUASHFS error: Unable to read directory block 
> > [873e0fff:1586]
> > [1.122862] Starting init: /sbin/init exists but couldn't execute
> > it (error -5)
> > [1.123027] SQUASHFS error: Unable to read metadata cache entry 
> > [873e0fff]
> > [1.123152] SQUASHFS error: Unable to read directory block 
> > [873e0fff:1586]
> > [1.123279] Starting init: /etc/init exists but couldn't execute it
> > (error -5)
> > [1.123444] SQUASHFS error: Unable to read metadata cache entry 
> > [873e0fff]
> > [1.123573] SQUASHFS error: Unable to read directory block 
> > [873e0fff:1586]
> > [1.123713] Starting init: /bin/init exists but couldn't execute it
> > (error -5)
> > [1.123900] SQUASHFS error: Unable to read metadata cache entry 
> > [873e0fff]
> >
> > or
> >
> > [ 4960.910693] attempt to access beyond end of device
> > [ 4960.910695] loop0: rw=2048, want=46, limit=40
> > [ 4960.910696] SQUASHFS error: Failed to read block 0x4001: -5
> > [ 4960.910697] SQUASHFS error: Unable to read metadata cache entry [3fff]
> > [ 4960.910698] SQUASHFS error: Unable to read inode 0x20c5000c
> >
> > I bisected the issue to the commit "squashfs: migrate from ll_rw_block
> > usage to BIO"
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/fs/squashfs?id=93e72b3c612adcaca13d874fcc86c53e6c8da541
> >
> > The issue can be reproduced by downloading
> > https://theworldsend.eu/demo.squashfs (20K) and the following command
> > line.
> > # mount demo.squashfs mnt && ls mnt && umount mnt
> >
> > The same squashfs can be mounted using Linux <=5.7.8.
> > The kernel config is identical to the Arch Linux Kernel configuration,
> > build using gcc 9 and 10 on x86_64.
>
> Thanks.  I queued a reversion patch.  I'll go ahead with this if we are
> unable to get this fixed in the next week or so.
>

Yes, there is a bug in the patch.  I have tracked it down today, and I
will send out a fix patch tomorrow.

The bug is here:

+   /* Extract the length of the metadata block */
+   data = page_address(bvec->bv_page) + bvec->bv_offset;
+   length = data[offset];
+   if (offset <= bvec->bv_len - 1) {

This check is wrong, it should be

+   if (offset < bvec->bv_len - 1) {


Phillip

> Are you able to check that the below fixes things up?
>
> Thanks.
>
>
> From: Andrew Morton 
> Subject: revert "squashfs: migrate from ll_rw_block usage to BIO"
>
> Revert 93e72b3c612adc ("squashfs: migrate from ll_rw_block usage to BIO")
> due to a regression reported by Bernd Amend.
>
> Link: 
> http://lkml.kernel.org/r/caf31+h5zb7zn73obrc5svlzgfstnyye5tkvr7-6atuoqrry...@mail.gmail.com
> Reported-by: Bernd Amend 
> Cc: Philippe Liard 
> Cc: Christoph Hellwig 
> Cc: Adrien Schildknecht 
> Cc: Phillip Lougher 
> Cc: Guenter Roeck 
> Cc: Daniel Rosenberg 
> Signed-off-by: Andrew Morton 
> ---
>
>  fs/squashfs/block.c |  273 ++
>  fs/squashfs/decompressor.h  |5
>  fs/squashfs/decompressor_multi.c|9
>  fs/squashfs/decompressor_multi_percpu.c |6
>  fs/squashfs/decompressor_single.c   |9
>  fs/squashfs/lz4_wrapper.c   |   17 -
>  fs/squashfs/lzo_wrapper.c   |   17 -
>  fs/squashfs/squashfs.h  |4
>  fs/squashfs/xz_wrapper.c|   51 +---
>  fs/squashfs/zlib_wrapper.c  |   63 ++---
>  fs/squashfs/zstd_wrapper.c  |   62 ++--
>  11 files changed, 237 insertions(+), 279 deletions(-)
>
> --- 
> a/fs/squashfs/block.c~revert-squashfs-migrate-from-ll_rw_block-usage-to-bio
> +++ a/fs/squashfs/block.c
> @@ -13,7 +13,6 @@
>   * datablocks and metadata blocks.
>   */
>
> -#include 
>  #include 
>  #include 
>  #include 
> @@ -28,104 +27,45 @@
>  #include "page_actor.h"
>
>  /*
> - * Returns the amount of bytes copied to the page actor.
> + * Read the metadata block length, this is stored in the first two
> + * bytes of the metadata block.
>   */
> -static int copy_bio_to_actor(struct bio *bio,
> -struct squashfs_page_actor *actor,
> -int offset, int req_length)
> -{
> -   void *actor_addr = squashfs_first_page(actor);
> -   

Re: Regression: squashfs issues since change "squashfs: migrate from ll_rw_block usage to BIO"

2020-07-16 Thread Andrew Morton
On Tue, 14 Jul 2020 21:41:07 +0200 Bernd Amend  wrote:

> Hi,
> 
> With the Linux Kernel version 5.8-rc5/master I am unable to mount some
> squashfs filesystems compressed with "-comp lz4".
> If I try to mount them I get the following error:
> [1.084246] SQUASHFS error: lz4 decompression failed, data probably corrupt
> [1.084545] SQUASHFS error: Failed to read block 0x873e1001: -5
> [1.084761] SQUASHFS error: Unable to read metadata cache entry [873e0fff]
> [1.084983] SQUASHFS error: Unable to read directory block [873e0fff:1586]
> [1.122564] SQUASHFS error: Unable to read metadata cache entry [873e0fff]
> [1.122708] SQUASHFS error: Unable to read directory block [873e0fff:1586]
> [1.122862] Starting init: /sbin/init exists but couldn't execute
> it (error -5)
> [1.123027] SQUASHFS error: Unable to read metadata cache entry [873e0fff]
> [1.123152] SQUASHFS error: Unable to read directory block [873e0fff:1586]
> [1.123279] Starting init: /etc/init exists but couldn't execute it
> (error -5)
> [1.123444] SQUASHFS error: Unable to read metadata cache entry [873e0fff]
> [1.123573] SQUASHFS error: Unable to read directory block [873e0fff:1586]
> [1.123713] Starting init: /bin/init exists but couldn't execute it
> (error -5)
> [1.123900] SQUASHFS error: Unable to read metadata cache entry [873e0fff]
> 
> or
> 
> [ 4960.910693] attempt to access beyond end of device
> [ 4960.910695] loop0: rw=2048, want=46, limit=40
> [ 4960.910696] SQUASHFS error: Failed to read block 0x4001: -5
> [ 4960.910697] SQUASHFS error: Unable to read metadata cache entry [3fff]
> [ 4960.910698] SQUASHFS error: Unable to read inode 0x20c5000c
> 
> I bisected the issue to the commit "squashfs: migrate from ll_rw_block
> usage to BIO"
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/fs/squashfs?id=93e72b3c612adcaca13d874fcc86c53e6c8da541
> 
> The issue can be reproduced by downloading
> https://theworldsend.eu/demo.squashfs (20K) and the following command
> line.
> # mount demo.squashfs mnt && ls mnt && umount mnt
> 
> The same squashfs can be mounted using Linux <=5.7.8.
> The kernel config is identical to the Arch Linux Kernel configuration,
> build using gcc 9 and 10 on x86_64.

Thanks.  I queued a reversion patch.  I'll go ahead with this if we are
unable to get this fixed in the next week or so.

Are you able to check that the below fixes things up?

Thanks.


From: Andrew Morton 
Subject: revert "squashfs: migrate from ll_rw_block usage to BIO"

Revert 93e72b3c612adc ("squashfs: migrate from ll_rw_block usage to BIO")
due to a regression reported by Bernd Amend.

Link: 
http://lkml.kernel.org/r/caf31+h5zb7zn73obrc5svlzgfstnyye5tkvr7-6atuoqrry...@mail.gmail.com
Reported-by: Bernd Amend 
Cc: Philippe Liard 
Cc: Christoph Hellwig 
Cc: Adrien Schildknecht 
Cc: Phillip Lougher 
Cc: Guenter Roeck 
Cc: Daniel Rosenberg 
Signed-off-by: Andrew Morton 
---

 fs/squashfs/block.c |  273 ++
 fs/squashfs/decompressor.h  |5 
 fs/squashfs/decompressor_multi.c|9 
 fs/squashfs/decompressor_multi_percpu.c |6 
 fs/squashfs/decompressor_single.c   |9 
 fs/squashfs/lz4_wrapper.c   |   17 -
 fs/squashfs/lzo_wrapper.c   |   17 -
 fs/squashfs/squashfs.h  |4 
 fs/squashfs/xz_wrapper.c|   51 +---
 fs/squashfs/zlib_wrapper.c  |   63 ++---
 fs/squashfs/zstd_wrapper.c  |   62 ++--
 11 files changed, 237 insertions(+), 279 deletions(-)

--- a/fs/squashfs/block.c~revert-squashfs-migrate-from-ll_rw_block-usage-to-bio
+++ a/fs/squashfs/block.c
@@ -13,7 +13,6 @@
  * datablocks and metadata blocks.
  */
 
-#include 
 #include 
 #include 
 #include 
@@ -28,104 +27,45 @@
 #include "page_actor.h"
 
 /*
- * Returns the amount of bytes copied to the page actor.
+ * Read the metadata block length, this is stored in the first two
+ * bytes of the metadata block.
  */
-static int copy_bio_to_actor(struct bio *bio,
-struct squashfs_page_actor *actor,
-int offset, int req_length)
-{
-   void *actor_addr = squashfs_first_page(actor);
-   struct bvec_iter_all iter_all = {};
-   struct bio_vec *bvec = bvec_init_iter_all(_all);
-   int copied_bytes = 0;
-   int actor_offset = 0;
-
-   if (WARN_ON_ONCE(!bio_next_segment(bio, _all)))
-   return 0;
-
-   while (copied_bytes < req_length) {
-   int bytes_to_copy = min_t(int, bvec->bv_len - offset,
- PAGE_SIZE - actor_offset);
-
-   bytes_to_copy = min_t(int, bytes_to_copy,
- req_length - copied_bytes);
-   memcpy(actor_addr + actor_offset,
-  page_address(bvec->bv_page) + bvec->bv_offset + offset,
-  bytes_to_copy);
-
-   actor_offset += 

Regression: squashfs issues since change "squashfs: migrate from ll_rw_block usage to BIO"

2020-07-14 Thread Bernd Amend
Hi,

With the Linux Kernel version 5.8-rc5/master I am unable to mount some
squashfs filesystems compressed with "-comp lz4".
If I try to mount them I get the following error:
[1.084246] SQUASHFS error: lz4 decompression failed, data probably corrupt
[1.084545] SQUASHFS error: Failed to read block 0x873e1001: -5
[1.084761] SQUASHFS error: Unable to read metadata cache entry [873e0fff]
[1.084983] SQUASHFS error: Unable to read directory block [873e0fff:1586]
[1.122564] SQUASHFS error: Unable to read metadata cache entry [873e0fff]
[1.122708] SQUASHFS error: Unable to read directory block [873e0fff:1586]
[1.122862] Starting init: /sbin/init exists but couldn't execute
it (error -5)
[1.123027] SQUASHFS error: Unable to read metadata cache entry [873e0fff]
[1.123152] SQUASHFS error: Unable to read directory block [873e0fff:1586]
[1.123279] Starting init: /etc/init exists but couldn't execute it
(error -5)
[1.123444] SQUASHFS error: Unable to read metadata cache entry [873e0fff]
[1.123573] SQUASHFS error: Unable to read directory block [873e0fff:1586]
[1.123713] Starting init: /bin/init exists but couldn't execute it
(error -5)
[1.123900] SQUASHFS error: Unable to read metadata cache entry [873e0fff]

or

[ 4960.910693] attempt to access beyond end of device
[ 4960.910695] loop0: rw=2048, want=46, limit=40
[ 4960.910696] SQUASHFS error: Failed to read block 0x4001: -5
[ 4960.910697] SQUASHFS error: Unable to read metadata cache entry [3fff]
[ 4960.910698] SQUASHFS error: Unable to read inode 0x20c5000c

I bisected the issue to the commit "squashfs: migrate from ll_rw_block
usage to BIO"
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/fs/squashfs?id=93e72b3c612adcaca13d874fcc86c53e6c8da541

The issue can be reproduced by downloading
https://theworldsend.eu/demo.squashfs (20K) and the following command
line.
# mount demo.squashfs mnt && ls mnt && umount mnt

The same squashfs can be mounted using Linux <=5.7.8.
The kernel config is identical to the Arch Linux Kernel configuration,
build using gcc 9 and 10 on x86_64.

Best regards,
Bernd Amend