Re: [PATCH v2 01/15] lib/decompress_unlz4: use LZ4_decompress_safe_partial() before boot

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential
issues to consider:
- [Medium] lib/decompress_unlz4: unvalidated chunksize passed to decoder
  in unlz4()
- [Low] lib/decompress_unlz4: partial decode skips full-block checks in
  unlz4()
- [Low] lib/decompress_unlz4: unlz4() does not check out_len == 0 at
  stream end
--

commit 8a11d492456e8edabb6dad4a7ecb828134c99572
Author: Michal Wilczynski <m.wilczynski@samsung.com>

lib/decompress_unlz4: use LZ4_decompress_safe_partial() before boot

This commit replaces LZ4_decompress_fast() with
LZ4_decompress_safe_partial() in the pre-boot unlz4() path and checks that
each chunk produces the expected output size.

> diff --git a/lib/decompress_unlz4.c b/lib/decompress_unlz4.c
> index c0dbb3cea915e..dc20c22e08790 100644
> --- a/lib/decompress_unlz4.c
> +++ b/lib/decompress_unlz4.c
> @@ -151,14 +151,17 @@ STATIC inline int INIT unlz4(u8 *input, long in_len,
>  			}
>  		}
>  #ifdef PREBOOT
> -		if (out_len >= uncomp_chunksize) {
> +		if (out_len >= uncomp_chunksize)
>  			dest_len = uncomp_chunksize;
> -			out_len -= dest_len;
> -		} else
> +		else
>  			dest_len = out_len;
> +		out_len -= dest_len;
>  
> -		ret = LZ4_decompress_fast(inp, outp, dest_len);
> -		chunksize = ret;
> +		ret = LZ4_decompress_safe_partial(inp, outp, chunksize,
> +						  dest_len, dest_len);

[Severity: Medium]
Can a truncated or corrupted chunk header cause LZ4_decompress_safe_partial()
to read past the end of the input buffer?

Previously, LZ4_decompress_fast() ignored chunksize and overwrote it with the
consumed byte count before the !fill path checked size -= chunksize at the
bottom of the loop:

lib/decompress_unlz4.c:unlz4() {
	...
		if (!fill) {
			inp += 4;
			size -= 4;
		} else {
	...
		if (!fill) {
			size -= chunksize;

			if (size == 0)
				break;
			else if (size < 0) {
				error("data corrupted");
				goto exit_2;
			}
			inp += chunksize;
		}
}

Passing chunksize from get_unaligned_le32(inp) directly to
LZ4_decompress_safe_partial() before checking it against size lets the
decoder read past input + in_len when chunksize > size, or pass a negative
compressedSize when chunksize >= 0x80000000.

> +		/* A chunk once the image's stated size is reached is corrupt. */
> +		if (!dest_len || ret != (int)dest_len)
> +			ret = -1;

[Severity: Low]
Should this call LZ4_decompress_safe(inp, outp, chunksize, dest_len) instead
of LZ4_decompress_safe_partial()?

In LZ4_decompress_generic(), partial_decode mode clamps literal runs and
matches that would exceed dest_len and stops as soon as dest_len bytes are
written without verifying that all chunksize input bytes were consumed.  A
corrupted chunk that expands beyond dest_len or leaves trailing bytes inside
chunksize will still return dest_len and pass this check.

[Severity: Low]
Can a truncated image that ends on a chunk boundary still return success when
out_len has not reached zero?

This check rejects chunks that decode short or arrive after out_len reaches
zero, but if the input stream ends early between chunks (or has chunksize ==
0), the loop jumps to exit_3 and returns 0 with out_len > 0.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106-lz4-vendor-upstream-v2-0-ee04c3f0d7dc@samsung.com?part=1