[PATCH] iwlwifi: dbg_ini: fix structure packing

[PATCH] iwlwifi: dbg_ini: fix structure packing

[Arnd Bergmann]

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;
 	};
-- 
2.39.2

Re: [PATCH] iwlwifi: dbg_ini: fix structure packing

[Johannes Berg]

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

Re: [PATCH] iwlwifi: dbg_ini: fix structure packing

[Arnd Bergmann]

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

Re: [PATCH] iwlwifi: dbg_ini: fix structure packing

[Greenman, Gregory]

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>