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>

diff --git a/fs/xfs/libxfs/xfs_da_format.h b/fs/xfs/libxfs/xfs_da_format.h
index 25e2841084e1147fc0f8b32b81f3d79948627ae5..b2362717c42ed85395d9592c1edfbc05c59ea83e 100644 (file)
--- 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 9737b5a9f405e03717d2ae2f4cc68c4bb77e921f..37be297f2532b508a96bf485a4aab59943ab9896 100644 (file)
--- 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);