| Message ID | 20200821162948.146947-1-paul@crapouillou.net (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/2] lib: decompress_unzstd: Limit output size | expand |
> 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. 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. -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 >
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 >> >
> On Aug 24, 2020, at 2:05 PM, Paul Cercueil <paul@crapouillou.net> wrote: > > > > 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. Strange… I don’t quite know what is going on then. Thanks for the fix! You can add: Reviewed-By: Nick Terrell <terrelln@fb.com> Best, Nick >> 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 > >
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) /*
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. Signed-off-by: Paul Cercueil <paul@crapouillou.net> --- lib/decompress_unzstd.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)