Re: [PATCH 1/2] lib: decompress_unzstd: Limit output size

[Paul Cercueil <paul@crapouillou.net>]

Le lun. 24 août 2020 à 20:11, Nick Terrell <terrelln@fb.com> a écrit 
:
> 
> 
>>  On Aug 21, 2020, at 9:29 AM, Paul Cercueil <paul@crapouillou.net> 
>> wrote:
>> 
>>  The zstd decompression code, as it is right now, will have internal
>>  values overflow on 32-bit systems when the output size is LONG_MAX.
>> 
>>  Until someone smarter than me can figure out how to fix the zstd 
>> code
>>  properly, limit the destination buffer size to 512 MiB, which 
>> should be
>>  enough for everybody, in order to make it usable on 32-bit systems.
> 
> Can you bump the size up to 2GB? I suspect the problem inside of zstd
> is an off-by-one error or something similar, so getting closer to the 
> limit
> shouldn't be a problem. I’d feel more comfortable with 2GB, since
> kernels can get pretty large.

SZ_1G is the biggest I can go to get the kernel to boot. With SZ_2G it 
won't boot.

> Hmm, zstd shouldn’t be overflowing that value. I’m currently 
> preparing
> a patch to updating the version of zstd in the kernel, and using 
> upstream
> directly. I will add a test upstream in 32-bit mode to ensure that we 
> don’t
> overflow a 32-bit size_t, so this will be fixed after the update.

Great, thanks.

Cheers,
-Paul

> 
> -Nick
> 
>>  Signed-off-by: Paul Cercueil <paul@crapouillou.net>
>>  ---
>>  lib/decompress_unzstd.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>> 
>>  diff --git a/lib/decompress_unzstd.c b/lib/decompress_unzstd.c
>>  index 0ad2c15479ed..e1c03b1eaa6e 100644
>>  --- a/lib/decompress_unzstd.c
>>  +++ b/lib/decompress_unzstd.c
>>  @@ -77,6 +77,7 @@
>> 
>>  #include <linux/decompress/mm.h>
>>  #include <linux/kernel.h>
>>  +#include <linux/sizes.h>
>>  #include <linux/zstd.h>
>> 
>>  /* 128MB is the maximum window size supported by zstd. */
>>  @@ -179,7 +180,7 @@ static int INIT __unzstd(unsigned char *in_buf, 
>> long in_len,
>>  	size_t ret;
>> 
>>  	if (out_len == 0)
>>  -		out_len = LONG_MAX; /* no limit */
>>  +		out_len = SZ_512M; /* should be big enough, right? */
>> 
>>  	if (fill == NULL && flush == NULL)
>>  		/*
>>  --
>>  2.28.0
>> 
>