Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.36 as permitted sender) client-ip=23.128.96.36;
Date:   Thu, 12 Oct 2023 13:13:19 -0700
From:   Kees Cook <keescook@chromium.org>
To:     Nick Terrell <terrelln@meta.com>
Cc:     Eric Biggers <ebiggers@kernel.org>,
        syzbot <syzbot+1f2eb3e8cd123ffce499@syzkaller.appspotmail.com>,
        Chris Mason <clm@meta.com>,
        "dsterba@suse.com" <dsterba@suse.com>,
        "josef@toxicpanda.com" <josef@toxicpanda.com>,
        "linux-btrfs@vger.kernel.org" <linux-btrfs@vger.kernel.org>,
        "linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "syzkaller-bugs@googlegroups.com" <syzkaller-bugs@googlegroups.com>,
        "linux-hardening@vger.kernel.org" <linux-hardening@vger.kernel.org>
Subject: Re: [syzbot] [zstd] UBSAN: array-index-out-of-bounds in
 FSE_decompress_wksp_body_bmi2
Message-ID: <202310121311.4B9DD96E51@keescook>
References: <00000000000049964e06041f2cbf@google.com>
 <20231007210556.GA174883@sol.localdomain>
 <202310091025.4939AEBC9@keescook>
 <19E42116-8FE3-4C4B-8D26-E9B47B0B9AC5@meta.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <19E42116-8FE3-4C4B-8D26-E9B47B0B9AC5@meta.com>
Precedence: bulk

On Thu, Oct 12, 2023 at 07:55:55PM +0000, Nick Terrell wrote:
> 
> > On Oct 9, 2023, at 1:29 PM, Kees Cook <keescook@chromium.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?

For all these flex array conversions we're mostly on a "slow and steady"
route, so there's no rush really. I think waiting for v6.7 is fine. If
anyone ends up wanting to backport it, it should be pretty clean
I imagine.

Thanks for getting it all landed! :)

-Kees

-- 
Kees Cook