Linux-EROFS Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2 1/3] erofs: correct the remaining shrink objects
@ 2020-02-26  8:10 Gao Xiang
  2020-02-26  8:10 ` [PATCH v2 2/3] erofs: use LZ4_decompress_safe() for full decoding Gao Xiang
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Gao Xiang @ 2020-02-26  8:10 UTC (permalink / raw)
  To: Chao Yu, linux-erofs; +Cc: Miao Xie, LKML, stable

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


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

* [PATCH v2 2/3] erofs: use LZ4_decompress_safe() for full decoding
  2020-02-26  8:10 [PATCH v2 1/3] erofs: correct the remaining shrink objects Gao Xiang
@ 2020-02-26  8:10 ` Gao Xiang
  2020-02-26  8:10 ` [PATCH v2 3/3] erofs: handle corrupted images whose decompressed size less than it'd be Gao Xiang
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Gao Xiang @ 2020-02-26  8:10 UTC (permalink / raw)
  To: Chao Yu, linux-erofs; +Cc: Miao Xie, LKML, Lasse Collin

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


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

* [PATCH v2 3/3] erofs: handle corrupted images whose decompressed size less than it'd be
  2020-02-26  8:10 [PATCH v2 1/3] erofs: correct the remaining shrink objects Gao Xiang
  2020-02-26  8:10 ` [PATCH v2 2/3] erofs: use LZ4_decompress_safe() for full decoding Gao Xiang
@ 2020-02-26  8:10 ` Gao Xiang
  2020-02-28 19:44 ` [PATCH v2 1/3] erofs: correct the remaining shrink objects Sasha Levin
  2020-03-03  9:33 ` Chao Yu
  3 siblings, 0 replies; 6+ messages in thread
From: Gao Xiang @ 2020-02-26  8:10 UTC (permalink / raw)
  To: Chao Yu, linux-erofs; +Cc: Miao Xie, LKML, Lasse Collin

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


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

* Re: [PATCH v2 1/3] erofs: correct the remaining shrink objects
  2020-02-26  8:10 [PATCH v2 1/3] erofs: correct the remaining shrink objects Gao Xiang
  2020-02-26  8:10 ` [PATCH v2 2/3] erofs: use LZ4_decompress_safe() for full decoding Gao Xiang
  2020-02-26  8:10 ` [PATCH v2 3/3] erofs: handle corrupted images whose decompressed size less than it'd be Gao Xiang
@ 2020-02-28 19:44 ` Sasha Levin
  2020-02-29  1:11   ` Gao Xiang
  2020-03-03  9:33 ` Chao Yu
  3 siblings, 1 reply; 6+ messages in thread
From: Sasha Levin @ 2020-02-28 19:44 UTC (permalink / raw)
  To: Sasha Levin, Gao Xiang, Chao Yu, linux-erofs; +Cc: Miao Xie, LKML, stable

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

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

* Re: [PATCH v2 1/3] erofs: correct the remaining shrink objects
  2020-02-28 19:44 ` [PATCH v2 1/3] erofs: correct the remaining shrink objects Sasha Levin
@ 2020-02-29  1:11   ` Gao Xiang
  0 siblings, 0 replies; 6+ messages in thread
From: Gao Xiang @ 2020-02-29  1:11 UTC (permalink / raw)
  To: Sasha Levin; +Cc: Miao Xie, linux-erofs, LKML, stable

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

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

* Re: [PATCH v2 1/3] erofs: correct the remaining shrink objects
  2020-02-26  8:10 [PATCH v2 1/3] erofs: correct the remaining shrink objects Gao Xiang
                   ` (2 preceding siblings ...)
  2020-02-28 19:44 ` [PATCH v2 1/3] erofs: correct the remaining shrink objects Sasha Levin
@ 2020-03-03  9:33 ` Chao Yu
  3 siblings, 0 replies; 6+ messages in thread
From: Chao Yu @ 2020-03-03  9:33 UTC (permalink / raw)
  To: Gao Xiang, linux-erofs; +Cc: Miao Xie, LKML, stable

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,

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

end of thread, back to index

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-26  8:10 [PATCH v2 1/3] erofs: correct the remaining shrink objects Gao Xiang
2020-02-26  8:10 ` [PATCH v2 2/3] erofs: use LZ4_decompress_safe() for full decoding Gao Xiang
2020-02-26  8:10 ` [PATCH v2 3/3] erofs: handle corrupted images whose decompressed size less than it'd be Gao Xiang
2020-02-28 19:44 ` [PATCH v2 1/3] erofs: correct the remaining shrink objects Sasha Levin
2020-02-29  1:11   ` Gao Xiang
2020-03-03  9:33 ` Chao Yu

Linux-EROFS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-erofs/0 linux-erofs/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-erofs linux-erofs/ https://lore.kernel.org/linux-erofs \
		linux-erofs@lists.ozlabs.org linux-erofs@ozlabs.org
	public-inbox-index linux-erofs

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.ozlabs.lists.linux-erofs


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git