[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 4/5][next] uapi: net: arp: 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 a couple of 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/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, update some related code, accordingly.

No binary differences are present after these changes.

Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
---
 include/uapi/linux/if_arp.h | 18 +++++++++---------
 net/ipv4/arp.c              |  2 +-
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/include/uapi/linux/if_arp.h b/include/uapi/linux/if_arp.h
index 4783af9fe520..cb6813f7783a 100644
--- a/include/uapi/linux/if_arp.h
+++ b/include/uapi/linux/if_arp.h
@@ -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 sockaddr_legacy	arp_pa;		/* protocol address		 */
+	struct sockaddr_legacy	arp_ha;		/* hardware address		 */
+	int			arp_flags;	/* flags			 */
+	struct 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 sockaddr_legacy	arp_pa;		/* protocol address		 */
+	struct sockaddr_legacy	arp_ha;		/* hardware address		 */
+	int			arp_flags;	/* flags			 */
+	struct sockaddr		arp_netmask;    /* netmask (only for proxy arps) */
 };
 
 /* ARP Flag values. */
diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index 11c1519b3699..3a97efe1587b 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -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);
 
-- 
2.34.1

[PATCH 5/5][next] uapi: net: 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 a couple of 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/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.

Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
---
 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(-)

diff --git a/include/net/compat.h b/include/net/compat.h
index 84c163f40f38..89e891d8dcf3 100644
--- a/include/net/compat.h
+++ b/include/net/compat.h
@@ -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,
diff --git a/include/uapi/linux/route.h b/include/uapi/linux/route.h
index a0de9a7331a2..7e43765e03dd 100644
--- a/include/uapi/linux/route.h
+++ b/include/uapi/linux/route.h
@@ -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 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;
+	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			*/
 };
 
 
diff --git a/net/appletalk/ddp.c b/net/appletalk/ddp.c
index b068651984fe..aac82a4af36f 100644
--- a/net/appletalk/ddp.c
+++ b/net/appletalk/ddp.c
@@ -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) ||
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index b24d74616637..75bd15d884e3 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1021,7 +1021,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) ||
diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
index 8353518b110a..595b9ac58e92 100644
--- a/net/ipv4/fib_frontend.c
+++ b/net/ipv4/fib_frontend.c
@@ -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;
 }
-- 
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 4/5][next] uapi: net: arp: Avoid -Wflex-array-member-not-at-end warnings

[Kuniyuki Iwashima]

From: "Gustavo A. R. Silva" <gustavoars@kernel.org>
Date: Tue, 15 Oct 2024 18:32:43 -0600
> -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 sockaddr_legacy`. Note that
> the latter struct doesn't contain a flexible-array member.
> 
> 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, update some related code, accordingly.
> 
> No binary differences are present after these changes.
> 
> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> ---
>  include/uapi/linux/if_arp.h | 18 +++++++++---------
>  net/ipv4/arp.c              |  2 +-
>  2 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/include/uapi/linux/if_arp.h b/include/uapi/linux/if_arp.h
> index 4783af9fe520..cb6813f7783a 100644
> --- a/include/uapi/linux/if_arp.h
> +++ b/include/uapi/linux/if_arp.h
> @@ -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 sockaddr_legacy	arp_pa;		/* protocol address		 */
> +	struct sockaddr_legacy	arp_ha;		/* hardware address		 */
> +	int			arp_flags;	/* flags			 */
> +	struct 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 sockaddr_legacy	arp_pa;		/* protocol address		 */
> +	struct sockaddr_legacy	arp_ha;		/* hardware address		 */
> +	int			arp_flags;	/* flags			 */
> +	struct sockaddr		arp_netmask;    /* netmask (only for proxy arps) */

I think we can use _legacy here too, 14 bytes are enough for ARP.

But whichever is fine to me because arpreq_old is not used in
kernel, so

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


>  };
>  
>  /* ARP Flag values. */
> diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
> index 11c1519b3699..3a97efe1587b 100644
> --- a/net/ipv4/arp.c
> +++ b/net/ipv4/arp.c
> @@ -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);
>  
> -- 
> 2.34.1

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

[Kuniyuki Iwashima]

From: "Gustavo A. R. Silva" <gustavoars@kernel.org>
Date: Tue, 15 Oct 2024 18:33:23 -0600
> diff --git a/net/appletalk/ddp.c b/net/appletalk/ddp.c
> index b068651984fe..aac82a4af36f 100644
> --- a/net/appletalk/ddp.c
> +++ b/net/appletalk/ddp.c
> @@ -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)) ||

While at it, please fix the indent.


>  	    get_user(rt.rt_flags, &ur->rt_flags) ||
>  	    get_user(rt.rt_metric, &ur->rt_metric) ||
>  	    get_user(rt.rt_mtu, &ur->rt_mtu) ||
> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> index b24d74616637..75bd15d884e3 100644
> --- a/net/ipv4/af_inet.c
> +++ b/net/ipv4/af_inet.c
> @@ -1021,7 +1021,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)) ||

Same here.

Otherwise looks good to me.

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


>  	    get_user(rt.rt_flags, &ur->rt_flags) ||
>  	    get_user(rt.rt_metric, &ur->rt_metric) ||
>  	    get_user(rt.rt_mtu, &ur->rt_mtu) ||

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

[Andrew Lunn]

On Tue, Oct 15, 2024 at 06:32:43PM -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 a couple of 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/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, update some related code, accordingly.
> 
> No binary differences are present after these changes.

These are clearly UAPI files. It would be good to state in the commit
message why this is a safe change, at the source level.

Could user space code expect the type struct sockaddr and we are going
to get warnings when struct sockaddr_legacy is found? Have you tried
compiling an old arp binary, one that uses the IOCTL? The netfilter
code also references it:

http://charette.no-ip.com:81/programming/doxygen/netfilter/structarpreq.html

You also need to submit a patch to the man page:

https://man7.org/linux/man-pages/man7/arp.7.html

You might also want to build some Rust code:

https://docs.diesel.rs/master/libc/struct.arpreq.html

I've no idea what Rust does when a C structure has a type change.

	Andrew

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

[Kees Cook]

On Wed, Oct 16, 2024 at 02:30:02PM +0200, Andrew Lunn wrote:
> On Tue, Oct 15, 2024 at 06:32:43PM -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 a couple of 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/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, update some related code, accordingly.
> > 
> > No binary differences are present after these changes.
> 
> These are clearly UAPI files. It would be good to state in the commit
> message why this is a safe change, at the source level.

I think we can avoid complicating UAPI by doing something like this in
include/uapi/linux/socket.h:

#ifdef __KERNEL__
#define __kernel_sockaddr_legacy        sockaddr_legacy
#else
#define __kernel_sockaddr_legacy        sockaddr
#endif

And then the UAPI changes can use __kernel_sockaddr_legacy and userspace
will resolve to sockaddr (unchanged), and the kernel internals will
resolve to sockaddr_legacy (fixing the warnings).

-Kees

-- 
Kees Cook

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

[Gustavo A. R. Silva]

>> These are clearly UAPI files. It would be good to state in the commit
>> message why this is a safe change, at the source level.

Yes, I'll update it!

> 
> I think we can avoid complicating UAPI by doing something like this in
> include/uapi/linux/socket.h:
> 
> #ifdef __KERNEL__
> #define __kernel_sockaddr_legacy        sockaddr_legacy
> #else
> #define __kernel_sockaddr_legacy        sockaddr
> #endif
> 
> And then the UAPI changes can use __kernel_sockaddr_legacy and userspace
> will resolve to sockaddr (unchanged), and the kernel internals will
> resolve to sockaddr_legacy (fixing the warnings).

Here are a couple of test patches (Don't mind the changelog text):

https://git.kernel.org/pub/scm/linux/kernel/git/gustavoars/linux.git/commit/?h=testing/wfamnae-next20241015-2&id=c3b631a5036cbf45b3308d563bf74a518490f3e6
https://git.kernel.org/pub/scm/linux/kernel/git/gustavoars/linux.git/commit/?h=testing/wfamnae-next20241015-2&id=66db096a530b95ce0ac33f9fdec66401ec5f2204

__kernel_sockaddr_legacy seems a bit too long, but at the same time
it makes it quite clear what's going on.

Thanks
--
Gustavo

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)
>