netdev - Re: [PATCH 1/4] rtw88: Add packed attribute to the eFuse structs

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAFBinCC+1jGJx1McnBY+kr3RTQ-UpxW6JYNpHzStUTredDuCug@mail.gmail.com>
Date:   Wed, 4 Jan 2023 16:30:07 +0100
From:   Martin Blumenstingl <martin.blumenstingl@...glemail.com>
To:     Ping-Ke Shih <pkshih@...ltek.com>,
        "David.Laight@...LAB.COM" <david.laight@...lab.com>
Cc:     "linux-wireless@...r.kernel.org" <linux-wireless@...r.kernel.org>,
        "kvalo@...nel.org" <kvalo@...nel.org>,
        "tehuang@...ltek.com" <tehuang@...ltek.com>,
        "s.hauer@...gutronix.de" <s.hauer@...gutronix.de>,
        "tony0620emma@...il.com" <tony0620emma@...il.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/4] rtw88: Add packed attribute to the eFuse structs

Hi Ping-Ke, Hi David,

On Sun, Jan 1, 2023 at 2:09 PM Ping-Ke Shih <pkshih@...ltek.com> wrote:
[...]
> Yes, it should not use bit filed. Instead, use a __le16 for all fields, such as
I think this can be done in a separate patch.
My v2 of this patch has reduced these changes to a minimum, see [0]

[...]
> struct rtw8821ce_efuse {
>    ...
>    u8 data1;       // offset 0x100
>    __le16 data2;   // offset 0x101-0x102
>    ...
> } __packed;
>
> Without __packed, compiler could has pad between data1 and data2,
> and then get wrong result.
My understanding is that this is the reason why we need __packed.

So my idea for the next steps is:
- I will send a v3 of my series but change the wording in the commit
description so it only mentions padding (but dropping the re-ordering
part)
- maybe Ping-Ke or his team can send a patch to fix the endian/bit
field problem in the PCIe eFuse structs
- (I'll keep working on SDIO support)

Does this make sense to both of you?


Best regards,
Martin


[0] https://lore.kernel.org/linux-wireless/20221229124845.1155429-2-martin.blumenstingl@googlemail.com/