[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <19E42116-8FE3-4C4B-8D26-E9B47B0B9AC5@meta.com>
Date: Thu, 12 Oct 2023 19:55:55 +0000
From: Nick Terrell <terrelln@...a.com>
To: Kees Cook <keescook@...omium.org>
CC: Nick Terrell <terrelln@...a.com>, Eric Biggers <ebiggers@...nel.org>,
Nick
Terrell <terrelln@...a.com>,
syzbot
<syzbot+1f2eb3e8cd123ffce499@...kaller.appspotmail.com>,
Chris Mason
<clm@...a.com>, "dsterba@...e.com" <dsterba@...e.com>,
"josef@...icpanda.com"
<josef@...icpanda.com>,
"linux-btrfs@...r.kernel.org"
<linux-btrfs@...r.kernel.org>,
"linux-fsdevel@...r.kernel.org"
<linux-fsdevel@...r.kernel.org>,
"linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>,
"syzkaller-bugs@...glegroups.com"
<syzkaller-bugs@...glegroups.com>,
"linux-hardening@...r.kernel.org"
<linux-hardening@...r.kernel.org>
Subject: Re: [syzbot] [zstd] UBSAN: array-index-out-of-bounds in
FSE_decompress_wksp_body_bmi2
> On Oct 9, 2023, at 1:29 PM, Kees Cook <keescook@...omium.org> wrote:
>
> !-------------------------------------------------------------------|
> This Message Is From an External Sender
>
> |-------------------------------------------------------------------!
>
> On Sat, Oct 07, 2023 at 02:05:56PM -0700, Eric Biggers wrote:
>> Hi Nick,
>>
>> On Wed, Aug 30, 2023 at 12:49:53AM -0700, syzbot wrote:
>>> UBSAN: array-index-out-of-bounds in lib/zstd/common/fse_decompress.c:345:30
>>> index 33 is out of range for type 'FSE_DTable[1]' (aka 'unsigned int[1]')
>>
>> Zstandard needs to be converted to use C99 flex-arrays instead of length-1
>> arrays. https://github.com/facebook/zstd/pull/3785 would fix this in upstream
>> Zstandard, though it doesn't work well with the fact that upstream Zstandard
>> supports C90. Not sure how you want to handle this.
>
> For the kernel, we just need:
>
> diff --git a/lib/zstd/common/fse_decompress.c b/lib/zstd/common/fse_decompress.c
> index a0d06095be83..b11e87fff261 100644
> --- a/lib/zstd/common/fse_decompress.c
> +++ b/lib/zstd/common/fse_decompress.c
> @@ -312,7 +312,7 @@ size_t FSE_decompress_wksp(void* dst, size_t dstCapacity, const void* cSrc, size
>
> typedef struct {
> short ncount[FSE_MAX_SYMBOL_VALUE + 1];
> - FSE_DTable dtable[1]; /* Dynamically sized */
> + FSE_DTable dtable[]; /* Dynamically sized */
> } FSE_DecompressWksp;
Thanks Eric and Kees for the report and the fix! I am working on putting this
patch up now, just need to test the fix myself to ensure I can reproduce the
issue and the fix.
In your opinion does this worth trying to get this patch into v6.6, or should it
wait for v6.7?
Best,
Nick Terrell
> And if upstream wants to stay C89 compat, perhaps:
>
> #if __STDC_VERSION__ >= 199901L
> # define __FLEX_ARRAY_DIM /*C99*/
> #else
> # define __FLEX_ARRAY_DIM 0
> #endif
>
> and then use __FLEX_ARRAY_DIM as needed (and keep the other "-1" changes
> in the github commit):
>
> typedef struct {
> short ncount[FSE_MAX_SYMBOL_VALUE + 1];
> - FSE_DTable dtable[1]; /* Dynamically sized */
> + FSE_DTable dtable[__FLEX_ARRAY_DIM]; /* Dynamically sized */
> } FSE_DecompressWksp;
>
>
> --
> Kees Cook
Powered by blists - more mailing lists