[PATCH v2 1/3] erofs: correct the remaining shrink objects

[PATCH v2 1/3] erofs: correct the remaining shrink objects

[Gao Xiang]

The remaining count should not include successful
shrink attempts.

Fixes: e7e9a307be9d ("staging: erofs: introduce workstation for decompression")
Cc: <stable@vger.kernel.org> # 4.19+
Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
---

Changes since v1:
 - Add "Fixes:" tags respectively suggested by Eric. I'd suggest
   no rush to backport PATCH 2/3 and 3/3 since it's not quite
   sure whether they behave well for normal images for now and
   I will backport them manually later since they have no impact
   on system stability with corrupted images;

 - Fix PATCH 2/3 to exclude legacy (no decompression inplace
   support, < v5.3) images.

 fs/erofs/utils.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/erofs/utils.c b/fs/erofs/utils.c
index fddc5059c930..df42ea552a44 100644
--- a/fs/erofs/utils.c
+++ b/fs/erofs/utils.c
@@ -286,7 +286,7 @@ static unsigned long erofs_shrink_scan(struct shrinker *shrink,
 		spin_unlock(&erofs_sb_list_lock);
 		sbi->shrinker_run_no = run_no;
 
-		freed += erofs_shrink_workstation(sbi, nr);
+		freed += erofs_shrink_workstation(sbi, nr - freed);
 
 		spin_lock(&erofs_sb_list_lock);
 		/* Get the next list element before we move this one */
-- 
2.17.1

[PATCH v2 2/3] erofs: use LZ4_decompress_safe() for full decoding

[Gao Xiang]

As Lasse pointed out, "EROFS uses LZ4_decompress_safe_partial
for both partial and full blocks. Thus when it is decoding a
full block, it doesn't know if the LZ4 decoder actually decoded
all the input. The real uncompressed size could be bigger than
the value stored in the file system metadata.

Using LZ4_decompress_safe instead of _safe_partial when
decompressing a full block would help to detect errors."

So it's reasonable to use _safe in case of potential corrupted
images and it might have some speed gain as well although
I didn't observe much difference.

Note that legacy compressor (< 5.3, no LZ4_0PADDING) could
encode extra data in a pcluster, which is excluded as well.

Cc: Lasse Collin <lasse.collin@tukaani.org>
Fixes: 0ffd71bcc3a0 ("staging: erofs: introduce LZ4 decompression inplace")
[ Gao Xiang: v5.3+, I will manually backport this to stable later. ]
Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
---
 fs/erofs/decompressor.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/fs/erofs/decompressor.c b/fs/erofs/decompressor.c
index 5779a15c2cd6..cd978af6bdb9 100644
--- a/fs/erofs/decompressor.c
+++ b/fs/erofs/decompressor.c
@@ -157,9 +157,15 @@ static int z_erofs_lz4_decompress(struct z_erofs_decompress_req *rq, u8 *out)
 		}
 	}
 
-	ret = LZ4_decompress_safe_partial(src + inputmargin, out,
-					  inlen, rq->outputsize,
-					  rq->outputsize);
+	/* legacy format could compress extra data in a pcluster. */
+	if (rq->partial_decoding || !support_0padding)
+		ret = LZ4_decompress_safe_partial(src + inputmargin, out,
+						  inlen, rq->outputsize,
+						  rq->outputsize);
+	else
+		ret = LZ4_decompress_safe(src + inputmargin, out,
+					  inlen, rq->outputsize);
+
 	if (ret < 0) {
 		erofs_err(rq->sb, "failed to decompress, in[%u, %u] out[%u]",
 			  inlen, inputmargin, rq->outputsize);
-- 
2.17.1

[PATCH v2 3/3] erofs: handle corrupted images whose decompressed size less than it'd be

[Gao Xiang]

As Lasse pointed out, "Looking at fs/erofs/decompress.c,
the return value from LZ4_decompress_safe_partial is only
checked for negative value to catch errors. ... So if
I understood it correctly, if there is bad data whose
uncompressed size is much less than it should be, it can
leave part of the output buffer untouched and expose the
previous data as the file content. "

Let's fix it now.

Cc: Lasse Collin <lasse.collin@tukaani.org>
Fixes: 7fc45dbc938a ("staging: erofs: introduce generic decompression backend")
[ Gao Xiang: v5.3+, I will manually backport this to stable later. ]
Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
---
 fs/erofs/decompressor.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/fs/erofs/decompressor.c b/fs/erofs/decompressor.c
index cd978af6bdb9..5d2d81940679 100644
--- a/fs/erofs/decompressor.c
+++ b/fs/erofs/decompressor.c
@@ -166,14 +166,18 @@ static int z_erofs_lz4_decompress(struct z_erofs_decompress_req *rq, u8 *out)
 		ret = LZ4_decompress_safe(src + inputmargin, out,
 					  inlen, rq->outputsize);
 
-	if (ret < 0) {
-		erofs_err(rq->sb, "failed to decompress, in[%u, %u] out[%u]",
-			  inlen, inputmargin, rq->outputsize);
+	if (ret != rq->outputsize) {
+		erofs_err(rq->sb, "failed to decompress %d in[%u, %u] out[%u]",
+			  ret, inlen, inputmargin, rq->outputsize);
+
 		WARN_ON(1);
 		print_hex_dump(KERN_DEBUG, "[ in]: ", DUMP_PREFIX_OFFSET,
 			       16, 1, src + inputmargin, inlen, true);
 		print_hex_dump(KERN_DEBUG, "[out]: ", DUMP_PREFIX_OFFSET,
 			       16, 1, out, rq->outputsize, true);
+
+		if (ret >= 0)
+			memset(out + ret, 0, rq->outputsize - ret);
 		ret = -EIO;
 	}
 
-- 
2.17.1

Re: [PATCH v2 1/3] erofs: correct the remaining shrink objects

[Sasha Levin]

Hi

[This is an automated email]

This commit has been processed because it contains a "Fixes:" tag
fixing commit: e7e9a307be9d ("staging: erofs: introduce workstation for decompression").

The bot has tested the following trees: v5.5.6, v5.4.22, v4.19.106.

v5.5.6: Build OK!
v5.4.22: Failed to apply! Possible dependencies:
    bda17a4577da ("erofs: remove dead code since managed cache is now built-in")

v4.19.106: Failed to apply! Possible dependencies:
    05f9d4a0c8c4 ("staging: erofs: use the new LZ4_decompress_safe_partial()")
    0a64d62d5399 ("staging: erofs: fixed -Wmissing-prototype warnings by making functions static.")
    14f362b4f405 ("staging: erofs: clean up internal.h")
    152a333a5895 ("staging: erofs: add compacted compression indexes support")
    22fe04a77d10 ("staging: erofs: clean up shrinker stuffs")
    3b423417d0d1 ("staging: erofs: clean up erofs_map_blocks_iter")
    5fb76bb04216 ("staging: erofs: cleanup `z_erofs_vle_normalaccess_readpages'")
    6e78901a9f23 ("staging: erofs: separate erofs_get_meta_page")
    7dd68b147d60 ("staging: erofs: use explicit unsigned int type")
    7fc45dbc938a ("staging: erofs: introduce generic decompression backend")
    89fcd8360e7b ("staging: erofs: change 'unsigned' to 'unsigned int'")
    8be31270362b ("staging: erofs: introduce erofs_grab_bio")
    ab47dd2b0819 ("staging: erofs: cleanup z_erofs_vle_work_{lookup, register}")
    bda17a4577da ("erofs: remove dead code since managed cache is now built-in")
    d1ab82443bed ("staging: erofs: Modify conditional checks")
    e7dfb1cff65b ("staging: erofs: fixed -Wmissing-prototype warnings by moving prototypes to header file.")
    f0950b02a74c ("staging: erofs: Modify coding style alignments")


NOTE: The patch will not be queued to stable trees until it is upstream.

How should we proceed with this patch?

-- 
Thanks
Sasha

Re: [PATCH v2 1/3] erofs: correct the remaining shrink objects

[Gao Xiang]

Hi,

On Fri, Feb 28, 2020 at 07:44:50PM +0000, Sasha Levin wrote:
> Hi
> 
> [This is an automated email]
> 
> This commit has been processed because it contains a "Fixes:" tag
> fixing commit: e7e9a307be9d ("staging: erofs: introduce workstation for decompression").
> 
> The bot has tested the following trees: v5.5.6, v5.4.22, v4.19.106.
> 
> v5.5.6: Build OK!
> v5.4.22: Failed to apply! Possible dependencies:
>     bda17a4577da ("erofs: remove dead code since managed cache is now built-in")
> 
> v4.19.106: Failed to apply! Possible dependencies:
>     05f9d4a0c8c4 ("staging: erofs: use the new LZ4_decompress_safe_partial()")
>     0a64d62d5399 ("staging: erofs: fixed -Wmissing-prototype warnings by making functions static.")
>     14f362b4f405 ("staging: erofs: clean up internal.h")
>     152a333a5895 ("staging: erofs: add compacted compression indexes support")
>     22fe04a77d10 ("staging: erofs: clean up shrinker stuffs")
>     3b423417d0d1 ("staging: erofs: clean up erofs_map_blocks_iter")
>     5fb76bb04216 ("staging: erofs: cleanup `z_erofs_vle_normalaccess_readpages'")
>     6e78901a9f23 ("staging: erofs: separate erofs_get_meta_page")
>     7dd68b147d60 ("staging: erofs: use explicit unsigned int type")
>     7fc45dbc938a ("staging: erofs: introduce generic decompression backend")
>     89fcd8360e7b ("staging: erofs: change 'unsigned' to 'unsigned int'")
>     8be31270362b ("staging: erofs: introduce erofs_grab_bio")
>     ab47dd2b0819 ("staging: erofs: cleanup z_erofs_vle_work_{lookup, register}")
>     bda17a4577da ("erofs: remove dead code since managed cache is now built-in")
>     d1ab82443bed ("staging: erofs: Modify conditional checks")
>     e7dfb1cff65b ("staging: erofs: fixed -Wmissing-prototype warnings by moving prototypes to header file.")
>     f0950b02a74c ("staging: erofs: Modify coding style alignments")

I will manually backport this if it can not be automatically applied.

Thanks,
Gao Xiang

> 
> 
> NOTE: The patch will not be queued to stable trees until it is upstream.
> 
> How should we proceed with this patch?
> 
> -- 
> Thanks
> Sasha

Re: [PATCH v2 1/3] erofs: correct the remaining shrink objects

[Chao Yu]

On 2020/2/26 16:10, Gao Xiang wrote:
> The remaining count should not include successful
> shrink attempts.
> 
> Fixes: e7e9a307be9d ("staging: erofs: introduce workstation for decompression")
> Cc: <stable@vger.kernel.org> # 4.19+
> Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>

Reviewed-by: Chao Yu <yuchao0@huawei.com>

Thanks,