* [PATCHSET 0/3] xfsprogs: ubsan fixes for 6.5-rc2
@ 2023-07-14 14:45 Darrick J. Wong
2023-07-14 14:45 ` [PATCH 1/3] xfs: convert flex-array declarations in struct xfs_attrlist* Darrick J. Wong
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Darrick J. Wong @ 2023-07-14 14:45 UTC (permalink / raw)
To: djwong, cem; +Cc: linux-xfs, hch, keescook, david
Hi all,
Fix some UBSAN complaints, since apparently they don't allow flex array
declarations with array[1] anymore.
If you're going to start using this mess, you probably ought to just
pull from my git trees, which are linked below.
This is an extraordinary way to destroy everything. Enjoy!
Comments and questions are, as always, welcome.
kernel git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfs-linux.git/log/?h=ubsan-fixes-6.5
xfsprogs git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=ubsan-fixes-6.5
---
db/metadump.c | 4 +--
libxfs/xfs_da_format.h | 75 ++++++++++++++++++++++++++++++++++++++++++------
libxfs/xfs_fs.h | 4 +--
3 files changed, 70 insertions(+), 13 deletions(-)
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 1/3] xfs: convert flex-array declarations in struct xfs_attrlist* 2023-07-14 14:45 [PATCHSET 0/3] xfsprogs: ubsan fixes for 6.5-rc2 Darrick J. Wong @ 2023-07-14 14:45 ` Darrick J. Wong 2023-07-14 14:46 ` Christoph Hellwig 2023-07-14 14:45 ` [PATCH 2/3] xfs: convert flex-array declarations in xfs attr leaf blocks Darrick J. Wong 2023-07-14 14:45 ` [PATCH 3/3] xfs: convert flex-array declarations in xfs attr shortform objects Darrick J. Wong 2 siblings, 1 reply; 10+ messages in thread From: Darrick J. Wong @ 2023-07-14 14:45 UTC (permalink / raw) To: djwong, cem; +Cc: linux-xfs, hch, keescook, david From: Darrick J. Wong <djwong@kernel.org> 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: <TASK> 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 <djwong@kernel.org> --- 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 { ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] xfs: convert flex-array declarations in struct xfs_attrlist* 2023-07-14 14:45 ` [PATCH 1/3] xfs: convert flex-array declarations in struct xfs_attrlist* Darrick J. Wong @ 2023-07-14 14:46 ` Christoph Hellwig 2023-07-14 14:54 ` Darrick J. Wong 0 siblings, 1 reply; 10+ messages in thread From: Christoph Hellwig @ 2023-07-14 14:46 UTC (permalink / raw) To: Darrick J. Wong; +Cc: cem, linux-xfs, hch, keescook, david The change itself looks good: Reviewed-by: Christoph Hellwig <hch@lst.de> But as mentioned in the last thread leaving these out in the UAPI is a bit dangerous, and at the same time they really shouldn't be in the uapi. Do you want me to send an incremental patch for that? ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] xfs: convert flex-array declarations in struct xfs_attrlist* 2023-07-14 14:46 ` Christoph Hellwig @ 2023-07-14 14:54 ` Darrick J. Wong 2023-07-14 14:56 ` Christoph Hellwig 0 siblings, 1 reply; 10+ messages in thread From: Darrick J. Wong @ 2023-07-14 14:54 UTC (permalink / raw) To: Christoph Hellwig; +Cc: cem, linux-xfs, keescook, david On Fri, Jul 14, 2023 at 04:46:38PM +0200, Christoph Hellwig wrote: > The change itself looks good: > > Reviewed-by: Christoph Hellwig <hch@lst.de> > > But as mentioned in the last thread leaving these out in the UAPI > is a bit dangerous, and at the same time they really shouldn't > be in the uapi. Do you want me to send an incremental patch for > that? Yes please. :) FWIW I tried removing the attrlist ioctl structs, but I couldn't find anywhere else in the kernel uapi headers that defines them so that the ioctl code can actually format the buffer. --D ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] xfs: convert flex-array declarations in struct xfs_attrlist* 2023-07-14 14:54 ` Darrick J. Wong @ 2023-07-14 14:56 ` Christoph Hellwig 0 siblings, 0 replies; 10+ messages in thread From: Christoph Hellwig @ 2023-07-14 14:56 UTC (permalink / raw) To: Darrick J. Wong; +Cc: Christoph Hellwig, cem, linux-xfs, keescook, david On Fri, Jul 14, 2023 at 07:54:39AM -0700, Darrick J. Wong wrote: > FWIW I tried removing the attrlist ioctl structs, but I couldn't find > anywhere else in the kernel uapi headers that defines them so that the > ioctl code can actually format the buffer. We need to keep them somewhere. But I'd rather not do that in xfs_fs.h which through xfsprogs goes into /usr/include/. I'd probably do it locally in xfs_ioctl.c with an extended version of the comment about matching the libattr structures. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/3] xfs: convert flex-array declarations in xfs attr leaf blocks 2023-07-14 14:45 [PATCHSET 0/3] xfsprogs: ubsan fixes for 6.5-rc2 Darrick J. Wong 2023-07-14 14:45 ` [PATCH 1/3] xfs: convert flex-array declarations in struct xfs_attrlist* Darrick J. Wong @ 2023-07-14 14:45 ` Darrick J. Wong 2023-07-14 14:54 ` Christoph Hellwig 2023-07-14 14:45 ` [PATCH 3/3] xfs: convert flex-array declarations in xfs attr shortform objects Darrick J. Wong 2 siblings, 1 reply; 10+ messages in thread From: Darrick J. Wong @ 2023-07-14 14:45 UTC (permalink / raw) To: djwong, cem; +Cc: linux-xfs, hch, keescook, david From: Darrick J. Wong <djwong@kernel.org> 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: <TASK> 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 <djwong@kernel.org> --- 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) ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] xfs: convert flex-array declarations in xfs attr leaf blocks 2023-07-14 14:45 ` [PATCH 2/3] xfs: convert flex-array declarations in xfs attr leaf blocks Darrick J. Wong @ 2023-07-14 14:54 ` Christoph Hellwig 0 siblings, 0 replies; 10+ messages in thread From: Christoph Hellwig @ 2023-07-14 14:54 UTC (permalink / raw) To: Darrick J. Wong; +Cc: cem, linux-xfs, hch, keescook, david Looks good: Reviewed-by: Christoph Hellwig <hch@lst.de> ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/3] xfs: convert flex-array declarations in xfs attr shortform objects 2023-07-14 14:45 [PATCHSET 0/3] xfsprogs: ubsan fixes for 6.5-rc2 Darrick J. Wong 2023-07-14 14:45 ` [PATCH 1/3] xfs: convert flex-array declarations in struct xfs_attrlist* Darrick J. Wong 2023-07-14 14:45 ` [PATCH 2/3] xfs: convert flex-array declarations in xfs attr leaf blocks Darrick J. Wong @ 2023-07-14 14:45 ` Darrick J. Wong 2023-07-14 14:50 ` Christoph Hellwig 2 siblings, 1 reply; 10+ messages in thread From: Darrick J. Wong @ 2023-07-14 14:45 UTC (permalink / raw) To: djwong, cem; +Cc: linux-xfs, hch, keescook, david From: Darrick J. Wong <djwong@kernel.org> 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 <djwong@kernel.org> --- 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 */ ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] xfs: convert flex-array declarations in xfs attr shortform objects 2023-07-14 14:45 ` [PATCH 3/3] xfs: convert flex-array declarations in xfs attr shortform objects Darrick J. Wong @ 2023-07-14 14:50 ` Christoph Hellwig 0 siblings, 0 replies; 10+ messages in thread From: Christoph Hellwig @ 2023-07-14 14:50 UTC (permalink / raw) To: Darrick J. Wong; +Cc: cem, linux-xfs, hch, keescook, david No one ever does a sizeof here, so this looks good: Reviewed-by: Christoph Hellwig <hch@lst.de> ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCHSET 0/3] xfs: ubsan fixes for 6.5-rc2 @ 2023-07-14 14:42 Darrick J. Wong 2023-07-14 14:42 ` [PATCH 2/3] xfs: convert flex-array declarations in xfs attr leaf blocks Darrick J. Wong 0 siblings, 1 reply; 10+ messages in thread From: Darrick J. Wong @ 2023-07-14 14:42 UTC (permalink / raw) To: david, djwong; +Cc: linux-xfs, keescook Hi all, Fix some UBSAN complaints, since apparently they don't allow flex array declarations with array[1] anymore. If you're going to start using this mess, you probably ought to just pull from my git trees, which are linked below. This is an extraordinary way to destroy everything. Enjoy! Comments and questions are, as always, welcome. --D kernel git tree: https://git.kernel.org/cgit/linux/kernel/git/djwong/xfs-linux.git/log/?h=ubsan-fixes-6.5 --- fs/xfs/libxfs/xfs_da_format.h | 75 ++++++++++++++++++++++++++++++++++++----- fs/xfs/libxfs/xfs_fs.h | 4 +- fs/xfs/xfs_ondisk.h | 5 ++- 3 files changed, 71 insertions(+), 13 deletions(-) ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/3] xfs: convert flex-array declarations in xfs attr leaf blocks 2023-07-14 14:42 [PATCHSET 0/3] xfs: ubsan fixes for 6.5-rc2 Darrick J. Wong @ 2023-07-14 14:42 ` Darrick J. Wong 0 siblings, 0 replies; 10+ messages in thread From: Darrick J. Wong @ 2023-07-14 14:42 UTC (permalink / raw) To: david, djwong; +Cc: linux-xfs, keescook From: Darrick J. Wong <djwong@kernel.org> 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: <TASK> 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 <djwong@kernel.org> --- fs/xfs/libxfs/xfs_da_format.h | 73 +++++++++++++++++++++++++++++++++++++---- fs/xfs/xfs_ondisk.h | 4 +- 2 files changed, 67 insertions(+), 10 deletions(-) diff --git a/fs/xfs/libxfs/xfs_da_format.h b/fs/xfs/libxfs/xfs_da_format.h index 25e2841084e1..b2362717c42e 100644 --- a/fs/xfs/libxfs/xfs_da_format.h +++ b/fs/xfs/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) diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h index 9737b5a9f405..37be297f2532 100644 --- a/fs/xfs/xfs_ondisk.h +++ b/fs/xfs/xfs_ondisk.h @@ -56,7 +56,7 @@ xfs_check_ondisk_structs(void) /* dir/attr trees */ XFS_CHECK_STRUCT_SIZE(struct xfs_attr3_leaf_hdr, 80); - XFS_CHECK_STRUCT_SIZE(struct xfs_attr3_leafblock, 88); + XFS_CHECK_STRUCT_SIZE(struct xfs_attr3_leafblock, 80); XFS_CHECK_STRUCT_SIZE(struct xfs_attr3_rmt_hdr, 56); XFS_CHECK_STRUCT_SIZE(struct xfs_da3_blkinfo, 56); XFS_CHECK_STRUCT_SIZE(struct xfs_da3_intnode, 64); @@ -88,7 +88,7 @@ xfs_check_ondisk_structs(void) XFS_CHECK_OFFSET(xfs_attr_leaf_name_remote_t, valuelen, 4); XFS_CHECK_OFFSET(xfs_attr_leaf_name_remote_t, namelen, 8); XFS_CHECK_OFFSET(xfs_attr_leaf_name_remote_t, name, 9); - XFS_CHECK_STRUCT_SIZE(xfs_attr_leafblock_t, 40); + XFS_CHECK_STRUCT_SIZE(xfs_attr_leafblock_t, 32); XFS_CHECK_OFFSET(struct xfs_attr_shortform, hdr.totsize, 0); XFS_CHECK_OFFSET(struct xfs_attr_shortform, hdr.count, 2); XFS_CHECK_OFFSET(struct xfs_attr_shortform, list[0].namelen, 4); ^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2023-07-14 14:56 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-07-14 14:45 [PATCHSET 0/3] xfsprogs: ubsan fixes for 6.5-rc2 Darrick J. Wong 2023-07-14 14:45 ` [PATCH 1/3] xfs: convert flex-array declarations in struct xfs_attrlist* Darrick J. Wong 2023-07-14 14:46 ` Christoph Hellwig 2023-07-14 14:54 ` Darrick J. Wong 2023-07-14 14:56 ` Christoph Hellwig 2023-07-14 14:45 ` [PATCH 2/3] xfs: convert flex-array declarations in xfs attr leaf blocks Darrick J. Wong 2023-07-14 14:54 ` Christoph Hellwig 2023-07-14 14:45 ` [PATCH 3/3] xfs: convert flex-array declarations in xfs attr shortform objects Darrick J. Wong 2023-07-14 14:50 ` Christoph Hellwig -- strict thread matches above, loose matches on Subject: below -- 2023-07-14 14:42 [PATCHSET 0/3] xfs: ubsan fixes for 6.5-rc2 Darrick J. Wong 2023-07-14 14:42 ` [PATCH 2/3] xfs: convert flex-array declarations in xfs attr leaf blocks Darrick J. Wong
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox