[PATCH] libxfs: Redefine 1-element arrays as flexible arrays

* [PATCH] libxfs: Redefine 1-element arrays as flexible arrays
@ 2023-07-11 22:20 Kees Cook
  2023-07-12  5:37 ` Darrick J. Wong
  0 siblings, 1 reply; 6+ messages in thread
From: Kees Cook @ 2023-07-11 22:20 UTC (permalink / raw)
  To: Carlos Maiolino
  Cc: Kees Cook, Darrick J. Wong, Dave Chinner, Jeff Layton,
	Eric Biggers, Gustavo A. R. Silva, linux-xfs, linux-hardening

To allow for code bases that include libxfs (e.g. the Linux kernel) and
build with strict flexible array handling (-fstrict-flex-arrays=3),
FORTIFY_SOURCE, and/or UBSAN bounds checking, redefine the remaining
1-element trailing arrays as true flexible arrays, but without changing
their structure sizes. This is done via a union to retain a single element
(named "legacy_padding"). As not all distro headers may yet have the
UAPI stddef.h __DECLARE_FLEX_ARRAY macro, include it explicitly in
platform_defs.h.in.

Addresses the warnings being seen under Linux:
https://lore.kernel.org/all/20230710170243.GF11456@frogsfrogsfrogs

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 include/platform_defs.h.in | 18 ++++++++++++++++++
 libxfs/xfs_da_format.h     | 38 ++++++++++++++++++++++++++++----------
 libxfs/xfs_fs.h            | 12 ++++++++++--
 3 files changed, 56 insertions(+), 12 deletions(-)

diff --git a/include/platform_defs.h.in b/include/platform_defs.h.in
index 315ad77cfb78..a55965ce050d 100644
--- a/include/platform_defs.h.in
+++ b/include/platform_defs.h.in
@@ -134,6 +134,24 @@ static inline size_t __ab_c_size(size_t a, size_t b, size_t c)
 #    define fallthrough                    do {} while (0)  /* fallthrough */
 #endif
 
+#ifndef __DECLARE_FLEX_ARRAY
+/**
+ * __DECLARE_FLEX_ARRAY() - Declare a flexible array usable in a union
+ *
+ * @type: The type of each flexible array element
+ * @name: The name of the flexible array member
+ *
+ * In order to have a flexible array member in a union or alone in a
+ * struct, it needs to be wrapped in an anonymous struct with at least 1
+ * named member, but that member can be empty.
+ */
+#define __DECLARE_FLEX_ARRAY(type, name)	\
+	struct { \
+		struct { } __empty_ ## name; \
+		type name[]; \
+	}
+#endif
+
 /* Only needed for the kernel. */
 #define __init
 
diff --git a/libxfs/xfs_da_format.h b/libxfs/xfs_da_format.h
index 25e2841084e1..4af92f16d5cd 100644
--- a/libxfs/xfs_da_format.h
+++ b/libxfs/xfs_da_format.h
@@ -580,18 +580,23 @@ xfs_dir2_block_leaf_p(struct xfs_dir2_block_tail *btp)
 /*
  * Entries are packed toward the top as tight as possible.
  */
+struct xfs_attr_sf_entry {
+	uint8_t namelen;	/* actual length of name (no NULL) */
+	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 */
+};
+
 struct xfs_attr_shortform {
 	struct xfs_attr_sf_hdr {	/* constant-structure header block */
 		__be16	totsize;	/* total bytes in shortform list */
 		__u8	count;	/* count of active entries */
 		__u8	padding;
 	} hdr;
-	struct xfs_attr_sf_entry {
-		uint8_t namelen;	/* actual length of name (no NULL) */
-		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 */
+	union {
+		struct xfs_attr_sf_entry legacy_padding;
+		__DECLARE_FLEX_ARRAY(struct xfs_attr_sf_entry, list);
+	};
 };
 
 typedef struct xfs_attr_leaf_map {	/* RLE map of free bytes */
@@ -620,19 +625,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 */
+	union {
+		__u8	legacy_padding;
+		__DECLARE_FLEX_ARRAY(__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 */
+	union {
+		__u8	legacy_padding;
+		__DECLARE_FLEX_ARRAY(__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 */
+	union {
+		xfs_attr_leaf_entry_t	legacy_padding;
+		/* sorted on key, not name */
+		__DECLARE_FLEX_ARRAY(xfs_attr_leaf_entry_t, entries);
+	};
 	/*
 	 * The rest of the block contains the following structures after the
 	 * leaf entries, growing from the bottom up. The variables are never
@@ -664,7 +679,10 @@ struct xfs_attr3_leaf_hdr {
 
 struct xfs_attr3_leafblock {
 	struct xfs_attr3_leaf_hdr	hdr;
-	struct xfs_attr_leaf_entry	entries[1];
+	union {
+		struct xfs_attr_leaf_entry	legacy_padding;
+		__DECLARE_FLEX_ARRAY(struct xfs_attr_leaf_entry, entries);
+	};
 
 	/*
 	 * The rest of the block contains the following structures after the
diff --git a/libxfs/xfs_fs.h b/libxfs/xfs_fs.h
index 1cfd5bc6520a..d6ec66ef0194 100644
--- a/libxfs/xfs_fs.h
+++ b/libxfs/xfs_fs.h
@@ -590,12 +590,20 @@ 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] */
+	union {
+		__s32	legacy_padding;
+		/* byte offsets of attrs [var-sized] */
+		__DECLARE_FLEX_ARRAY(__s32, al_offset);
+	};
 };
 
 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) */
+	union {
+		char	legacy_padding;
+		/* attr name (NULL terminated) */
+		__DECLARE_FLEX_ARRAY(char, a_name);
+	};
 };
 
 typedef struct xfs_fsop_attrlist_handlereq {
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread