| 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