netdev - 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 for Android: free password hash cracker in your pocket

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

Message-ID: <a0338310-e66c-497c-bc1f-a597e50aa3ff@intel.com>
Date: Thu, 15 Aug 2024 14:16:13 -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: lib/packing.c behaving weird if buffer length is not multiple of 4
 with QUIRK_LSW32_IS_FIRST

Hi Vladimir!

I am recently attempting to refactor some bespoke code for packing data
in the ice networking driver into a bit packed hardware array, using the
<linux/packing.h> and lib/packing.c infrastructure.

My hardware packed buffer needs QUIRK_LITTLE_ENDIAN and
QUIRK_LSW32_IS_FIRST, as the entire buffer is packed as little endian,
and the lower words come first in the buffer.

Everything was working ok in initial testing, until I tried packing a
buffer that was 22 bytes long. Everything seemed to shift by 2 bytes:

Here, I print the initial details such as offset, lsb, width, then the
hex dump of the u64 value I'm trying to insert.

I do the call to packing() with the appropriate quirks set, and then hex
dump the 22-byte buffer after.

> kernel: offset=0, size=8, lsb=0, width=57
> kernel: value: 60 9b fe 01 00 00 00 00
> kernel: 0x0000000001fe9b60 -> 000-056: fe 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> kernel: 0x0000000001fe9b60 -> 000-056: 00 00 00 00 00 00

It seems to have failed to copy the first two bytes in to the buffer.

I discovered that if I pretend the buffer is 24 bytes (a multiple of 4
bytes, i.e. a word size), then everything works as expected:

> kernel: offset=0, size=8, lsb=0, width=57
> kernel: value: 60 fc fe 01 00 00 00 00
> kernel: 0x0000000001fefc60 -> 000-056: 60 fc fe 01 00 00 00 00 00 00 00 00 00 00 00 00
> kernel: 0x0000000001fefc60 -> 000-056: 00 00 00 00 00 00 00 00

It seems like this is either a misunderstanding of a fundamental
requirement of the packing() infrastructure, a bug in the
QUIRK_LSW32_IS_FIRST, or I need a new quirk for the packing infrastructure?

Essentially, I think the problem is that it uses the length of the
buffer to find the word, but when the length is not a multiple of 2, the
word offset picked is incorrect.

Using a larger length than the size of the buffer "works" as long as I
never use a bit offset thats larger than the *actual* buffer.. but that
seems like a poor solution.

Essentially, it seems like the default indexing for big endian is
searching for each byte from the end of the array and then using the
quirks to swap back to the inverse ending.

I think the fix is probably just to do a round-up division on the len/4
check in get_reverse_lsw32_offset, since its calculating the total
number of words in the length, and effectively rounds down for lengths
that aren't multiples of 4.

Does this seem like the correct analysis to you?

Thanks,
Jake