linux-kernel - Re: [PATCH 1/2] lib: decompress

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <D0112FE3-394D-44DB-9D5C-60A789A67A4D@fb.com>
Date:   Mon, 24 Aug 2020 21:50:37 +0000
From:   Nick Terrell <terrelln@...com>
To:     Paul Cercueil <paul@...pouillou.net>
CC:     Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
        "linux-mips@...r.kernel.org" <linux-mips@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "od@...c.me" <od@...c.me>
Subject: Re: [PATCH 1/2] lib: decompress_unzstd: Limit output size



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