diff mbox series

iwlwifi: dbg_ini: fix structure packing

Message ID 20230616090343.2454061-1-arnd@kernel.org (mailing list archive)
State Accepted
Delegated to: Johannes Berg
Headers show
Series iwlwifi: dbg_ini: fix structure packing | expand

Commit Message

Arnd Bergmann June 16, 2023, 9:03 a.m. UTC
From: Arnd Bergmann <arnd@arndb.de>

The iwl_fw_ini_error_dump_range structure has conflicting alignment
requirements for the inner union and the outer struct:

In file included from drivers/net/wireless/intel/iwlwifi/fw/dbg.c:9:
drivers/net/wireless/intel/iwlwifi/fw/error-dump.h:312:2: error: field  within 'struct iwl_fw_ini_error_dump_range' is less aligned than 'union iwl_fw_ini_error_dump_range::(anonymous at drivers/net/wireless/intel/iwlwifi/fw/error-dump.h:312:2)' and is usually due to 'struct iwl_fw_ini_error_dump_range' being packed, which can lead to unaligned accesses [-Werror,-Wunaligned-access]
        union {

As the original intention was apparently to make the entire structure
unaligned, mark the innermost members the same way so the union
becomes packed as well.

Fixes: 973193554cae6 ("iwlwifi: dbg_ini: dump headers cleanup")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/wireless/intel/iwlwifi/fw/error-dump.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Johannes Berg Aug. 14, 2023, 6:28 p.m. UTC | #1
On Fri, 2023-06-16 at 11:03 +0200, Arnd Bergmann wrote:
> 
> As the original intention was apparently to make the entire structure
> unaligned, mark the innermost members the same way so the union
> becomes packed as well.

Hm. Not sure exactly what you mean by that, but shouldn't we make that
"union { ... } __packed"?

johannes
Arnd Bergmann Aug. 14, 2023, 9:02 p.m. UTC | #2
On Mon, Aug 14, 2023, at 20:28, Johannes Berg wrote:
> On Fri, 2023-06-16 at 11:03 +0200, Arnd Bergmann wrote:
>> 
>> As the original intention was apparently to make the entire structure
>> unaligned, mark the innermost members the same way so the union
>> becomes packed as well.
>
> Hm. Not sure exactly what you mean by that, but shouldn't we make that
> "union { ... } __packed"?

Up to you, the effect is the same, as the other two members are already
packed. I generally try to keep the packing to the members that are
actually unaligned, to make the code more efficient, but in this case
the entire structure is unaligned, so it doesn't matter.

      Arnd
Greenman, Gregory Aug. 28, 2023, 12:46 p.m. UTC | #3
On Fri, 2023-06-16 at 11:03 +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> The iwl_fw_ini_error_dump_range structure has conflicting alignment
> requirements for the inner union and the outer struct:
> 
> In file included from drivers/net/wireless/intel/iwlwifi/fw/dbg.c:9:
> drivers/net/wireless/intel/iwlwifi/fw/error-dump.h:312:2: error: field  within 'struct iwl_fw_ini_error_dump_range' is less aligned than 'union iwl_fw_ini_error_dump_range::(anonymous at
> drivers/net/wireless/intel/iwlwifi/fw/error-dump.h:312:2)' and is usually due to 'struct iwl_fw_ini_error_dump_range' being packed, which can lead to unaligned accesses [-Werror,-Wunaligned-access]
>         union {
> 
> As the original intention was apparently to make the entire structure
> unaligned, mark the innermost members the same way so the union
> becomes packed as well.
> 
> Fixes: 973193554cae6 ("iwlwifi: dbg_ini: dump headers cleanup")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/net/wireless/intel/iwlwifi/fw/error-dump.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/wireless/intel/iwlwifi/fw/error-dump.h b/drivers/net/wireless/intel/iwlwifi/fw/error-dump.h
> index f5e08988dc7bf..06d6f7f664308 100644
> --- a/drivers/net/wireless/intel/iwlwifi/fw/error-dump.h
> +++ b/drivers/net/wireless/intel/iwlwifi/fw/error-dump.h
> @@ -310,9 +310,9 @@ struct iwl_fw_ini_fifo_hdr {
>  struct iwl_fw_ini_error_dump_range {
>         __le32 range_data_size;
>         union {
> -               __le32 internal_base_addr;
> -               __le64 dram_base_addr;
> -               __le32 page_num;
> +               __le32 internal_base_addr __packed;
> +               __le64 dram_base_addr __packed;
> +               __le32 page_num __packed;
>                 struct iwl_fw_ini_fifo_hdr fifo_hdr;
>                 struct iwl_cmd_header fw_pkt_hdr;
>         };

Acked-by: Gregory Greenman <gregory.greenman@intel.com>
diff mbox series

Patch

diff --git a/drivers/net/wireless/intel/iwlwifi/fw/error-dump.h b/drivers/net/wireless/intel/iwlwifi/fw/error-dump.h
index f5e08988dc7bf..06d6f7f664308 100644
--- a/drivers/net/wireless/intel/iwlwifi/fw/error-dump.h
+++ b/drivers/net/wireless/intel/iwlwifi/fw/error-dump.h
@@ -310,9 +310,9 @@  struct iwl_fw_ini_fifo_hdr {
 struct iwl_fw_ini_error_dump_range {
 	__le32 range_data_size;
 	union {
-		__le32 internal_base_addr;
-		__le64 dram_base_addr;
-		__le32 page_num;
+		__le32 internal_base_addr __packed;
+		__le64 dram_base_addr __packed;
+		__le32 page_num __packed;
 		struct iwl_fw_ini_fifo_hdr fifo_hdr;
 		struct iwl_cmd_header fw_pkt_hdr;
 	};