Skip to content

Commit a49bbce

Browse files
author
Darrick J. Wong
committed
xfs: convert flex-array declarations in xfs attr leaf blocks
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> Reviewed-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Kees Cook <keescook@chromium.org>
1 parent 371baf5 commit a49bbce

File tree

2 files changed

+67
-10
lines changed

2 files changed

+67
-10
lines changed

fs/xfs/libxfs/xfs_da_format.h

Lines changed: 65 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -620,19 +620,29 @@ typedef struct xfs_attr_leaf_entry { /* sorted on key, not name */
620620
typedef struct xfs_attr_leaf_name_local {
621621
__be16 valuelen; /* number of bytes in value */
622622
__u8 namelen; /* length of name bytes */
623-
__u8 nameval[1]; /* name/value bytes */
623+
/*
624+
* In Linux 6.5 this flex array was converted from nameval[1] to
625+
* nameval[]. Be very careful here about extra padding at the end;
626+
* see xfs_attr_leaf_entsize_local() for details.
627+
*/
628+
__u8 nameval[]; /* name/value bytes */
624629
} xfs_attr_leaf_name_local_t;
625630

626631
typedef struct xfs_attr_leaf_name_remote {
627632
__be32 valueblk; /* block number of value bytes */
628633
__be32 valuelen; /* number of bytes in value */
629634
__u8 namelen; /* length of name bytes */
630-
__u8 name[1]; /* name bytes */
635+
/*
636+
* In Linux 6.5 this flex array was converted from name[1] to name[].
637+
* Be very careful here about extra padding at the end; see
638+
* xfs_attr_leaf_entsize_remote() for details.
639+
*/
640+
__u8 name[]; /* name bytes */
631641
} xfs_attr_leaf_name_remote_t;
632642

633643
typedef struct xfs_attr_leafblock {
634644
xfs_attr_leaf_hdr_t hdr; /* constant-structure header block */
635-
xfs_attr_leaf_entry_t entries[1]; /* sorted on key, not name */
645+
xfs_attr_leaf_entry_t entries[]; /* sorted on key, not name */
636646
/*
637647
* The rest of the block contains the following structures after the
638648
* leaf entries, growing from the bottom up. The variables are never
@@ -664,7 +674,7 @@ struct xfs_attr3_leaf_hdr {
664674

665675
struct xfs_attr3_leafblock {
666676
struct xfs_attr3_leaf_hdr hdr;
667-
struct xfs_attr_leaf_entry entries[1];
677+
struct xfs_attr_leaf_entry entries[];
668678

669679
/*
670680
* 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)
747757
*/
748758
static inline int xfs_attr_leaf_entsize_remote(int nlen)
749759
{
750-
return round_up(sizeof(struct xfs_attr_leaf_name_remote) - 1 +
751-
nlen, XFS_ATTR_LEAF_NAME_ALIGN);
760+
/*
761+
* Prior to Linux 6.5, struct xfs_attr_leaf_name_remote ended with
762+
* name[1], which was used as a flexarray. The layout of this struct
763+
* is 9 bytes of fixed-length fields followed by a __u8 flex array at
764+
* offset 9.
765+
*
766+
* On most architectures, struct xfs_attr_leaf_name_remote had two
767+
* bytes of implicit padding at the end of the struct to make the
768+
* struct length 12. After converting name[1] to name[], there are
769+
* three implicit padding bytes and the struct size remains 12.
770+
* However, there are compiler configurations that do not add implicit
771+
* padding at all (m68k) and have been broken for years.
772+
*
773+
* This entsize computation historically added (the xattr name length)
774+
* to (the padded struct length - 1) and rounded that sum up to the
775+
* nearest multiple of 4 (NAME_ALIGN). IOWs, round_up(11 + nlen, 4).
776+
* This is encoded in the ondisk format, so we cannot change this.
777+
*
778+
* Compute the entsize from offsetof of the flexarray and manually
779+
* adding bytes for the implicit padding.
780+
*/
781+
const size_t remotesize =
782+
offsetof(struct xfs_attr_leaf_name_remote, name) + 2;
783+
784+
return round_up(remotesize + nlen, XFS_ATTR_LEAF_NAME_ALIGN);
752785
}
753786

754787
static inline int xfs_attr_leaf_entsize_local(int nlen, int vlen)
755788
{
756-
return round_up(sizeof(struct xfs_attr_leaf_name_local) - 1 +
757-
nlen + vlen, XFS_ATTR_LEAF_NAME_ALIGN);
789+
/*
790+
* Prior to Linux 6.5, struct xfs_attr_leaf_name_local ended with
791+
* nameval[1], which was used as a flexarray. The layout of this
792+
* struct is 3 bytes of fixed-length fields followed by a __u8 flex
793+
* array at offset 3.
794+
*
795+
* struct xfs_attr_leaf_name_local had zero bytes of implicit padding
796+
* at the end of the struct to make the struct length 4. On most
797+
* architectures, after converting nameval[1] to nameval[], there is
798+
* one implicit padding byte and the struct size remains 4. However,
799+
* there are compiler configurations that do not add implicit padding
800+
* at all (m68k) and would break.
801+
*
802+
* This entsize computation historically added (the xattr name and
803+
* value length) to (the padded struct length - 1) and rounded that sum
804+
* up to the nearest multiple of 4 (NAME_ALIGN). IOWs, the formula is
805+
* round_up(3 + nlen + vlen, 4). This is encoded in the ondisk format,
806+
* so we cannot change this.
807+
*
808+
* Compute the entsize from offsetof of the flexarray and manually
809+
* adding bytes for the implicit padding.
810+
*/
811+
const size_t localsize =
812+
offsetof(struct xfs_attr_leaf_name_local, nameval);
813+
814+
return round_up(localsize + nlen + vlen, XFS_ATTR_LEAF_NAME_ALIGN);
758815
}
759816

760817
static inline int xfs_attr_leaf_entsize_local_max(int bsize)

fs/xfs/xfs_ondisk.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ xfs_check_ondisk_structs(void)
5656

5757
/* dir/attr trees */
5858
XFS_CHECK_STRUCT_SIZE(struct xfs_attr3_leaf_hdr, 80);
59-
XFS_CHECK_STRUCT_SIZE(struct xfs_attr3_leafblock, 88);
59+
XFS_CHECK_STRUCT_SIZE(struct xfs_attr3_leafblock, 80);
6060
XFS_CHECK_STRUCT_SIZE(struct xfs_attr3_rmt_hdr, 56);
6161
XFS_CHECK_STRUCT_SIZE(struct xfs_da3_blkinfo, 56);
6262
XFS_CHECK_STRUCT_SIZE(struct xfs_da3_intnode, 64);
@@ -88,7 +88,7 @@ xfs_check_ondisk_structs(void)
8888
XFS_CHECK_OFFSET(xfs_attr_leaf_name_remote_t, valuelen, 4);
8989
XFS_CHECK_OFFSET(xfs_attr_leaf_name_remote_t, namelen, 8);
9090
XFS_CHECK_OFFSET(xfs_attr_leaf_name_remote_t, name, 9);
91-
XFS_CHECK_STRUCT_SIZE(xfs_attr_leafblock_t, 40);
91+
XFS_CHECK_STRUCT_SIZE(xfs_attr_leafblock_t, 32);
9292
XFS_CHECK_OFFSET(struct xfs_attr_shortform, hdr.totsize, 0);
9393
XFS_CHECK_OFFSET(struct xfs_attr_shortform, hdr.count, 2);
9494
XFS_CHECK_OFFSET(struct xfs_attr_shortform, list[0].namelen, 4);

0 commit comments

Comments
 (0)