From patchwork Fri Jul 14 14:45:10 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Darrick J. Wong" X-Patchwork-Id: 13313755 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id A6D39C0015E for ; Fri, 14 Jul 2023 14:45:21 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236172AbjGNOpU (ORCPT ); Fri, 14 Jul 2023 10:45:20 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55722 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235996AbjGNOpN (ORCPT ); Fri, 14 Jul 2023 10:45:13 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 86B2C268F for ; Fri, 14 Jul 2023 07:45:12 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 2467861D3D for ; Fri, 14 Jul 2023 14:45:12 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 85B3CC433C8; Fri, 14 Jul 2023 14:45:11 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1689345911; bh=9sIVcZbhCxcCIppatIwU3lDDyfui51oHHSEw/UZz2JM=; h=Subject:From:To:Cc:Date:In-Reply-To:References:From; b=fZ9+ITRqzHL+gYLulGseXIFXJJ4QN4DWa5/J+DrGK+cLxTx7ncMIpt6MB7uacITr9 L1GQ2ACoBHkDVuox1wFkht3zNo/tE4SazqkVFirZTGvrCRL0yu0tK+bFL404wfLZEQ n7qaN4ZV1vxydN4Z02Xv9PV5VMWOXaL1HL6FLrAFNszdNUssQ6TPc9IKyRjCdhwJWC daH0a5Jo+zVTIF2pX/Br2seaap3EwzwvVgYvHl+qSsICZtvVq2GidE9sK8AyPfAdAs epsW0g7vn5WLwqsnyHJsR9G3FXB1l7+PTEyNz5Cphe2Koq5oKavz7Ice/7aO8odtka kvXytV6wSHP2A== Subject: [PATCH 1/3] xfs: convert flex-array declarations in struct xfs_attrlist* From: "Darrick J. Wong" To: djwong@kernel.org, cem@kernel.org Cc: linux-xfs@vger.kernel.org, hch@lst.de, keescook@chromium.org, david@fromorbit.com Date: Fri, 14 Jul 2023 07:45:10 -0700 Message-ID: <168934591095.3368057.15849162788748534581.stgit@frogsfrogsfrogs> In-Reply-To: <168934590524.3368057.8686152348214871657.stgit@frogsfrogsfrogs> References: <168934590524.3368057.8686152348214871657.stgit@frogsfrogsfrogs> User-Agent: StGit/0.19 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org From: Darrick J. Wong Source kernel commit: d6da44c23ec357f132717301ea3319b8aa95274e As of 6.5-rc1, UBSAN trips over the attrlist ioctl definitions using an array length of 1 to pretend to be a flex array. Kernel compilers have to support unbounded array declarations, so let's correct this. This may cause friction with userspace header declarations, but suck is life. ================================================================================ UBSAN: array-index-out-of-bounds in fs/xfs/xfs_ioctl.c:345:18 index 1 is out of range for type '__s32 [1]' Call Trace: dump_stack_lvl+0x33/0x50 __ubsan_handle_out_of_bounds+0x9c/0xd0 xfs_ioc_attr_put_listent+0x413/0x420 [xfs 4a986a89a77bb77402ab8a87a37da369ef6a3f09] xfs_attr_list_ilocked+0x170/0x850 [xfs 4a986a89a77bb77402ab8a87a37da369ef6a3f09] xfs_attr_list+0xb7/0x120 [xfs 4a986a89a77bb77402ab8a87a37da369ef6a3f09] xfs_ioc_attr_list+0x13b/0x2e0 [xfs 4a986a89a77bb77402ab8a87a37da369ef6a3f09] xfs_attrlist_by_handle+0xab/0x120 [xfs 4a986a89a77bb77402ab8a87a37da369ef6a3f09] xfs_file_ioctl+0x1ff/0x15e0 [xfs 4a986a89a77bb77402ab8a87a37da369ef6a3f09] vfs_ioctl+0x1f/0x60 The kernel and xfsprogs code that uses these structures will not have problems, but the long tail of external user programs might. Signed-off-by: Darrick J. Wong Reviewed-by: Christoph Hellwig --- libxfs/xfs_fs.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libxfs/xfs_fs.h b/libxfs/xfs_fs.h index 9c60ebb3..2cbf9ea3 100644 --- a/libxfs/xfs_fs.h +++ b/libxfs/xfs_fs.h @@ -592,12 +592,12 @@ typedef struct xfs_attrlist_cursor { struct xfs_attrlist { __s32 al_count; /* number of entries in attrlist */ __s32 al_more; /* T/F: more attrs (do call again) */ - __s32 al_offset[1]; /* byte offsets of attrs [var-sized] */ + __s32 al_offset[]; /* byte offsets of attrs [var-sized] */ }; struct xfs_attrlist_ent { /* data from attr_list() */ __u32 a_valuelen; /* number bytes in value of attr */ - char a_name[1]; /* attr name (NULL terminated) */ + char a_name[]; /* attr name (NULL terminated) */ }; typedef struct xfs_fsop_attrlist_handlereq { From patchwork Fri Jul 14 14:45:16 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Darrick J. Wong" X-Patchwork-Id: 13313756 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id F3FA7C0015E for ; Fri, 14 Jul 2023 14:45:24 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236158AbjGNOpY (ORCPT ); Fri, 14 Jul 2023 10:45:24 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55890 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236173AbjGNOpV (ORCPT ); Fri, 14 Jul 2023 10:45:21 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 78985213B for ; Fri, 14 Jul 2023 07:45:18 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id D1FCB61D3E for ; Fri, 14 Jul 2023 14:45:17 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 3B56EC433C8; Fri, 14 Jul 2023 14:45:17 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1689345917; bh=4Ww3w8Cliljwncan3qmjZ6bom6mlUV8YHs5dWUImNDQ=; h=Subject:From:To:Cc:Date:In-Reply-To:References:From; b=kHg3BJX+YVHWvMsUUKST6S9YOf7DC3wmeDvSGTVySJt5bbVhF25unBkz3Y65of533 Cw0GvkK6tSdIEIw8kqoB9Z2I0qXCfNDNr2WE2rDqeqbfP1d/x1qFde/sm/0tU3fgxq DJpooRnxbAvbkylDq5N5JtZ1itl1GF7nSO+5eE/lGE9ZlARuDKT+KVljAZTofuueU1 yNun/F4ZWy0PRcz2MENjrZoXCb/AsrnqKkJmhC/UzyzGXexDYbV9uNh0QClBFOq7sV dOfrwtPsphAzSHshxnk5oNsifFjU7HzDRObe8yyGKbRZA3xFhSFsHKqibzmtErVTHW SzK3HdtivqNzw== Subject: [PATCH 2/3] xfs: convert flex-array declarations in xfs attr leaf blocks From: "Darrick J. Wong" To: djwong@kernel.org, cem@kernel.org Cc: linux-xfs@vger.kernel.org, hch@lst.de, keescook@chromium.org, david@fromorbit.com Date: Fri, 14 Jul 2023 07:45:16 -0700 Message-ID: <168934591669.3368057.4588378580238137738.stgit@frogsfrogsfrogs> In-Reply-To: <168934590524.3368057.8686152348214871657.stgit@frogsfrogsfrogs> References: <168934590524.3368057.8686152348214871657.stgit@frogsfrogsfrogs> User-Agent: StGit/0.19 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org From: Darrick J. Wong Source kernel commit: 3d282679ac2249f0d239a4ae2403d884fccd3ea1 As of 6.5-rc1, UBSAN trips over the ondisk extended attribute leaf block definitions using an array length of 1 to pretend to be a flex array. Kernel compilers have to support unbounded array declarations, so let's correct this. ================================================================================ UBSAN: array-index-out-of-bounds in fs/xfs/libxfs/xfs_attr_leaf.c:2535:24 index 2 is out of range for type '__u8 [1]' Call Trace: dump_stack_lvl+0x33/0x50 __ubsan_handle_out_of_bounds+0x9c/0xd0 xfs_attr3_leaf_getvalue+0x2ce/0x2e0 [xfs 4a986a89a77bb77402ab8a87a37da369ef6a3f09] xfs_attr_leaf_get+0x148/0x1c0 [xfs 4a986a89a77bb77402ab8a87a37da369ef6a3f09] xfs_attr_get_ilocked+0xae/0x110 [xfs 4a986a89a77bb77402ab8a87a37da369ef6a3f09] xfs_attr_get+0xee/0x150 [xfs 4a986a89a77bb77402ab8a87a37da369ef6a3f09] xfs_xattr_get+0x7d/0xc0 [xfs 4a986a89a77bb77402ab8a87a37da369ef6a3f09] __vfs_getxattr+0xa3/0x100 vfs_getxattr+0x87/0x1d0 do_getxattr+0x17a/0x220 getxattr+0x89/0xf0 Signed-off-by: Darrick J. Wong Reviewed-by: Christoph Hellwig --- db/metadump.c | 4 +-- libxfs/xfs_da_format.h | 73 +++++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 67 insertions(+), 10 deletions(-) diff --git a/db/metadump.c b/db/metadump.c index d9a616a9..3545124f 100644 --- a/db/metadump.c +++ b/db/metadump.c @@ -1475,7 +1475,7 @@ process_attr_block( nlen = local->namelen; vlen = be16_to_cpu(local->valuelen); zlen = xfs_attr_leaf_entsize_local(nlen, vlen) - - (sizeof(xfs_attr_leaf_name_local_t) - 1 + + (offsetof(struct xfs_attr_leaf_name_local, nameval) + nlen + vlen); if (zero_stale_data) memset(&local->nameval[nlen + vlen], 0, zlen); @@ -1497,7 +1497,7 @@ process_attr_block( /* zero from end of name[] to next name start */ nlen = remote->namelen; zlen = xfs_attr_leaf_entsize_remote(nlen) - - (sizeof(xfs_attr_leaf_name_remote_t) - 1 + + (offsetof(struct xfs_attr_leaf_name_remote, name) + nlen); if (zero_stale_data) memset(&remote->name[nlen], 0, zlen); diff --git a/libxfs/xfs_da_format.h b/libxfs/xfs_da_format.h index 25e28410..b2362717 100644 --- a/libxfs/xfs_da_format.h +++ b/libxfs/xfs_da_format.h @@ -620,19 +620,29 @@ typedef struct xfs_attr_leaf_entry { /* sorted on key, not name */ typedef struct xfs_attr_leaf_name_local { __be16 valuelen; /* number of bytes in value */ __u8 namelen; /* length of name bytes */ - __u8 nameval[1]; /* name/value bytes */ + /* + * In Linux 6.5 this flex array was converted from nameval[1] to + * nameval[]. Be very careful here about extra padding at the end; + * see xfs_attr_leaf_entsize_local() for details. + */ + __u8 nameval[]; /* name/value bytes */ } xfs_attr_leaf_name_local_t; typedef struct xfs_attr_leaf_name_remote { __be32 valueblk; /* block number of value bytes */ __be32 valuelen; /* number of bytes in value */ __u8 namelen; /* length of name bytes */ - __u8 name[1]; /* name bytes */ + /* + * In Linux 6.5 this flex array was converted from name[1] to name[]. + * Be very careful here about extra padding at the end; see + * xfs_attr_leaf_entsize_remote() for details. + */ + __u8 name[]; /* name bytes */ } xfs_attr_leaf_name_remote_t; typedef struct xfs_attr_leafblock { xfs_attr_leaf_hdr_t hdr; /* constant-structure header block */ - xfs_attr_leaf_entry_t entries[1]; /* sorted on key, not name */ + xfs_attr_leaf_entry_t entries[]; /* sorted on key, not name */ /* * The rest of the block contains the following structures after the * leaf entries, growing from the bottom up. The variables are never @@ -664,7 +674,7 @@ struct xfs_attr3_leaf_hdr { struct xfs_attr3_leafblock { struct xfs_attr3_leaf_hdr hdr; - struct xfs_attr_leaf_entry entries[1]; + struct xfs_attr_leaf_entry entries[]; /* * The rest of the block contains the following structures after the @@ -747,14 +757,61 @@ xfs_attr3_leaf_name_local(xfs_attr_leafblock_t *leafp, int idx) */ static inline int xfs_attr_leaf_entsize_remote(int nlen) { - return round_up(sizeof(struct xfs_attr_leaf_name_remote) - 1 + - nlen, XFS_ATTR_LEAF_NAME_ALIGN); + /* + * Prior to Linux 6.5, struct xfs_attr_leaf_name_remote ended with + * name[1], which was used as a flexarray. The layout of this struct + * is 9 bytes of fixed-length fields followed by a __u8 flex array at + * offset 9. + * + * On most architectures, struct xfs_attr_leaf_name_remote had two + * bytes of implicit padding at the end of the struct to make the + * struct length 12. After converting name[1] to name[], there are + * three implicit padding bytes and the struct size remains 12. + * However, there are compiler configurations that do not add implicit + * padding at all (m68k) and have been broken for years. + * + * This entsize computation historically added (the xattr name length) + * to (the padded struct length - 1) and rounded that sum up to the + * nearest multiple of 4 (NAME_ALIGN). IOWs, round_up(11 + nlen, 4). + * This is encoded in the ondisk format, so we cannot change this. + * + * Compute the entsize from offsetof of the flexarray and manually + * adding bytes for the implicit padding. + */ + const size_t remotesize = + offsetof(struct xfs_attr_leaf_name_remote, name) + 2; + + return round_up(remotesize + nlen, XFS_ATTR_LEAF_NAME_ALIGN); } static inline int xfs_attr_leaf_entsize_local(int nlen, int vlen) { - return round_up(sizeof(struct xfs_attr_leaf_name_local) - 1 + - nlen + vlen, XFS_ATTR_LEAF_NAME_ALIGN); + /* + * Prior to Linux 6.5, struct xfs_attr_leaf_name_local ended with + * nameval[1], which was used as a flexarray. The layout of this + * struct is 3 bytes of fixed-length fields followed by a __u8 flex + * array at offset 3. + * + * struct xfs_attr_leaf_name_local had zero bytes of implicit padding + * at the end of the struct to make the struct length 4. On most + * architectures, after converting nameval[1] to nameval[], there is + * one implicit padding byte and the struct size remains 4. However, + * there are compiler configurations that do not add implicit padding + * at all (m68k) and would break. + * + * This entsize computation historically added (the xattr name and + * value length) to (the padded struct length - 1) and rounded that sum + * up to the nearest multiple of 4 (NAME_ALIGN). IOWs, the formula is + * round_up(3 + nlen + vlen, 4). This is encoded in the ondisk format, + * so we cannot change this. + * + * Compute the entsize from offsetof of the flexarray and manually + * adding bytes for the implicit padding. + */ + const size_t localsize = + offsetof(struct xfs_attr_leaf_name_local, nameval); + + return round_up(localsize + nlen + vlen, XFS_ATTR_LEAF_NAME_ALIGN); } static inline int xfs_attr_leaf_entsize_local_max(int bsize) From patchwork Fri Jul 14 14:45:22 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Darrick J. Wong" X-Patchwork-Id: 13313757 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 44489EB64DC for ; Fri, 14 Jul 2023 14:45:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235360AbjGNOp1 (ORCPT ); Fri, 14 Jul 2023 10:45:27 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55862 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236164AbjGNOpZ (ORCPT ); Fri, 14 Jul 2023 10:45:25 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [IPv6:2604:1380:4641:c500::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1259B10FA for ; Fri, 14 Jul 2023 07:45:25 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 7679661D42 for ; Fri, 14 Jul 2023 14:45:23 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id D9688C433C7; Fri, 14 Jul 2023 14:45:22 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1689345922; bh=nKRd9q6jo/GDnTKZIOTHDyXYs8JqzUSf2jEvdKDk2HU=; h=Subject:From:To:Cc:Date:In-Reply-To:References:From; b=lzXv55mD+R9LatiV6YKl8M0KWpffogFJH9quKmnft+95JHu/D4BsAs/e1jBibqlHT PhoBajFmTMIFKcnlUb7cZZF6/BLOvl/mtPS/RokgCu3y6hdvt0QY17YSxdi7euylZn rbBU2p/jEjTO5OWXc2WF6eHgS+scRKc6gHBCHA3kTjSUsDdRfwZkNLVSUoZU2JkF+I QPzyo2c23MiKRPOEoNQcAJHLcW0LttHuVm4rZwFiEa2TcOAbt5TvyZTy9eI4IBItr4 41zuQk4HmANJjkgCiWSXc97EPBlCVZETy0hOkGDEuWcxFuc4ehTLjMGYZSii5fSwpf sWjsNXAuPfGYg== Subject: [PATCH 3/3] xfs: convert flex-array declarations in xfs attr shortform objects From: "Darrick J. Wong" To: djwong@kernel.org, cem@kernel.org Cc: linux-xfs@vger.kernel.org, hch@lst.de, keescook@chromium.org, david@fromorbit.com Date: Fri, 14 Jul 2023 07:45:22 -0700 Message-ID: <168934592239.3368057.13821438121542148084.stgit@frogsfrogsfrogs> In-Reply-To: <168934590524.3368057.8686152348214871657.stgit@frogsfrogsfrogs> References: <168934590524.3368057.8686152348214871657.stgit@frogsfrogsfrogs> User-Agent: StGit/0.19 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org From: Darrick J. Wong Source kernel commit: 980f90c04e1b0fcbc4ccfb1009a724f38adced7d As of 6.5-rc1, UBSAN trips over the ondisk extended attribute shortform definitions using an array length of 1 to pretend to be a flex array. Kernel compilers have to support unbounded array declarations, so let's correct this. Signed-off-by: Darrick J. Wong Reviewed-by: Christoph Hellwig --- libxfs/xfs_da_format.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libxfs/xfs_da_format.h b/libxfs/xfs_da_format.h index b2362717..f9015f88 100644 --- a/libxfs/xfs_da_format.h +++ b/libxfs/xfs_da_format.h @@ -591,7 +591,7 @@ struct xfs_attr_shortform { uint8_t valuelen; /* actual length of value (no NULL) */ uint8_t flags; /* flags bits (see xfs_attr_leaf.h) */ uint8_t nameval[]; /* name & value bytes concatenated */ - } list[1]; /* variable sized array */ + } list[]; /* variable sized array */ }; typedef struct xfs_attr_leaf_map { /* RLE map of free bytes */