From: Simon Horman <[email protected]>
Sent: Wednesday, July 30, 2025 5:54 PM
>On Wed, Jul 30, 2025 at 04:52:09PM +0200, Jedrzej Jagielski wrote:
>> The current layout of struct ixgbe_orom_civd_info causes incorrect data
>> storage due to compiler-inserted padding. This results in issues when
>> writing OROM data into the structure.
>>
>> Add the __packed attribute to ensure the structure layout matches the
>> expected binary format without padding.
>>
>> Fixes: 70db0788a262 ("ixgbe: read the OROM version information")
>> Reviewed-by: Aleksandr Loktionov <[email protected]>
>> Signed-off-by: Jedrzej Jagielski <[email protected]>
>> ---
>> drivers/net/ethernet/intel/ixgbe/ixgbe_type_e610.h | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type_e610.h
>> b/drivers/net/ethernet/intel/ixgbe/ixgbe_type_e610.h
>> index 09df67f03cf4..38a41d81de0f 100644
>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type_e610.h
>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type_e610.h
>> @@ -1150,7 +1150,7 @@ struct ixgbe_orom_civd_info {
>> __le32 combo_ver; /* Combo Image Version number */
>> u8 combo_name_len; /* Length of the unicode combo image version
>> string, max of 32 */
>> __le16 combo_name[32]; /* Unicode string representing the Combo Image
>> version */
>> -};
>> +} __packed;
>
>...
>
>Hi Jedrzej,
>
>I agree that this is correct. Otherwise there will be a 3 byte hole before
>combo_ver and a 1 byte hole before and combo_name. Which, based on the
>commit message, I assume is not part of the layout this structure
>represents.
>
>A side effect of this change is that both combo_ver and elements of
>combo_name become unaligned. As elements combo_ver does not seem to be
>accessed directly, things seem clean there. But in the case of combo_ver,
>I wonder if this change is also needed. (Compile tested only!)
Hi Simon
it's definitely worth incorporating into the patch, due to risk of unaligmnent.
Thanks for your suggestion!
>
>diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
>b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
>index 71ea25de1bac..754c176fd4a7 100644
>--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
>+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
>@@ -3123,7 +3123,7 @@ static int ixgbe_get_orom_ver_info(struct ixgbe_hw *hw,
> if (err)
> return err;
>
>- combo_ver = le32_to_cpu(civd.combo_ver);
>+ combo_ver = get_unaligned_le32(&civd.combo_ver);
>
> orom->major = (u8)FIELD_GET(IXGBE_OROM_VER_MASK, combo_ver);
> orom->patch = (u8)FIELD_GET(IXGBE_OROM_VER_PATCH_MASK, combo_ver);
>
