wifi: iwlegacy: Avoid multiple -Wflex-array-member-not-at-end warnings
PATCH v4 1/1 wifi: iwlegacy: Avoid multiple -Wflex-array-member-not-at-end warnings
-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]
---
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(-)
drivers/net/wireless/intel/iwlegacy/3945.h +3 -1
@@ -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
drivers/net/wireless/intel/iwlegacy/4965-mac.c +1 -1
@@ -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);
drivers/net/wireless/intel/iwlegacy/commands.h +7 -2
@@ -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;
drivers/net/wireless/intel/iwlegacy/common.h +3 -1
@@ -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 {
Signature · 1 line
2.43.0
Quoted 56 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.
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 structil3945_frame.Notice that
struct il_tx_beacon_cmd,struct il4965_tx_resp, andstruct il3945_tx_beacon_cmdare flexible structures, this is structures that contain a flexible-array member.The case for struct
il4965_beacon_notifis different. Since this structure is defined by hardware, we create the newstruct il4965_tx_resp_hdrtype. We then use this newly created type to replace the object type causing trouble in structil4965_beacon_notif, namelystruct 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 structil4965_beacon_notifneeds to have its own view ofstatus. To preserve the memory layout, we therefore add member__le32 beacon_tx_statusto structil4965_beacon_notif.After these changes, the size of struct
il4965_beacon_notifalong with its member's offsets remain the same, hence the memory layout doesn't change:Before changes:
structil4965_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:
structil4965_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.
Quoted 7 lines
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] 11drivers/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] 4drivers/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 <[email protected]>
Regards
Stanislaw
Quoted 119 lines
---
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 structil4965_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 thestruct_group()helper, and update the conflicting type (structil4965_tx_resp-> structil4965_tx_resp_hdr) in structil4965_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(-)drivers/net/wireless/intel/iwlegacy/3945.h +3
-1@@ -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 8drivers/net/wireless/intel/iwlegacy/4965-mac.c +1
-1@@ -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);drivers/net/wireless/intel/iwlegacy/commands.h +7
-2@@ -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;drivers/net/wireless/intel/iwlegacy/common.h +3
-1@@ -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 {Signature · 1 line
2.43.0