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

[Paul Cercueil]

Le lun. 24 août 2020 à 20:11, Nick Terrell <terrelln@xxxxxx> a écrit 
:
> >  On Aug 21, 2020, at 9:29 AM, Paul Cercueil <paul@xxxxxxxxxxxxxxx> 
> > 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@xxxxxxxxxxxxxxx>
> >  ---
> >  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