Re: [PATCH AUTOSEL 5.4 09/10] s390/decompressor: specify __decompress() buf len to avoid overflow

From: Nick Terrell
Date: Wed Feb 15 2023 - 13:57:33 EST




> On Feb 9, 2023, at 3:19 AM, Sasha Levin <sashal@xxxxxxxxxx> wrote:
>
> !-------------------------------------------------------------------|
> This Message Is From an External Sender
>
> |-------------------------------------------------------------------!
>
> From: Vasily Gorbik <gor@xxxxxxxxxxxxx>
>
> [ Upstream commit 7ab41c2c08a32132ba8c14624910e2fe8ce4ba4b ]
>
> Historically calls to __decompress() didn't specify "out_len" parameter
> on many architectures including s390, expecting that no writes beyond
> uncompressed kernel image are performed. This has changed since commit
> 2aa14b1ab2c4 ("zstd: import usptream v1.5.2") which includes zstd library
> commit 6a7ede3dfccb ("Reduce size of dctx by reutilizing dst buffer
> (#2751)"). Now zstd decompression code might store literal buffer in
> the unwritten portion of the destination buffer. Since "out_len" is
> not set, it is considered to be unlimited and hence free to use for
> optimization needs. On s390 this might corrupt initrd or ipl report
> which are often placed right after the decompressor buffer. Luckily the
> size of uncompressed kernel image is already known to the decompressor,
> so to avoid the problem simply specify it in the "out_len" parameter.

Thanks for the CC!

Reviewed-by: Nick Terrell <terrelln@fb. <mailto:terrelln@xxxxxx>com>

It looks like s390 doesn't use in-place decompression, but x86 does,
and we'll need to backport upstream commit 5b266196a [0] to make
sure we don't overwrite the input buffer.

Best,
Nick Terrell

[0] https://github.com/facebook/zstd/commit/5b266196a41e6a15e21bd4f0eeab43b938db1d90

> Link: https://github.com/facebook/zstd/commit/6a7ede3dfccb
> Signed-off-by: Vasily Gorbik <gor@xxxxxxxxxxxxx>
> Tested-by: Alexander Egorenkov <egorenar@xxxxxxxxxxxxx>
> Link: https://lore.kernel.org/r/patch-1.thread-41c676.git-41c676c2d153.your-ad-here.call-01675030179-ext-9637@work.hours
> Signed-off-by: Heiko Carstens <hca@xxxxxxxxxxxxx>
> Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
> ---
> arch/s390/boot/compressed/decompressor.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/s390/boot/compressed/decompressor.c b/arch/s390/boot/compressed/decompressor.c
> index 45046630c56ac..c42ab33bd4524 100644
> --- a/arch/s390/boot/compressed/decompressor.c
> +++ b/arch/s390/boot/compressed/decompressor.c
> @@ -80,6 +80,6 @@ void *decompress_kernel(void)
> void *output = (void *)decompress_offset;
>
> __decompress(_compressed_start, _compressed_end - _compressed_start,
> - NULL, NULL, output, 0, NULL, error);
> + NULL, NULL, output, vmlinux.image_size, NULL, error);
> return output;
> }
> --
> 2.39.0
>