Skip to contentSkip to conversation

net: Avoid thousands of -Wflex-array-member-not-at-end warnings

PATCH In review Gustavo A. R. Silva 5 participants 15 messages Oct 24, 2024 – Nov 4, 2024

Subsystems (5)
lore
Share & tools
lore
#0 lore raw

v2 cover net: Avoid thousands of -Wflex-array-member-not-at-end warnings

This series aims to resolve thousands of -Wflex-array-member-not-at-end warnings by introducing struct sockaddr_legacy. The intention is to use it to replace the type of several struct members in the middle of composite structures, currently of type struct sockaddr.

These middle struct members are currently causing thousands of warnings because struct sockaddr contains a flexible-array member, introduced by commit b5f0de6df6dce ("net: dev: Convert sa_data to flexible array in struct sockaddR").

The new struct sockaddr_legacy doesn't include a flexible-array member, making it suitable for use as the type of middle members in composite structs that don't really require the flexible-array member in struct sockaddr, thus avoiding -Wflex-array-member-not-at-end warnings.

As this new struct will live in UAPI, to avoid breaking user-space code that expects struct sockaddr, the __kernel_sockaddr_legacy macro is introduced. This macro allows us to use either struct sockaddr or struct sockaddr_legacy depending on the context in which the code is
used: kernel-space or user-space.

Changes in v2
- Drop nfsd patch.
- Move struct sockaddr_legacy to include/uapi/linux/socket.h
- Introduce __kernel_sockaddr_legacy macro (Kees)

v1:

Gustavo A. R. Silva (4):
uapi: socket: Introduce struct sockaddr_legacy
uapi: wireless: Avoid -Wflex-array-member-not-at-end warnings
uapi: net: arp: Avoid -Wflex-array-member-not-at-end warnings
uapi: net: Avoid -Wflex-array-member-not-at-end warnings

 include/net/compat.h          | 30 +++++++++----------
 include/uapi/linux/if_arp.h   | 18 +++++------
 include/uapi/linux/route.h    | 28 +++++++++---------
 include/uapi/linux/socket.h   | 28 ++++++++++++++++++
 include/uapi/linux/wireless.h | 56 +++++++++++++++++------------------
 net/appletalk/ddp.c           |  2 +-
 net/ipv4/af_inet.c            |  2 +-
 net/ipv4/arp.c                |  2 +-
 net/ipv4/fib_frontend.c       |  2 +-
 9 files changed, 98 insertions(+), 70 deletions(-)
Signature · 1 line
2.34.1
#1 lore raw

PATCH v2 1/4 uapi: socket: Introduce struct sockaddr_legacy

We are currently working on enabling the -Wflex-array-member-not-at-end compiler option. This option has helped us detect several objects of the type struct sockaddr that appear in the middle of composite structures like struct rtentry, struct compat_rtentry, and others:

include/uapi/linux/wireless.h:751:33: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] include/uapi/linux/wireless.h:776:25: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] include/uapi/linux/wireless.h:833:25: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] include/uapi/linux/wireless.h:857:25: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] include/uapi/linux/wireless.h:864:25: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] include/uapi/linux/route.h:33:25: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] include/uapi/linux/route.h:34:25: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] include/uapi/linux/route.h:35:25: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] include/uapi/linux/if_arp.h:118:25: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] include/uapi/linux/if_arp.h:119:25: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] include/uapi/linux/if_arp.h:121:25: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] include/uapi/linux/if_arp.h:126:25: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] include/uapi/linux/if_arp.h:127:25: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] include/net/compat.h:34:25: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] include/net/compat.h:35:25: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]

In order to fix the warnings above, we introduce struct sockaddr_legacy. The intention is to use it to replace the type of several struct members in the middle of composite structures, currently of type struct sockaddr.

These middle struct members are currently causing thousands of warnings because struct sockaddr contains a flexible-array member, introduced by commit b5f0de6df6dce ("net: dev: Convert sa_data to flexible array in struct sockaddr").

The new struct sockaddr_legacy doesn't include a flexible-array member, making it suitable for use as the type of middle members in composite structs that don't really require the flexible-array member in struct sockaddr, thus avoiding -Wflex-array-member-not-at-end warnings.

As this new struct will live in UAPI, to avoid breaking user-space code that expects struct sockaddr, the __kernel_sockaddr_legacy macro is introduced. This macro allows us to use either struct sockaddr or struct sockaddr_legacy depending on the context in which the code is
used: kernel-space or user-space.

---
 include/uapi/linux/socket.h | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)
include/uapi/linux/socket.h +28 -0
index d3fcd3b5ec53..2e179706bec4 100644
@@ -35,4 +35,32 @@ struct __kernel_sockaddr_storage {
 #define SOCK_TXREHASH_DISABLED	0
 #define SOCK_TXREHASH_ENABLED	1
 
+typedef __kernel_sa_family_t    sa_family_t;
+
+/*
+ * This is the legacy form of `struct sockaddr`. The original `struct sockaddr`
+ * was modified in commit b5f0de6df6dce ("net: dev: Convert sa_data to flexible
+ * array in struct sockaddr") due to the fact that "One of the worst offenders
+ * of "fake flexible arrays" is struct sockaddr". This means that the original
+ * `char sa_data[14]` behaved as a flexible array at runtime, so a proper
+ * flexible-array member was introduced.
+ *
+ * This caused several flexible-array-in-the-middle issues:
+ * https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wflex-array-member-not-at-end
+ *
+ * `struct sockaddr_legacy` replaces `struct sockaddr` in all instances where
+ * objects of this type do not appear at the end of composite structures.
+ */
+struct sockaddr_legacy {
+        sa_family_t     sa_family;      /* address family, AF_xxx       */
+        char            sa_data[14];    /* 14 bytes of protocol address */
+};
+
+#ifdef __KERNEL__
+#	define __kernel_sockaddr_legacy		sockaddr_legacy
+#else
+#	define __kernel_sockaddr_legacy		sockaddr
+#endif
+
+
 #endif /* _UAPI_LINUX_SOCKET_H */
Signature · 1 line
2.34.1
#2 lore raw

Re: [PATCH v2 1/4][next] uapi: socket: Introduce struct sockaddr_legacy

Quoted 5 lines

As this new struct will live in UAPI, to avoid breaking user-space code that expects struct sockaddr, the __kernel_sockaddr_legacy macro is introduced. This macro allows us to use either struct sockaddr or struct sockaddr_legacy depending on the context in which the code is
used: kernel-space or user-space.

Are there cases of userspace API structures where the flexiable array appears in the middle? I assume this new compiler flag is not only for use in the kernel? When it gets turned on in user space, will the kernel headers will again produce warnings? Should we be considering allowing user space to opt in to using sockaddr_legacy?

Andrew

maintainer
#3 lore raw

Re: [PATCH v2 1/4][next] uapi: socket: Introduce struct sockaddr_legacy

Quoted 8 lines · Andrew Lunn
Quoted 5 lines

As this new struct will live in UAPI, to avoid breaking user-space code that expects struct sockaddr, the __kernel_sockaddr_legacy macro is introduced. This macro allows us to use either struct sockaddr or struct sockaddr_legacy depending on the context in which the code is
used: kernel-space or user-space.

Are there cases of userspace API structures where the flexiable array appears in the middle?

Clearly, it's the case for all the three other patches in this series.

Quoted 4 lines

I assume this new compiler flag is not only for
use in the kernel? When it gets turned on in user space, will the kernel headers will again produce warnings? Should we be considering allowing user space to opt in to using sockaddr_legacy?

For the userspace covered by patch 2 this will almost certainly never happen, and I suspect that might also be true for the others (arp and rtnetlink ioctls)? But it probably wouldn't be difficult either.

johannes

#4 lore raw

Re: [PATCH v2 1/4][next] uapi: socket: Introduce struct sockaddr_legacy

Quoted 11 lines · Johannes Berg
Quoted 8 lines · Andrew Lunn
Quoted 5 lines

As this new struct will live in UAPI, to avoid breaking user-space code that expects struct sockaddr, the __kernel_sockaddr_legacy macro is introduced. This macro allows us to use either struct sockaddr or struct sockaddr_legacy depending on the context in which the code is
used: kernel-space or user-space.

Are there cases of userspace API structures where the flexiable array appears in the middle?

Clearly, it's the case for all the three other patches in this series.

The issue is that the kernel uses these structures, and the kernel's view of sockaddr is that it (correctly) has a flexible array. Userspace's view of sockaddr is the old struct (which comes from the libc, not the kernel) which ends with a fake flexible array. We need to correct the kernel's view of these structures to use the introduced legacy struct to avoid lying to the compiler about what's going on. :)

Signature · 1 line
Kees Cook
#5 lore raw

Re: [PATCH v2 1/4][next] uapi: socket: Introduce struct sockaddr_legacy

In reply to Andrew Lunn
Quoted 11 lines · Andrew Lunn
Quoted 5 lines

As this new struct will live in UAPI, to avoid breaking user-space code that expects struct sockaddr, the __kernel_sockaddr_legacy macro is introduced. This macro allows us to use either struct sockaddr or struct sockaddr_legacy depending on the context in which the code is
used: kernel-space or user-space.

Are there cases of userspace API structures where the flexiable array appears in the middle? I assume this new compiler flag is not only for use in the kernel? When it gets turned on in user space, will the kernel headers will again produce warnings? Should we be considering allowing user space to opt in to using sockaddr_legacy?

I expect that the userspace usage of -Wflex-array-member-not-at-end will be driven by the libc projects, as it'll need to happen there before it can be done anywhere else. We'll be able to coordinate with them at that time, but I'm not aware of any plans by any libc to use this flag yet.

Signature · 1 line
Kees Cook
Reviewed-by on v2 1/4
#6 lore raw

Re: [PATCH v2 1/4][next] uapi: socket: Introduce struct sockaddr_legacy

In reply to Gustavo A. R. Silva
Quoted 24 lines · Gustavo A. R. Silva
include/uapi/linux/socket.h +16 -0
index d3fcd3b5ec53..2e179706bec4 100644
@@ -35,4 +35,32 @@ struct __kernel_sockaddr_storage {
 #define SOCK_TXREHASH_DISABLED	0
 #define SOCK_TXREHASH_ENABLED	1
 
+typedef __kernel_sa_family_t    sa_family_t;
+
+/*
+ * This is the legacy form of `struct sockaddr`. The original `struct sockaddr`
+ * was modified in commit b5f0de6df6dce ("net: dev: Convert sa_data to flexible
+ * array in struct sockaddr") due to the fact that "One of the worst offenders
+ * of "fake flexible arrays" is struct sockaddr". This means that the original
+ * `char sa_data[14]` behaved as a flexible array at runtime, so a proper
+ * flexible-array member was introduced.
+ *
+ * This caused several flexible-array-in-the-middle issues:
+ * https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wflex-array-member-not-at-end
+ *
+ * `struct sockaddr_legacy` replaces `struct sockaddr` in all instances where
+ * objects of this type do not appear at the end of composite structures.
+ */

include/uapi/linux/socket.h struct __kernel_sockaddr_storage { · line 63

Quoted 10 lines
include/uapi/linux/socket.h +10 -0
+struct sockaddr_legacy {
+        sa_family_t     sa_family;      /* address family, AF_xxx       */
+        char            sa_data[14];    /* 14 bytes of protocol address */
+};
+
+#ifdef __KERNEL__
+#	define __kernel_sockaddr_legacy		sockaddr_legacy
+#else
+#	define __kernel_sockaddr_legacy		sockaddr
+#endif

Yeah, this matches what I'd expect.

Signature · 1 line
Kees Cook
maintainer
#7 lore raw

Re: [PATCH v2 1/4][next] uapi: socket: Introduce struct sockaddr_legacy

In reply to Gustavo A. R. Silva
Quoted 6 lines · On Thu, 24 Oct 2024 15:11:24 -0600 Gustavo A. R. Silva wrote:

+ * This is the legacy form of struct sockaddr. The original struct sockaddr
+ * was modified in commit b5f0de6df6dce ("net: dev: Convert sa_data to flexible
+ * array in struct sockaddr") due to the fact that "One of the worst offenders
+ * of "fake flexible arrays" is struct sockaddr". This means that the original
+ * char sa_data[14] behaved as a flexible array at runtime, so a proper
+ * flexible-array member was introduced.

This isn't spelled out in the commit messages AFACT so let me ask.. Why aren't we reverting b5f0de6df6dce, then?
Feels like the best solution would be to have a separate type with the flex array to clearly annotate users who treat it as such. Is that not going to work?

My noob reading of b5f0de6df6dce is that it was a simpler workaround for the previous problem, avoided adding a new type (and the conversion churn). But now we are adding a type and another workaround on top. Sorry if I'm misunderstanding. No question that the struct is a mess, but I don't feel like this is helping the messiness...

#8 lore raw

Re: [PATCH v2 1/4][next] uapi: socket: Introduce struct sockaddr_legacy

Quoted 14 lines · Jakub Kicinski
Quoted 6 lines · On Thu, 24 Oct 2024 15:11:24 -0600 Gustavo A. R. Silva wrote:

+ * This is the legacy form of struct sockaddr. The original struct sockaddr
+ * was modified in commit b5f0de6df6dce ("net: dev: Convert sa_data to flexible
+ * array in struct sockaddr") due to the fact that "One of the worst offenders
+ * of "fake flexible arrays" is struct sockaddr". This means that the original
+ * char sa_data[14] behaved as a flexible array at runtime, so a proper
+ * flexible-array member was introduced.

This isn't spelled out in the commit messages AFACT so let me ask.. Why aren't we reverting b5f0de6df6dce, then?
Feels like the best solution would be to have a separate type with the flex array to clearly annotate users who treat it as such. Is that not going to work?

Quoted 5 lines

My noob reading of b5f0de6df6dce is that it was a simpler workaround for the previous problem, avoided adding a new type (and the conversion churn). But now we are adding a type and another workaround on top. Sorry if I'm misunderstanding. No question that the struct is a mess, but I don't feel like this is helping the messiness...

This is a pretty reasonable position. I think sockaddr turns out to be a really difficult problem, given its exposure to UAPI. Linux has been trying to deal with it for a while (e.g. sockaddr_storage), and perhaps b5f0de6df6dce wasn't the right solution. (It looked correct since it looks like what we've done in many other tricky places, but perhaps sockaddr should be dealt with differently yet.)

So, the rationale with b5f0de6df6dce was to change things as minimally as possible. The goal is to stop lying to the compiler about object sizes, with the big lie being "this object ends with a 14 byte array".

I think this results in two primary goals:
1- make sockaddr-like objects unambiguously sized
2- do not break userspace

For 2, we cannot change the existing (and externally defined) struct sockaddr. It will stay the historical horrible thing that it is.

The specific UAPI sockaddr-related things we have that are _fixed_ sizes are all fine. These can be seen with:
git grep '^struct sockaddr_' -- include/uapi/

For 1, we cannot use (the externally defined) sockaddr as-is since we run into size problems. (Basically anything casting from sockaddr, anything copying out of sockaddr, etc.)

The storage problem for sockaddr has been attempted to be addressed with the internally defined sockaddr_storage, but this doesn't capture the "I don't know how big this object is yet" issues associated with taking a buffer of arbitrary size from userspace that is initially defined as sockaddr.

If we internally add struct sockaddr_unknown, or _bytes, or _unspec, or _flex, or _array, or _raw, we can use that everywhere in the kernel, but it is going to cause a lot of churn. This is why b5f0de6df6dce happened, and why we went the direction of thinking sockaddr_legacy would be
workable: the kernel's view of sockaddr from userspace (sockaddr_legacy) would be the only change needed.

Now, if we want to get to a place with the least ambiguity, we need to abolish sockaddr from the kernel internally, and I think that might take a while. I only think so because of what I went through just for doing a portion of NFS in cf0d7e7f4520 ("NFS: Avoid memcpy() run-time warning for struct sockaddr overflows").

But maybe it won't be as bad all that since we already do a lot of "what is the family then cast to a fixed-size struct and carry on" stuff. But the passing things between layers (usually "up" from a family into the networking core) still depends on using sockaddr, and all of those APIs would need to switch to sockaddr_unknown both in the core and in all families. Maybe there is some Coccinelle magic that could be worked.

I think it's worth spending the time to try and figure out the size of the effort needed to make that change.

-Kees

Signature · 1 line
Kees Cook
#9 lore raw

PATCH v2 2/4 uapi: wireless: Avoid -Wflex-array-member-not-at-end warnings

In reply to Gustavo A. R. Silva

-Wflex-array-member-not-at-end was introduced in GCC-14, and we are getting ready to enable it, globally.

Address the following warnings by changing the type of the middle struct members in various composite structs, which are currently causing trouble, from struct sockaddr to struct __kernel_sockaddr_legacy.

include/uapi/linux/wireless.h:751:33: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] include/uapi/linux/wireless.h:776:25: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] include/uapi/linux/wireless.h:833:25: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] include/uapi/linux/wireless.h:857:25: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] include/uapi/linux/wireless.h:864:25: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]

---
 include/uapi/linux/wireless.h | 56 +++++++++++++++++------------------
 1 file changed, 28 insertions(+), 28 deletions(-)
include/uapi/linux/wireless.h +28 -28
index 3c2ad5fae17f..d8744113fc89 100644
@@ -748,10 +748,10 @@ struct iw_missed {
  *	Quality range (for spy threshold)
  */
 struct iw_thrspy {
-	struct sockaddr		addr;		/* Source address (hw/mac) */
-	struct iw_quality	qual;		/* Quality of the link */
-	struct iw_quality	low;		/* Low threshold */
-	struct iw_quality	high;		/* High threshold */
+	struct __kernel_sockaddr_legacy	addr;	/* Source address (hw/mac) */
+	struct iw_quality		qual;	/* Quality of the link */
+	struct iw_quality		low;	/* Low threshold */
+	struct iw_quality		high;	/* High threshold */
 };
 
 /*
@@ -766,15 +766,15 @@ struct iw_thrspy {
  *	current BSS if the driver is in Managed mode and associated with an AP.
  */
 struct iw_scan_req {
-	__u8		scan_type; /* IW_SCAN_TYPE_{ACTIVE,PASSIVE} */
-	__u8		essid_len;
-	__u8		num_channels; /* num entries in channel_list;
-				       * 0 = scan all allowed channels */
-	__u8		flags; /* reserved as padding; use zero, this may
-				* be used in the future for adding flags
-				* to request different scan behavior */
-	struct sockaddr	bssid; /* ff:ff:ff:ff:ff:ff for broadcast BSSID or
-				* individual address of a specific BSS */
+	__u8				scan_type; /* IW_SCAN_TYPE_{ACTIVE,PASSIVE} */
+	__u8				essid_len;
+	__u8				num_channels; /* num entries in channel_list;
+						       * 0 = scan all allowed channels */
+	__u8				flags; /* reserved as padding; use zero, this may
+						* be used in the future for adding flags
+						* to request different scan behavior */
+	struct __kernel_sockaddr_legacy	bssid; /* ff:ff:ff:ff:ff:ff for broadcast BSSID or
+						* individual address of a specific BSS */
 
 	/*
 	 * Use this ESSID if IW_SCAN_THIS_ESSID flag is used instead of using
@@ -827,15 +827,15 @@ struct iw_scan_req {
  *	debugging/testing.
  */
 struct iw_encode_ext {
-	__u32		ext_flags; /* IW_ENCODE_EXT_* */
-	__u8		tx_seq[IW_ENCODE_SEQ_MAX_SIZE]; /* LSB first */
-	__u8		rx_seq[IW_ENCODE_SEQ_MAX_SIZE]; /* LSB first */
-	struct sockaddr	addr; /* ff:ff:ff:ff:ff:ff for broadcast/multicast
-			       * (group) keys or unicast address for
-			       * individual keys */
-	__u16		alg; /* IW_ENCODE_ALG_* */
-	__u16		key_len;
-	__u8		key[];
+	__u32				ext_flags; /* IW_ENCODE_EXT_* */
+	__u8				tx_seq[IW_ENCODE_SEQ_MAX_SIZE]; /* LSB first */
+	__u8				rx_seq[IW_ENCODE_SEQ_MAX_SIZE]; /* LSB first */
+	struct __kernel_sockaddr_legacy	addr; /* ff:ff:ff:ff:ff:ff for broadcast/multicast
+					       * (group) keys or unicast address for
+					       * individual keys */
+	__u16				alg; /* IW_ENCODE_ALG_* */
+	__u16				key_len;
+	__u8				key[];
 };
 
 /* SIOCSIWMLME data */
@@ -853,16 +853,16 @@ struct iw_mlme {
 #define IW_PMKID_LEN	16
 
 struct iw_pmksa {
-	__u32		cmd; /* IW_PMKSA_* */
-	struct sockaddr	bssid;
-	__u8		pmkid[IW_PMKID_LEN];
+	__u32				cmd; /* IW_PMKSA_* */
+	struct __kernel_sockaddr_legacy	bssid;
+	__u8				pmkid[IW_PMKID_LEN];
 };
 
 /* IWEVMICHAELMICFAILURE data */
 struct iw_michaelmicfailure {
-	__u32		flags;
-	struct sockaddr	src_addr;
-	__u8		tsc[IW_ENCODE_SEQ_MAX_SIZE]; /* LSB first */
+	__u32				flags;
+	struct __kernel_sockaddr_legacy	src_addr;
+	__u8				tsc[IW_ENCODE_SEQ_MAX_SIZE]; /* LSB first */
 };
 
 /* IWEVPMKIDCAND data */
Signature · 1 line
2.34.1
Reviewed-by on v2 2/4
#10 lore raw

Re: [PATCH v2 2/4][next] uapi: wireless: Avoid -Wflex-array-member-not-at-end warnings

Quoted 7 lines · Gustavo A. R. Silva

-Wflex-array-member-not-at-end was introduced in GCC-14, and we are getting ready to enable it, globally.

Address the following warnings by changing the type of the middle struct members in various composite structs, which are currently causing trouble, from struct sockaddr to struct __kernel_sockaddr_legacy.

Quoted 7 lines

include/uapi/linux/wireless.h:751:33: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] include/uapi/linux/wireless.h:776:25: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] include/uapi/linux/wireless.h:833:25: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] include/uapi/linux/wireless.h:857:25: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] include/uapi/linux/wireless.h:864:25: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]

It's a little weird to have to update all the whitespace, but yeah, seems correct.

Signature · 1 line
Kees Cook
#11 lore raw

PATCH v2 3/4 uapi: net: arp: Avoid -Wflex-array-member-not-at-end warnings

In reply to Gustavo A. R. Silva

-Wflex-array-member-not-at-end was introduced in GCC-14, and we are getting ready to enable it, globally.

Address the following warnings by changing the type of the middle struct members in a couple of composite structs, which are currently causing trouble, from struct sockaddr to struct __kernel_sockaddr_legacy.

include/uapi/linux/if_arp.h:118:25: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] include/uapi/linux/if_arp.h:119:25: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] include/uapi/linux/if_arp.h:121:25: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] include/uapi/linux/if_arp.h:126:25: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] include/uapi/linux/if_arp.h:127:25: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]

Also, refactor some related code, accordingly.

---
 include/uapi/linux/if_arp.h | 18 +++++++++---------
 net/ipv4/arp.c              |  2 +-
 2 files changed, 10 insertions(+), 10 deletions(-)
include/uapi/linux/if_arp.h +9 -9
index 4783af9fe520..4a25a05125d3 100644
@@ -115,18 +115,18 @@
 
 /* ARP ioctl request. */
 struct arpreq {
-	struct sockaddr	arp_pa;		/* protocol address		 */
-	struct sockaddr	arp_ha;		/* hardware address		 */
-	int		arp_flags;	/* flags			 */
-	struct sockaddr arp_netmask;    /* netmask (only for proxy arps) */
-	char		arp_dev[IFNAMSIZ];
+	struct __kernel_sockaddr_legacy	arp_pa;		/* protocol address		 */
+	struct __kernel_sockaddr_legacy	arp_ha;		/* hardware address		 */
+	int				arp_flags;	/* flags			 */
+	struct __kernel_sockaddr_legacy	arp_netmask;    /* netmask (only for proxy arps) */
+	char				arp_dev[IFNAMSIZ];
 };
 
 struct arpreq_old {
-	struct sockaddr	arp_pa;		/* protocol address		 */
-	struct sockaddr	arp_ha;		/* hardware address		 */
-	int		arp_flags;	/* flags			 */
-	struct sockaddr	arp_netmask;    /* netmask (only for proxy arps) */
+	struct __kernel_sockaddr_legacy	arp_pa;		/* protocol address		 */
+	struct __kernel_sockaddr_legacy	arp_ha;		/* hardware address		 */
+	int				arp_flags;	/* flags			 */
+	struct sockaddr			arp_netmask;    /* netmask (only for proxy arps) */
 };
 
 /* ARP Flag values. */
net/ipv4/arp.c +1 -1
index 11c1519b3699..3a97efe1587b 100644
@@ -1185,7 +1185,7 @@ static int arp_req_get(struct net *net, struct arpreq *r)
 
 	read_lock_bh(&neigh->lock);
 	memcpy(r->arp_ha.sa_data, neigh->ha,
-	       min(dev->addr_len, sizeof(r->arp_ha.sa_data_min)));
+	       min(dev->addr_len, sizeof(r->arp_ha.sa_data)));
 	r->arp_flags = arp_state_to_flags(neigh);
 	read_unlock_bh(&neigh->lock);
 
Signature · 1 line
2.34.1
Reviewed-by on v2 3/4
#12 lore raw

Re: [PATCH v2 3/4][next] uapi: net: arp: Avoid -Wflex-array-member-not-at-end warnings

Quoted 13 lines · Gustavo A. R. Silva

-Wflex-array-member-not-at-end was introduced in GCC-14, and we are getting ready to enable it, globally.

Address the following warnings by changing the type of the middle struct members in a couple of composite structs, which are currently causing trouble, from struct sockaddr to struct __kernel_sockaddr_legacy.

include/uapi/linux/if_arp.h:118:25: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] include/uapi/linux/if_arp.h:119:25: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] include/uapi/linux/if_arp.h:121:25: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] include/uapi/linux/if_arp.h:126:25: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] include/uapi/linux/if_arp.h:127:25: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]

Quoted 3 lines

Also, refactor some related code, accordingly.

Signature · 1 line
Kees Cook
#13 lore raw

PATCH v2 4/4 uapi: net: Avoid -Wflex-array-member-not-at-end warnings

In reply to Gustavo A. R. Silva

-Wflex-array-member-not-at-end was introduced in GCC-14, and we are getting ready to enable it, globally.

Address the following warnings by changing the type of the middle struct members in a couple of composite structs, which are currently causing trouble, from struct sockaddr to struct __kernel_sockaddr_legacy in UAPI, and struct sockaddr_legacy for the rest of the kernel code.

include/uapi/linux/route.h:33:25: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] include/uapi/linux/route.h:34:25: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] include/uapi/linux/route.h:35:25: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end include/net/compat.h:34:25: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] include/net/compat.h:35:25: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]

Also, update some related code, accordingly.

---
 include/net/compat.h       | 30 +++++++++++++++---------------
 include/uapi/linux/route.h | 28 ++++++++++++++--------------
 net/appletalk/ddp.c        |  2 +-
 net/ipv4/af_inet.c         |  2 +-
 net/ipv4/fib_frontend.c    |  2 +-
 5 files changed, 32 insertions(+), 32 deletions(-)
include/net/compat.h +15 -15
index 84c163f40f38..89e891d8dcf3 100644
@@ -29,21 +29,21 @@ struct compat_cmsghdr {
 };
 
 struct compat_rtentry {
-	u32		rt_pad1;
-	struct sockaddr rt_dst;         /* target address               */
-	struct sockaddr rt_gateway;     /* gateway addr (RTF_GATEWAY)   */
-	struct sockaddr rt_genmask;     /* target network mask (IP)     */
-	unsigned short	rt_flags;
-	short		rt_pad2;
-	u32		rt_pad3;
-	unsigned char	rt_tos;
-	unsigned char	rt_class;
-	short		rt_pad4;
-	short		rt_metric;      /* +1 for binary compatibility! */
-	compat_uptr_t	rt_dev;         /* forcing the device at add    */
-	u32		rt_mtu;         /* per route MTU/Window         */
-	u32		rt_window;      /* Window clamping              */
-	unsigned short  rt_irtt;        /* Initial RTT                  */
+	u32			rt_pad1;
+	struct sockaddr_legacy	rt_dst;         /* target address               */
+	struct sockaddr_legacy	rt_gateway;     /* gateway addr (RTF_GATEWAY)   */
+	struct sockaddr_legacy	rt_genmask;     /* target network mask (IP)     */
+	unsigned short		rt_flags;
+	short			rt_pad2;
+	u32			rt_pad3;
+	unsigned char		rt_tos;
+	unsigned char		rt_class;
+	short			rt_pad4;
+	short			rt_metric;      /* +1 for binary compatibility! */
+	compat_uptr_t		rt_dev;         /* forcing the device at add    */
+	u32			rt_mtu;         /* per route MTU/Window         */
+	u32			rt_window;      /* Window clamping              */
+	unsigned short		rt_irtt;        /* Initial RTT                  */
 };
 
 int __get_compat_msghdr(struct msghdr *kmsg, struct compat_msghdr *msg,
include/uapi/linux/route.h +14 -14
index a0de9a7331a2..43fd79f90a47 100644
@@ -29,22 +29,22 @@
 
 /* This structure gets passed by the SIOCADDRT and SIOCDELRT calls. */
 struct rtentry {
-	unsigned long	rt_pad1;
-	struct sockaddr	rt_dst;		/* target address		*/
-	struct sockaddr	rt_gateway;	/* gateway addr (RTF_GATEWAY)	*/
-	struct sockaddr	rt_genmask;	/* target network mask (IP)	*/
-	unsigned short	rt_flags;
-	short		rt_pad2;
-	unsigned long	rt_pad3;
-	void		*rt_pad4;
-	short		rt_metric;	/* +1 for binary compatibility!	*/
-	char __user	*rt_dev;	/* forcing the device at add	*/
-	unsigned long	rt_mtu;		/* per route MTU/Window 	*/
+	unsigned long			rt_pad1;
+	struct __kernel_sockaddr_legacy	rt_dst;		/* target address		*/
+	struct __kernel_sockaddr_legacy	rt_gateway;	/* gateway addr (RTF_GATEWAY)	*/
+	struct __kernel_sockaddr_legacy	rt_genmask;	/* target network mask (IP)	*/
+	unsigned short			rt_flags;
+	short				rt_pad2;
+	unsigned long			rt_pad3;
+	void				*rt_pad4;
+	short				rt_metric;	/* +1 for binary compatibility!	*/
+	char __user			*rt_dev;	/* forcing the device at add	*/
+	unsigned long			rt_mtu;		/* per route MTU/Window		*/
 #ifndef __KERNEL__
-#define rt_mss	rt_mtu			/* Compatibility :-(            */
+#define rt_mss	rt_mtu					/* Compatibility :-(            */
 #endif
-	unsigned long	rt_window;	/* Window clamping 		*/
-	unsigned short	rt_irtt;	/* Initial RTT			*/
+	unsigned long			rt_window;	/* Window clamping		*/
+	unsigned short			rt_irtt;	/* Initial RTT			*/
 };
 
 
net/appletalk/ddp.c +1 -1
index b068651984fe..aac82a4af36f 100644
@@ -1832,7 +1832,7 @@ static int atalk_compat_routing_ioctl(struct sock *sk, unsigned int cmd,
 	struct rtentry rt;
 
 	if (copy_from_user(&rt.rt_dst, &ur->rt_dst,
-			3 * sizeof(struct sockaddr)) ||
+			3 * sizeof(struct sockaddr_legacy)) ||
 	    get_user(rt.rt_flags, &ur->rt_flags) ||
 	    get_user(rt.rt_metric, &ur->rt_metric) ||
 	    get_user(rt.rt_mtu, &ur->rt_mtu) ||
net/ipv4/af_inet.c +1 -1
index 8095e82de808..3beb52261b4b 100644
@@ -1019,7 +1019,7 @@ static int inet_compat_routing_ioctl(struct sock *sk, unsigned int cmd,
 	struct rtentry rt;
 
 	if (copy_from_user(&rt.rt_dst, &ur->rt_dst,
-			3 * sizeof(struct sockaddr)) ||
+			3 * sizeof(struct sockaddr_legacy)) ||
 	    get_user(rt.rt_flags, &ur->rt_flags) ||
 	    get_user(rt.rt_metric, &ur->rt_metric) ||
 	    get_user(rt.rt_mtu, &ur->rt_mtu) ||
net/ipv4/fib_frontend.c +1 -1
index 53bd26315df5..88c7a79946f2 100644
@@ -452,7 +452,7 @@ int fib_validate_source(struct sk_buff *skb, __be32 src, __be32 dst,
 				     itag);
 }
 
-static inline __be32 sk_extract_addr(struct sockaddr *addr)
+static inline __be32 sk_extract_addr(struct sockaddr_legacy *addr)
 {
 	return ((struct sockaddr_in *) addr)->sin_addr.s_addr;
 }
Signature · 1 line
2.34.1
Reviewed-by on v2 4/4
#14 lore raw

Re: [PATCH v2 4/4][next] uapi: net: Avoid -Wflex-array-member-not-at-end warnings

Quoted 14 lines · Gustavo A. R. Silva

-Wflex-array-member-not-at-end was introduced in GCC-14, and we are getting ready to enable it, globally.

Address the following warnings by changing the type of the middle struct members in a couple of composite structs, which are currently causing trouble, from struct sockaddr to struct __kernel_sockaddr_legacy in UAPI, and struct sockaddr_legacy for the rest of the kernel code.

include/uapi/linux/route.h:33:25: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] include/uapi/linux/route.h:34:25: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] include/uapi/linux/route.h:35:25: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end include/net/compat.h:34:25: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] include/net/compat.h:35:25: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]

Quoted 3 lines

Also, update some related code, accordingly.

Looks right, including the helper prototype update.

Signature · 1 line
Kees Cook