[PATCH v4][next] wifi: iwlegacy: Avoid multiple -Wflex-array-member-not-at-end warnings

Gustavo A. R. Silva posted 1 patch 1 month ago
drivers/net/wireless/intel/iwlegacy/3945.h     | 4 +++-
drivers/net/wireless/intel/iwlegacy/4965-mac.c | 2 +-
drivers/net/wireless/intel/iwlegacy/commands.h | 9 +++++++--
drivers/net/wireless/intel/iwlegacy/common.h   | 4 +++-
4 files changed, 14 insertions(+), 5 deletions(-)
[PATCH v4][next] wifi: iwlegacy: Avoid multiple -Wflex-array-member-not-at-end warnings
Posted by Gustavo A. R. Silva 1 month ago
-Wflex-array-member-not-at-end was introduced in GCC-14, and we are
getting ready to enable it, globally.

Move the conflicting declarations (which in a couple of cases happen
to be in a union, so the entire unions are moved) to the end of the
corresponding structures, struct il_frame, and struct il3945_frame.

Notice that `struct il_tx_beacon_cmd`, `struct il4965_tx_resp`, and
`struct il3945_tx_beacon_cmd` are flexible structures, this is
structures that contain a flexible-array member.

The case for struct il4965_beacon_notif is different. Since this
structure is defined by hardware, we create the new `struct
il4965_tx_resp_hdr` type. We then use this newly created type to
replace the object type causing trouble in struct il4965_beacon_notif,
namely `struct il4965_tx_resp`.

Also, once -fms-extensions is enabled, we can use transparent struct
members in struct il4965_tx_resp.

Notice that the newly created type does not contain the flex-array
member `agg_status`, which is the object causing the -Wfamnae warnings.
This object is currently in a union along with `__le32 status`, so
anything using struct il4965_beacon_notif needs to have its own view
of `status`. To preserve the memory layout, we therefore add member
`__le32 beacon_tx_status` to struct il4965_beacon_notif.

After these changes, the size of struct il4965_beacon_notif along
with its member's offsets remain the same, hence the memory layout
doesn't change:

Before changes:
struct il4965_beacon_notif {
	struct il4965_tx_resp      beacon_notify_hdr;    /*     0    24 */
	__le32                     low_tsf;              /*    24     4 */
	__le32                     high_tsf;             /*    28     4 */
	__le32                     ibss_mgr_status;      /*    32     4 */

	/* size: 36, cachelines: 1, members: 4 */
	/* last cacheline: 36 bytes */
};

After changes:
struct il4965_beacon_notif {
	struct il4965_tx_resp_hdr  beacon_notify_hdr;    /*     0    20 */
	__le32                     beacon_tx_status;     /*    20     4 */
	__le32                     low_tsf;              /*    24     4 */
	__le32                     high_tsf;             /*    28     4 */
	__le32                     ibss_mgr_status;      /*    32     4 */

	/* size: 36, cachelines: 1, members: 5 */
	/* last cacheline: 36 bytes */
};

Lastly, adjust the rest of the code, accordingly.

With these changes fix the following warnings:

11 drivers/net/wireless/intel/iwlegacy/common.h:526:11: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
11 drivers/net/wireless/intel/iwlegacy/commands.h:2667:31: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
4 drivers/net/wireless/intel/iwlegacy/3945.h:131:11: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]

Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
---
Changes in v4:
 - (Aggh...) Fix a couple of typos in changelog text:
   		s/stryct/struct
   		s/il4965_beacon_notif/il4965_tx_resp.

Changes in v3:
 - Create new separate struct il4965_tx_resp_hdr, and use
   transparent struct members instead of __struct_group(). (Kees)
 - Update subject line - Add 'wifi:' prefix. (Stanislaw)
 - Link: https://lore.kernel.org/linux-hardening/aZLgfO6z1Gt8aiyc@kspp/

Changes in v2:
 - Use the struct_group() helper, and update the conflicting type
   (struct il4965_tx_resp -> struct il4965_tx_resp_hdr) in
   struct il4965_beacon_notif.
 - Link: https://lore.kernel.org/linux-hardening/aYlkt2WP2oQ1TgeH@kspp/

v1:
 - Link: https://lore.kernel.org/linux-hardening/aR2CtqZI3atH0HmE@kspp/

 drivers/net/wireless/intel/iwlegacy/3945.h     | 4 +++-
 drivers/net/wireless/intel/iwlegacy/4965-mac.c | 2 +-
 drivers/net/wireless/intel/iwlegacy/commands.h | 9 +++++++--
 drivers/net/wireless/intel/iwlegacy/common.h   | 4 +++-
 4 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlegacy/3945.h b/drivers/net/wireless/intel/iwlegacy/3945.h
index fb1e33c89d0e..ed63b31fee9a 100644
--- a/drivers/net/wireless/intel/iwlegacy/3945.h
+++ b/drivers/net/wireless/intel/iwlegacy/3945.h
@@ -123,13 +123,15 @@ enum il3945_antenna {
 #define IEEE80211_FRAME_LEN             (IEEE80211_DATA_LEN + IEEE80211_HLEN)
 
 struct il3945_frame {
+	struct list_head list;
+
+	/* Must be last as it ends in a flexible-array member. */
 	union {
 		struct ieee80211_hdr frame;
 		struct il3945_tx_beacon_cmd beacon;
 		u8 raw[IEEE80211_FRAME_LEN];
 		u8 cmd[360];
 	} u;
-	struct list_head list;
 };
 
 #define SUP_RATE_11A_MAX_NUM_CHANNELS  8
diff --git a/drivers/net/wireless/intel/iwlegacy/4965-mac.c b/drivers/net/wireless/intel/iwlegacy/4965-mac.c
index 57fa866efd9f..f5a99a2ee95a 100644
--- a/drivers/net/wireless/intel/iwlegacy/4965-mac.c
+++ b/drivers/net/wireless/intel/iwlegacy/4965-mac.c
@@ -4076,7 +4076,7 @@ il4965_hdl_beacon(struct il_priv *il, struct il_rx_buf *rxb)
 	u8 rate = il4965_hw_get_rate(beacon->beacon_notify_hdr.rate_n_flags);
 
 	D_RX("beacon status %x retries %d iss %d tsf:0x%.8x%.8x rate %d\n",
-	     le32_to_cpu(beacon->beacon_notify_hdr.u.status) & TX_STATUS_MSK,
+	     le32_to_cpu(beacon->beacon_tx_status) & TX_STATUS_MSK,
 	     beacon->beacon_notify_hdr.failure_frame,
 	     le32_to_cpu(beacon->ibss_mgr_status),
 	     le32_to_cpu(beacon->high_tsf), le32_to_cpu(beacon->low_tsf), rate);
diff --git a/drivers/net/wireless/intel/iwlegacy/commands.h b/drivers/net/wireless/intel/iwlegacy/commands.h
index b61b8f377702..7aba84ac88a1 100644
--- a/drivers/net/wireless/intel/iwlegacy/commands.h
+++ b/drivers/net/wireless/intel/iwlegacy/commands.h
@@ -1690,7 +1690,7 @@ struct agg_tx_status {
 	__le16 sequence;
 } __packed;
 
-struct il4965_tx_resp {
+struct il4965_tx_resp_hdr {
 	u8 frame_count;		/* 1 no aggregation, >1 aggregation */
 	u8 bt_kill_count;	/* # blocked by bluetooth (unused for agg) */
 	u8 failure_rts;		/* # failures due to unsuccessful RTS */
@@ -1707,6 +1707,10 @@ struct il4965_tx_resp {
 	__le16 reserved;
 	__le32 pa_power1;	/* RF power amplifier measurement (not used) */
 	__le32 pa_power2;
+} __packed;
+
+struct il4965_tx_resp {
+	struct il4965_tx_resp_hdr;
 
 	/*
 	 * For non-agg:  frame status TX_STATUS_*
@@ -2664,7 +2668,8 @@ struct il3945_beacon_notif {
 } __packed;
 
 struct il4965_beacon_notif {
-	struct il4965_tx_resp beacon_notify_hdr;
+	struct il4965_tx_resp_hdr beacon_notify_hdr;
+	__le32 beacon_tx_status;
 	__le32 low_tsf;
 	__le32 high_tsf;
 	__le32 ibss_mgr_status;
diff --git a/drivers/net/wireless/intel/iwlegacy/common.h b/drivers/net/wireless/intel/iwlegacy/common.h
index 4c9836ab11dd..21f1c7702add 100644
--- a/drivers/net/wireless/intel/iwlegacy/common.h
+++ b/drivers/net/wireless/intel/iwlegacy/common.h
@@ -518,13 +518,15 @@ struct il_channel_info {
 #define IEEE80211_FRAME_LEN             (IEEE80211_DATA_LEN + IEEE80211_HLEN)
 
 struct il_frame {
+	struct list_head list;
+
+	/* Must be last as it ends in a flexible-array member. */
 	union {
 		struct ieee80211_hdr frame;
 		struct il_tx_beacon_cmd beacon;
 		u8 raw[IEEE80211_FRAME_LEN];
 		u8 cmd[360];
 	} u;
-	struct list_head list;
 };
 
 enum {
-- 
2.43.0
Re: [PATCH v4][next] wifi: iwlegacy: Avoid multiple -Wflex-array-member-not-at-end warnings
Posted by Stanislaw Gruszka 4 weeks, 1 day ago
On Mon, Feb 16, 2026 at 06:25:14PM +0900, 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.
> 
> Move the conflicting declarations (which in a couple of cases happen
> to be in a union, so the entire unions are moved) to the end of the
> corresponding structures, struct il_frame, and struct il3945_frame.
> 
> Notice that `struct il_tx_beacon_cmd`, `struct il4965_tx_resp`, and
> `struct il3945_tx_beacon_cmd` are flexible structures, this is
> structures that contain a flexible-array member.
> 
> The case for struct il4965_beacon_notif is different. Since this
> structure is defined by hardware, we create the new `struct
> il4965_tx_resp_hdr` type. We then use this newly created type to
> replace the object type causing trouble in struct il4965_beacon_notif,
> namely `struct il4965_tx_resp`.
> 
> Also, once -fms-extensions is enabled, we can use transparent struct
> members in struct il4965_tx_resp.
> 
> Notice that the newly created type does not contain the flex-array
> member `agg_status`, which is the object causing the -Wfamnae warnings.
> This object is currently in a union along with `__le32 status`, so
> anything using struct il4965_beacon_notif needs to have its own view
> of `status`. To preserve the memory layout, we therefore add member
> `__le32 beacon_tx_status` to struct il4965_beacon_notif.
> 
> After these changes, the size of struct il4965_beacon_notif along
> with its member's offsets remain the same, hence the memory layout
> doesn't change:
> 
> Before changes:
> struct il4965_beacon_notif {
> 	struct il4965_tx_resp      beacon_notify_hdr;    /*     0    24 */
> 	__le32                     low_tsf;              /*    24     4 */
> 	__le32                     high_tsf;             /*    28     4 */
> 	__le32                     ibss_mgr_status;      /*    32     4 */
> 
> 	/* size: 36, cachelines: 1, members: 4 */
> 	/* last cacheline: 36 bytes */
> };
> 
> After changes:
> struct il4965_beacon_notif {
> 	struct il4965_tx_resp_hdr  beacon_notify_hdr;    /*     0    20 */
> 	__le32                     beacon_tx_status;     /*    20     4 */
> 	__le32                     low_tsf;              /*    24     4 */
> 	__le32                     high_tsf;             /*    28     4 */
> 	__le32                     ibss_mgr_status;      /*    32     4 */
> 
> 	/* size: 36, cachelines: 1, members: 5 */
> 	/* last cacheline: 36 bytes */
> };
> 
> Lastly, adjust the rest of the code, accordingly.
> 
> With these changes fix the following warnings:
> 
> 11 drivers/net/wireless/intel/iwlegacy/common.h:526:11: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
> 11 drivers/net/wireless/intel/iwlegacy/commands.h:2667:31: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
> 4 drivers/net/wireless/intel/iwlegacy/3945.h:131:11: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
> 
> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Acked-by: Stanislaw Gruszka <stf_xl@wp.pl>

Regards
Stanislaw
> ---
> Changes in v4:
>  - (Aggh...) Fix a couple of typos in changelog text:
>    		s/stryct/struct
>    		s/il4965_beacon_notif/il4965_tx_resp.
> 
> Changes in v3:
>  - Create new separate struct il4965_tx_resp_hdr, and use
>    transparent struct members instead of __struct_group(). (Kees)
>  - Update subject line - Add 'wifi:' prefix. (Stanislaw)
>  - Link: https://lore.kernel.org/linux-hardening/aZLgfO6z1Gt8aiyc@kspp/
> 
> Changes in v2:
>  - Use the struct_group() helper, and update the conflicting type
>    (struct il4965_tx_resp -> struct il4965_tx_resp_hdr) in
>    struct il4965_beacon_notif.
>  - Link: https://lore.kernel.org/linux-hardening/aYlkt2WP2oQ1TgeH@kspp/
> 
> v1:
>  - Link: https://lore.kernel.org/linux-hardening/aR2CtqZI3atH0HmE@kspp/
> 
>  drivers/net/wireless/intel/iwlegacy/3945.h     | 4 +++-
>  drivers/net/wireless/intel/iwlegacy/4965-mac.c | 2 +-
>  drivers/net/wireless/intel/iwlegacy/commands.h | 9 +++++++--
>  drivers/net/wireless/intel/iwlegacy/common.h   | 4 +++-
>  4 files changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/wireless/intel/iwlegacy/3945.h b/drivers/net/wireless/intel/iwlegacy/3945.h
> index fb1e33c89d0e..ed63b31fee9a 100644
> --- a/drivers/net/wireless/intel/iwlegacy/3945.h
> +++ b/drivers/net/wireless/intel/iwlegacy/3945.h
> @@ -123,13 +123,15 @@ enum il3945_antenna {
>  #define IEEE80211_FRAME_LEN             (IEEE80211_DATA_LEN + IEEE80211_HLEN)
>  
>  struct il3945_frame {
> +	struct list_head list;
> +
> +	/* Must be last as it ends in a flexible-array member. */
>  	union {
>  		struct ieee80211_hdr frame;
>  		struct il3945_tx_beacon_cmd beacon;
>  		u8 raw[IEEE80211_FRAME_LEN];
>  		u8 cmd[360];
>  	} u;
> -	struct list_head list;
>  };
>  
>  #define SUP_RATE_11A_MAX_NUM_CHANNELS  8
> diff --git a/drivers/net/wireless/intel/iwlegacy/4965-mac.c b/drivers/net/wireless/intel/iwlegacy/4965-mac.c
> index 57fa866efd9f..f5a99a2ee95a 100644
> --- a/drivers/net/wireless/intel/iwlegacy/4965-mac.c
> +++ b/drivers/net/wireless/intel/iwlegacy/4965-mac.c
> @@ -4076,7 +4076,7 @@ il4965_hdl_beacon(struct il_priv *il, struct il_rx_buf *rxb)
>  	u8 rate = il4965_hw_get_rate(beacon->beacon_notify_hdr.rate_n_flags);
>  
>  	D_RX("beacon status %x retries %d iss %d tsf:0x%.8x%.8x rate %d\n",
> -	     le32_to_cpu(beacon->beacon_notify_hdr.u.status) & TX_STATUS_MSK,
> +	     le32_to_cpu(beacon->beacon_tx_status) & TX_STATUS_MSK,
>  	     beacon->beacon_notify_hdr.failure_frame,
>  	     le32_to_cpu(beacon->ibss_mgr_status),
>  	     le32_to_cpu(beacon->high_tsf), le32_to_cpu(beacon->low_tsf), rate);
> diff --git a/drivers/net/wireless/intel/iwlegacy/commands.h b/drivers/net/wireless/intel/iwlegacy/commands.h
> index b61b8f377702..7aba84ac88a1 100644
> --- a/drivers/net/wireless/intel/iwlegacy/commands.h
> +++ b/drivers/net/wireless/intel/iwlegacy/commands.h
> @@ -1690,7 +1690,7 @@ struct agg_tx_status {
>  	__le16 sequence;
>  } __packed;
>  
> -struct il4965_tx_resp {
> +struct il4965_tx_resp_hdr {
>  	u8 frame_count;		/* 1 no aggregation, >1 aggregation */
>  	u8 bt_kill_count;	/* # blocked by bluetooth (unused for agg) */
>  	u8 failure_rts;		/* # failures due to unsuccessful RTS */
> @@ -1707,6 +1707,10 @@ struct il4965_tx_resp {
>  	__le16 reserved;
>  	__le32 pa_power1;	/* RF power amplifier measurement (not used) */
>  	__le32 pa_power2;
> +} __packed;
> +
> +struct il4965_tx_resp {
> +	struct il4965_tx_resp_hdr;
>  
>  	/*
>  	 * For non-agg:  frame status TX_STATUS_*
> @@ -2664,7 +2668,8 @@ struct il3945_beacon_notif {
>  } __packed;
>  
>  struct il4965_beacon_notif {
> -	struct il4965_tx_resp beacon_notify_hdr;
> +	struct il4965_tx_resp_hdr beacon_notify_hdr;
> +	__le32 beacon_tx_status;
>  	__le32 low_tsf;
>  	__le32 high_tsf;
>  	__le32 ibss_mgr_status;
> diff --git a/drivers/net/wireless/intel/iwlegacy/common.h b/drivers/net/wireless/intel/iwlegacy/common.h
> index 4c9836ab11dd..21f1c7702add 100644
> --- a/drivers/net/wireless/intel/iwlegacy/common.h
> +++ b/drivers/net/wireless/intel/iwlegacy/common.h
> @@ -518,13 +518,15 @@ struct il_channel_info {
>  #define IEEE80211_FRAME_LEN             (IEEE80211_DATA_LEN + IEEE80211_HLEN)
>  
>  struct il_frame {
> +	struct list_head list;
> +
> +	/* Must be last as it ends in a flexible-array member. */
>  	union {
>  		struct ieee80211_hdr frame;
>  		struct il_tx_beacon_cmd beacon;
>  		u8 raw[IEEE80211_FRAME_LEN];
>  		u8 cmd[360];
>  	} u;
> -	struct list_head list;
>  };
>  
>  enum {
> -- 
> 2.43.0
>