Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:8 as permitted sender) client-ip=2620:137:e000::3:8;
From:   Nick Terrell <terrelln@meta.com>
To:     Kees Cook <keescook@chromium.org>
CC:     Nick Terrell <terrelln@meta.com>,
        Eric Biggers <ebiggers@kernel.org>,
        Nick Terrell <terrelln@meta.com>,
        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
Thread-Topic: [syzbot] [zstd] UBSAN: array-index-out-of-bounds in
 FSE_decompress_wksp_body_bmi2
Thread-Index: AQHZ+WIW2tZXKI1VfkS0RwXdr6keRLBBuYeAgATf4AA=
Date:   Thu, 12 Oct 2023 19:55:55 +0000
Message-ID: <19E42116-8FE3-4C4B-8D26-E9B47B0B9AC5@meta.com>
References: <00000000000049964e06041f2cbf@google.com>
 <20231007210556.GA174883@sol.localdomain> <202310091025.4939AEBC9@keescook>
In-Reply-To: <202310091025.4939AEBC9@keescook>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?aEkfsEurJJsKUKsGcZseVb0D1StzbuThw5CneQUuWKmC0e2XoAxgbd95yRXy?=
 =?us-ascii?Q?ozFuGqHQWuySrKCXmHyuJfJ9dJEoJikkIiGlkrf1Cp+1+tG/1zuxEIel9Uqm?=
 =?us-ascii?Q?nxS6ukw4E4NbbA5snPQGFQa82va9E9zhxc0qQGSZLGZlVljv19Xfeoz1ezL9?=
 =?us-ascii?Q?FbCKiX/ITdroTpKbRGibqo8yD9pff5jdLP2T6NYBlOQgaD7Go/7GnBID+t7W?=
 =?us-ascii?Q?UEajBBONvI2IgOL7ViJaVyemorIXNd1DeUPMH/y52jO9RMQeEkDjBVRzyYZV?=
 =?us-ascii?Q?lQV5pDq9bjCknUTQU1BkSln5hxcI+NAXiFhduN8ZD8ouVefEKqKMW6x1ocPZ?=
 =?us-ascii?Q?eOaunaVYIFqE1oMZB8YKafn1Wivb2MaWWzL6c/Ug0v4plZe/47RnFN5ViWPQ?=
 =?us-ascii?Q?lal9FxUC0f5dEKurYuYfhI6+86SPX/Mn7zctazo/SShDQJuyoKfR7DDYxzeK?=
 =?us-ascii?Q?Yq0hRA/PZjzhv+4hSiaqe+W8rdeDl6NwOrjsBibSo25oPHZ5Dp3H4ACmSebL?=
 =?us-ascii?Q?dCPjVpAWV9brO7zb9lGkaygcFWmLHBsExupPE9zyM8sy6ac+gRRpFsogV3mY?=
 =?us-ascii?Q?Sn9AFv9/m+pTW3XieLQgh3hcbq7e3dO93dmTFJnBDeojyxtEBJnjfdbtE8R0?=
 =?us-ascii?Q?zwERTkY0f6+2Hw3EWi0aUoek96AQ39T2Chw001B4MDLSzJ9AHO1tIsued6DH?=
 =?us-ascii?Q?HLv9AZYRW9ywMURwO/ErpiD+0tJSaAG4J/VZ6VCS2aSg3aqP8bEvIJLgND4k?=
 =?us-ascii?Q?tHvBt96PdFr6g3ZwMVOFd0SJ1WYlYtk+vfMG6Hqu0+u5mLFR0Fy/ouqI0Y36?=
 =?us-ascii?Q?khNuyiwtb1WHBKZk0vnksWo+UwKVQF/duyaPXhnQd7nOaXy12zDnx17Y5TkD?=
 =?us-ascii?Q?OSlf7WOPZFj+DZPPva3ilA0/fsoIcypKTSjG6BkUTZhcupuWqoDkB70KObt6?=
 =?us-ascii?Q?fJiSBMSlFtCFJXe4AzpwdekI2SgOZljKzQxrt0WqU00gstB9SCdIRDig1IWB?=
 =?us-ascii?Q?KfNf+K1ucU1AJrGin3OGZbrzAY4nypJ3emn2djVvwqTax/m2hLko8YnWaz3c?=
 =?us-ascii?Q?flv28El45SbNvplfAf02gfJQxhcjB7gkUgca8wkHRNhpilsiCZumoUlAlSyQ?=
 =?us-ascii?Q?cKtTTo9kcgU+wEBxs0A20lbih4iA9QxpjoEI8sBfzI0WqMioTc79WW7UnAqo?=
 =?us-ascii?Q?lnecbblph8yyEuvPBWJdWFXsOhzclGZP40Nt3UV+RH92P9WaXpZ+uqkSTMv/?=
 =?us-ascii?Q?N41kTGzgWxj0N8CRl8bUd5nn3+qU+fLVTWkr+b6jmRfw89j1J77HkGjvrzYU?=
 =?us-ascii?Q?CbfoUGu6JxLrCSA6RlNoVwnEMy41mpTqBQMV6speB9w18fu4SyFUJFBLyJuU?=
 =?us-ascii?Q?hK1yVA3oqzlFz+sL43mp4IxpLLFN8W0pL/LqF/ICQReWm6W7tTMmTi8WWxRi?=
 =?us-ascii?Q?tx0p9fb7SeVYqN0vc9O2fdZJn/fe7sl9Crhr/Acy/QdNk/wz6bTv5r8khjRX?=
 =?us-ascii?Q?A863cBFwa/W32G7wJrZiKIa3Zliq280GSeTKhYmWRKFGYPVmO1ZmoX/OIzAx?=
 =?us-ascii?Q?YzuEdytX2lS/QOPt7IdL4mpLmDaxqj1rQK49WlqjDTP/dKb109Wo4/OdaMuO?=
 =?us-ascii?Q?NepG8oiH/GiUdRB7u6uWWF0=3D?=
Content-ID: <7542183AE913FF4DBE5D0A19ECBBEDE0@namprd15.prod.outlook.com>
X-OriginatorOrg: meta.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: BY5PR15MB3667.namprd15.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: a9f60fb9-a1df-4ca1-6689-08dbcb5d3eef
X-MS-Exchange-CrossTenant-originalarrivaltime: 12 Oct 2023 19:55:55.1054
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 8ae927fe-1255-47a7-a2af-5f3a069daaa2
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: CD4zjJgj6ejthvuJANh1CRLLKftfioD657Wg/DG4nQHqMpMsHbmEq2oSaKW8CRVufEUq5y+smlxkM9rzQ3BiJA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR15MB3829
X-Proofpoint-GUID: pX3tiQ7Tk0GZIHmH1y2NVmCMh-0Xhwf2
X-Proofpoint-ORIG-GUID: pX3tiQ7Tk0GZIHmH1y2NVmCMh-0Xhwf2
Content-Type: text/plain; charset="us-ascii"
X-Proofpoint-UnRewURL: 0 URL was un-rewritten
MIME-Version: 1.0
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.267,Aquarius:18.0.980,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2023-10-12_12,2023-10-12_01,2023-05-22_02
X-Spam-Status: No, score=-0.8 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
	DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,
	SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no
	version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on fry.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (fry.vger.email [0.0.0.0]); Thu, 12 Oct 2023 12:56:32 -0700 (PDT)


> 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?

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