[PATCH 0/5][next] net: Avoid thousands of -Wflex-array-member-not-at-end warnings

[PATCH 0/5][next] net: Avoid thousands of -Wflex-array-member-not-at-end warnings

[Gustavo A. R. Silva]

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.

Gustavo A. R. Silva (5):
  net: dev: Introduce struct sockaddr_legacy
  nfsd: avoid -Wflex-array-member-not-at-end warnings
  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

 fs/nfsd/nfsctl.c              |  4 +--
 fs/nfsd/nfsd.h                |  4 +--
 include/linux/socket.h        | 19 +++++++++++++
 include/net/compat.h          | 30 ++++++++++-----------
 include/uapi/linux/if_arp.h   | 18 ++++++-------
 include/uapi/linux/route.h    | 28 ++++++++++----------
 include/uapi/linux/wireless.h | 50 +++++++++++++++++------------------
 net/appletalk/ddp.c           |  2 +-
 net/ipv4/af_inet.c            |  2 +-
 net/ipv4/arp.c                |  2 +-
 net/ipv4/fib_frontend.c       |  2 +-
 11 files changed, 90 insertions(+), 71 deletions(-)

-- 
2.34.1

[PATCH 1/5][next] net: dev: Introduce struct sockaddr_legacy

[Gustavo A. R. Silva]

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]
fs/nfsd/nfsd.h:74:33: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
fs/nfsd/nfsd.h:75:33: 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.

Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
---
 include/linux/socket.h | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/include/linux/socket.h b/include/linux/socket.h
index d18cc47e89bd..f370ae0e6c82 100644
--- a/include/linux/socket.h
+++ b/include/linux/socket.h
@@ -40,6 +40,25 @@ struct sockaddr {
 	};
 };
 
+/*
+ * 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	*/
+};
+
 struct linger {
 	int		l_onoff;	/* Linger active		*/
 	int		l_linger;	/* How long to linger for	*/
-- 
2.34.1

[PATCH 3/5][next] uapi: wireless: Avoid -Wflex-array-member-not-at-end warnings

[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 sockaddr_legacy`. Note that the latter
struct doesn't contain a flexible-array member.

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]

No binary differences are present after these changes.

Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
---
 include/uapi/linux/wireless.h | 50 +++++++++++++++++------------------
 1 file changed, 25 insertions(+), 25 deletions(-)

diff --git a/include/uapi/linux/wireless.h b/include/uapi/linux/wireless.h
index 3c2ad5fae17f..b29ab42fa2e2 100644
--- a/include/uapi/linux/wireless.h
+++ b/include/uapi/linux/wireless.h
@@ -748,7 +748,7 @@ struct iw_missed {
  *	Quality range (for spy threshold)
  */
 struct iw_thrspy {
-	struct sockaddr		addr;		/* Source address (hw/mac) */
+	struct 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 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 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 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 sockaddr_legacy	src_addr;
+	__u8			tsc[IW_ENCODE_SEQ_MAX_SIZE]; /* LSB first */
 };
 
 /* IWEVPMKIDCAND data */
-- 
2.34.1

Re: [PATCH 1/5][next] net: dev: Introduce struct sockaddr_legacy

[Kuniyuki Iwashima]

From: "Gustavo A. R. Silva" <gustavoars@kernel.org>
Date: Tue, 15 Oct 2024 18:27:16 -0600
> 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]
> fs/nfsd/nfsd.h:74:33: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
> fs/nfsd/nfsd.h:75:33: 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.
> 
> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> ---
>  include/linux/socket.h | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/include/linux/socket.h b/include/linux/socket.h
> index d18cc47e89bd..f370ae0e6c82 100644
> --- a/include/linux/socket.h
> +++ b/include/linux/socket.h
> @@ -40,6 +40,25 @@ struct sockaddr {
>  	};
>  };
>  
> +/*
> + * 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

s/sockaddR/sockaddr/

The same typo? exists in the cover letter.

With it fixed,

Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>


> + * 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	*/
> +};
> +
>  struct linger {
>  	int		l_onoff;	/* Linger active		*/
>  	int		l_linger;	/* How long to linger for	*/
> -- 
> 2.34.1

Re: [PATCH 3/5][next] uapi: wireless: Avoid -Wflex-array-member-not-at-end warnings

[Johannes Berg]

On Tue, 2024-10-15 at 18:31 -0600, Gustavo A. R. Silva wrote:
> -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 sockaddr_legacy`. Note that the latter
> struct doesn't contain a flexible-array member.
> 
> 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]
> 
> No binary differences are present after these changes.

I don't see how this works if you introduce "struct sockaddr_legacy" in
a non-UAPI header, but then use it in UAPI?!

Also, userspace might have pointers to it or whatnot, and warn/break if
you change the type?

johannes

RE: [PATCH 1/5][next] net: dev: Introduce struct sockaddr_legacy

[David Laight]

From: Gustavo A. R. Silva
> Sent: 16 October 2024 01:27
> 
> 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:
> 
...
> 
> 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.
> 
> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> ---
>  include/linux/socket.h | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/include/linux/socket.h b/include/linux/socket.h
> index d18cc47e89bd..f370ae0e6c82 100644
> --- a/include/linux/socket.h
> +++ b/include/linux/socket.h
> @@ -40,6 +40,25 @@ struct sockaddr {
>  	};
>  };
> 
> +/*
> + * 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

I'd bet that the code even indexed the array?
So it is all worse that just a compiler warning/

> + *
> + * `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	*/
> +};
> +

I'm not sure that is a very good name.
Reading it you don't know when it is 'legacy' from.
It's size is clearly that of the original IPv4 sockaddr.
(I'm not sure there was ever an earlier one.)

Perhaps 'strict sockaddr_16' would be better?
Or, looking at the actual failures, sockaddr_ipv4?

Alternatively revert b5f0de6df6dce and add a new type that has the char[]
field??

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH 1/5][next] net: dev: Introduce struct sockaddr_legacy

[Gustavo A. R. Silva]

>>   
>> +/*
>> + * 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
> 
> s/sockaddR/sockaddr/
> 
> The same typo? exists in the cover letter.

Thanks for catching this! :)

--
Gustavo

> 
> With it fixed,
> 
> Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>
> 
> 
>> + * 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	*/
>> +};
>> +
>>   struct linger {
>>   	int		l_onoff;	/* Linger active		*/
>>   	int		l_linger;	/* How long to linger for	*/
>> -- 
>> 2.34.1
> 

Re: [PATCH 1/5][next] net: dev: Introduce struct sockaddr_legacy

[Gustavo A. R. Silva]

On 22/10/24 06:13, David Laight wrote:
> From: Gustavo A. R. Silva
>> Sent: 16 October 2024 01:27
>>
>> 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:
>>
> ...
>>
>> 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.
>>
>> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
>> ---
>>   include/linux/socket.h | 19 +++++++++++++++++++
>>   1 file changed, 19 insertions(+)
>>
>> diff --git a/include/linux/socket.h b/include/linux/socket.h
>> index d18cc47e89bd..f370ae0e6c82 100644
>> --- a/include/linux/socket.h
>> +++ b/include/linux/socket.h
>> @@ -40,6 +40,25 @@ struct sockaddr {
>>   	};
>>   };
>>
>> +/*
>> + * 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
> 
> I'd bet that the code even indexed the array?
> So it is all worse that just a compiler warning/

I haven't found evidence of that, but this is precisely what we want to prevent
from happening. :)

> 
>> + *
>> + * `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	*/
>> +};
>> +
> 
> I'm not sure that is a very good name.
> Reading it you don't know when it is 'legacy' from.

Yep, naming is hard sometimes. This is why I added that long comment
above the struct. :)

However, this is changing and now it looks like this:

diff --git a/include/uapi/linux/socket.h b/include/uapi/linux/socket.h
index d3fcd3b5ec53d2..2e179706bec4d8 100644
--- a/include/uapi/linux/socket.h
+++ b/include/uapi/linux/socket.h
@@ -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 */

https://lore.kernel.org/linux-hardening/202410160942.000495E@keescook/

--
Gustavo

> It's size is clearly that of the original IPv4 sockaddr.
> (I'm not sure there was ever an earlier one.)
> 
> Perhaps 'strict sockaddr_16' would be better?
> Or, looking at the actual failures, sockaddr_ipv4?
> 
> Alternatively revert b5f0de6df6dce and add a new type that has the char[]
> field??
> 
> 	David
> 
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
>