netdev - Re: lib/packing.c behaving weird if buffer length is not multiple of 4 with QUIRK_LSW32_IS

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: <7d0e11a8-04c3-445b-89d3-fb347563dcd3@intel.com>
Date: Mon, 19 Aug 2024 14:53:34 -0700
From: Jacob Keller <jacob.e.keller@...el.com>
To: Vladimir Oltean <olteanv@...il.com>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: lib/packing.c behaving weird if buffer length is not multiple of
 4 with QUIRK_LSW32_IS_FIRST



On 8/19/2024 11:45 AM, Jacob Keller wrote:
> 
> 
> On 8/18/2024 6:29 AM, Vladimir Oltean wrote:
>> Hi Jake,
>>
>> On Fri, Aug 16, 2024 at 04:37:22PM -0700, Jacob Keller wrote:
>>> I'm honestly not sure what the right solution here is, because the way
>>> LITTLE_ENDIAN and LSW32_IS_FIRST work they effectively *require*
>>> word-aligned sizes. If we use a word-aligned size, then they both make
>>> sense, but my hardware buffer isn't word aligned. I can cheat, and just
>>> make sure I never use bits that access the invalid parts of the buffer..
>>> but that seems like the wrong solution... A larger size would break
>>> normal Big endian ordering without quirks...
>>
>> It is a use case that I would like to support. Thanks for having the
>> patience to explain the issue to me.
>>
> 
> Great, thank!
> 
>>> Really, what my hardware buffer wants is to map the lowest byte of the
>>> data to the lowest byte of the buffer. This is what i would consider
>>> traditionally little endian ordering.
>>>
>>> This also happens to be is equivalent to LSW32_IS_FIRST and
>>> LITTLE_ENDIAN when sizes are multiples of 4.
>>
>> Yes, "traditionally little endian" would indeed translate into
>> QUIRK_LSW32_IS_FIRST | QUIRK_LITTLE_ENDIAN. Your use of the API seems
>> correct. I did need that further distinction between "little endian
>> within a group of 4 bytes" and "little endian among groups of 4 bytes"
>> because the NXP SJA1105 memory layout is weird like that, and is
>> "little endian" in one way but not in another. Anyway..
> 
> 
> Yea, I figured the distinction was based on real hardware.
> 
>>
>> I've attached 2 patches which hopefully make the API usable for your
>> driver. I've tested them locally and did not notice issues.
> 
> I'll check these out and get back to you!
> 
> Thanks,
> Jake
> 

The patches work for my use-case! I also think it might be helpful to
add some Kunit tests to cover the packing and unpacking, which I
wouldn't mind trying to do.

If/when you send the patches, feel free to add:

Tested-by: Jacob Keller <jacob.e.keller@...el.com>

Alternatively, I could send them as part of the series where I implement
the changes to use lib/packing in the ice driver.

Thanks,
Jake