lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <4caca5e1-c8ea-46dd-a6f2-532030a48454@embeddedor.com> Date: Fri, 13 Sep 2024 10:00:05 +0200 From: "Gustavo A. R. Silva" <gustavo@...eddedor.com> To: "Gustavo A. R. Silva" <gustavoars@...nel.org>, Miri Korenblit <miriam.rachel.korenblit@...el.com>, Kalle Valo <kvalo@...nel.org> Cc: linux-wireless@...r.kernel.org, linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org Subject: Re: [PATCH][next] wifi: iwlwifi: dvm: Avoid -Wflex-array-member-not-at-end warnings Hi all, Friendly ping: who can take this, please? :) Thanks -- Gustavo On 15/08/24 21:00, 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. > > So, in order to avoid ending up with a flexible-array member in the > middle of multiple other structs, we use the `__struct_group()` > helper to create a new tagged `struct iwl_tx_cmd_hdr`. This structure > groups together all the members of the flexible `struct iwl_tx_cmd` > except the flexible array. > > As a result, the array is effectively separated from the rest of the > members without modifying the memory layout of the flexible structure. > We then change the type of the middle struct members currently causing > trouble from `struct iwl_tx_cmd` to `struct iwl_tx_cmd_hdr`. > > We also want to ensure that when new members need to be added to the > flexible structure, they are always included within the newly created > tagged struct. For this, we use `static_assert()`. This ensures that the > memory layout for both the flexible structure and the new tagged struct > is the same after any changes. > > This approach avoids having to implement `struct iwl_tx_cmd_hdr` > as a completely separate structure, thus preventing having to maintain > two independent but basically identical structures, closing the door > to potential bugs in the future. > > So, with these changes, fix the following warnings: > > drivers/net/wireless/intel/iwlwifi/dvm/commands.h:2315:27: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] > drivers/net/wireless/intel/iwlwifi/dvm/commands.h:2426:27: 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@...nel.org> > --- > .../net/wireless/intel/iwlwifi/dvm/commands.h | 154 +++++++++--------- > 1 file changed, 78 insertions(+), 76 deletions(-) > > diff --git a/drivers/net/wireless/intel/iwlwifi/dvm/commands.h b/drivers/net/wireless/intel/iwlwifi/dvm/commands.h > index 3f49c0bccb28..96ea6c8dfc89 100644 > --- a/drivers/net/wireless/intel/iwlwifi/dvm/commands.h > +++ b/drivers/net/wireless/intel/iwlwifi/dvm/commands.h > @@ -1180,85 +1180,87 @@ struct iwl_dram_scratch { > } __packed; > > struct iwl_tx_cmd { > - /* > - * MPDU byte count: > - * MAC header (24/26/30/32 bytes) + 2 bytes pad if 26/30 header size, > - * + 8 byte IV for CCM or TKIP (not used for WEP) > - * + Data payload > - * + 8-byte MIC (not used for CCM/WEP) > - * NOTE: Does not include Tx command bytes, post-MAC pad bytes, > - * MIC (CCM) 8 bytes, ICV (WEP/TKIP/CKIP) 4 bytes, CRC 4 bytes.i > - * Range: 14-2342 bytes. > - */ > - __le16 len; > - > - /* > - * MPDU or MSDU byte count for next frame. > - * Used for fragmentation and bursting, but not 11n aggregation. > - * Same as "len", but for next frame. Set to 0 if not applicable. > - */ > - __le16 next_frame_len; > - > - __le32 tx_flags; /* TX_CMD_FLG_* */ > - > - /* uCode may modify this field of the Tx command (in host DRAM!). > - * Driver must also set dram_lsb_ptr and dram_msb_ptr in this cmd. */ > - struct iwl_dram_scratch scratch; > - > - /* Rate for *all* Tx attempts, if TX_CMD_FLG_STA_RATE_MSK is cleared. */ > - __le32 rate_n_flags; /* RATE_MCS_* */ > - > - /* Index of destination station in uCode's station table */ > - u8 sta_id; > - > - /* Type of security encryption: CCM or TKIP */ > - u8 sec_ctl; /* TX_CMD_SEC_* */ > - > - /* > - * Index into rate table (see REPLY_TX_LINK_QUALITY_CMD) for initial > - * Tx attempt, if TX_CMD_FLG_STA_RATE_MSK is set. Normally "0" for > - * data frames, this field may be used to selectively reduce initial > - * rate (via non-0 value) for special frames (e.g. management), while > - * still supporting rate scaling for all frames. > - */ > - u8 initial_rate_index; > - u8 reserved; > - u8 key[16]; > - __le16 next_frame_flags; > - __le16 reserved2; > - union { > - __le32 life_time; > - __le32 attempt; > - } stop_time; > - > - /* Host DRAM physical address pointer to "scratch" in this command. > - * Must be dword aligned. "0" in dram_lsb_ptr disables usage. */ > - __le32 dram_lsb_ptr; > - u8 dram_msb_ptr; > - > - u8 rts_retry_limit; /*byte 50 */ > - u8 data_retry_limit; /*byte 51 */ > - u8 tid_tspec; > - union { > - __le16 pm_frame_timeout; > - __le16 attempt_duration; > - } timeout; > - > - /* > - * Duration of EDCA burst Tx Opportunity, in 32-usec units. > - * Set this if txop time is not specified by HCCA protocol (e.g. by AP). > - */ > - __le16 driver_txop; > - > + /* New members MUST be added within the __struct_group() macro below. */ > + __struct_group(iwl_tx_cmd_hdr, __hdr, __packed, > + /* > + * MPDU byte count: > + * MAC header (24/26/30/32 bytes) + 2 bytes pad if 26/30 header size, > + * + 8 byte IV for CCM or TKIP (not used for WEP) > + * + Data payload > + * + 8-byte MIC (not used for CCM/WEP) > + * NOTE: Does not include Tx command bytes, post-MAC pad bytes, > + * MIC (CCM) 8 bytes, ICV (WEP/TKIP/CKIP) 4 bytes, CRC 4 bytes.i > + * Range: 14-2342 bytes. > + */ > + __le16 len; > + > + /* > + * MPDU or MSDU byte count for next frame. > + * Used for fragmentation and bursting, but not 11n aggregation. > + * Same as "len", but for next frame. Set to 0 if not applicable. > + */ > + __le16 next_frame_len; > + > + __le32 tx_flags; /* TX_CMD_FLG_* */ > + > + /* uCode may modify this field of the Tx command (in host DRAM!). > + * Driver must also set dram_lsb_ptr and dram_msb_ptr in this cmd. */ > + struct iwl_dram_scratch scratch; > + > + /* Rate for *all* Tx attempts, if TX_CMD_FLG_STA_RATE_MSK is cleared. */ > + __le32 rate_n_flags; /* RATE_MCS_* */ > + > + /* Index of destination station in uCode's station table */ > + u8 sta_id; > + > + /* Type of security encryption: CCM or TKIP */ > + u8 sec_ctl; /* TX_CMD_SEC_* */ > + > + /* > + * Index into rate table (see REPLY_TX_LINK_QUALITY_CMD) for initial > + * Tx attempt, if TX_CMD_FLG_STA_RATE_MSK is set. Normally "0" for > + * data frames, this field may be used to selectively reduce initial > + * rate (via non-0 value) for special frames (e.g. management), while > + * still supporting rate scaling for all frames. > + */ > + u8 initial_rate_index; > + u8 reserved; > + u8 key[16]; > + __le16 next_frame_flags; > + __le16 reserved2; > + union { > + __le32 life_time; > + __le32 attempt; > + } stop_time; > + > + /* Host DRAM physical address pointer to "scratch" in this command. > + * Must be dword aligned. "0" in dram_lsb_ptr disables usage. */ > + __le32 dram_lsb_ptr; > + u8 dram_msb_ptr; > + > + u8 rts_retry_limit; /*byte 50 */ > + u8 data_retry_limit; /*byte 51 */ > + u8 tid_tspec; > + union { > + __le16 pm_frame_timeout; > + __le16 attempt_duration; > + } timeout; > + > + /* > + * Duration of EDCA burst Tx Opportunity, in 32-usec units. > + * Set this if txop time is not specified by HCCA protocol (e.g. by AP). > + */ > + __le16 driver_txop; > + > + ); > /* > * MAC header goes here, followed by 2 bytes padding if MAC header > * length is 26 or 30 bytes, followed by payload data > */ > - union { > - DECLARE_FLEX_ARRAY(u8, payload); > - DECLARE_FLEX_ARRAY(struct ieee80211_hdr, hdr); > - }; > + struct ieee80211_hdr hdr[]; > } __packed; > +static_assert(offsetof(struct iwl_tx_cmd, hdr) == sizeof(struct iwl_tx_cmd_hdr), > + "struct member likely outside of __struct_group()"); > > /* > * TX command response is sent after *agn* transmission attempts. > @@ -2312,7 +2314,7 @@ struct iwl_scan_cmd { > > /* For active scans (set to all-0s for passive scans). > * Does not include payload. Must specify Tx rate; no rate scaling. */ > - struct iwl_tx_cmd tx_cmd; > + struct iwl_tx_cmd_hdr tx_cmd; > > /* For directed active scans (set to all-0s otherwise) */ > struct iwl_ssid_ie direct_scan[PROBE_OPTION_MAX]; > @@ -2423,7 +2425,7 @@ struct iwlagn_beacon_notif { > */ > > struct iwl_tx_beacon_cmd { > - struct iwl_tx_cmd tx; > + struct iwl_tx_cmd_hdr tx; > __le16 tim_idx; > u8 tim_size; > u8 reserved1;
Powered by blists - more mailing lists