netdev - Re: [PATCH net-next v2 02/10] lib: packing: adjust definitions and implementation for arbitrary buffer lengths

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: <20241003150510.w2yhj5ox2tnqdjst@skbuf>
Date: Thu, 3 Oct 2024 18:05:10 +0300
From: Vladimir Oltean <vladimir.oltean@....com>
To: Jacob Keller <jacob.e.keller@...el.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
	Vladimir Oltean <olteanv@...il.com>,
	"David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org,
	Przemek Kitszel <przemyslaw.kitszel@...el.com>
Subject: Re: [PATCH net-next v2 02/10] lib: packing: adjust definitions and
 implementation for arbitrary buffer lengths

On Wed, Oct 02, 2024 at 02:51:51PM -0700, Jacob Keller wrote:
> From: Vladimir Oltean <vladimir.oltean@....com>
> 
> Jacob Keller has a use case for packing() in the intel/ice networking
> driver, but it cannot be used as-is.
> 
> Simply put, the API quirks for LSW32_IS_FIRST and LITTLE_ENDIAN are
> naively implemented with the undocumented assumption that the buffer
> length must be a multiple of 4. All calculations of group offsets and
> offsets of bytes within groups assume that this is the case. But in the
> ice case, this does not hold true. For example, packing into a buffer
> of 22 bytes would yield wrong results, but pretending it was a 24 byte
> buffer would work.
> 
> Rather than requiring such hacks, and leaving a big question mark when
> it comes to discontinuities in the accessible bit fields of such buffer,
> we should extend the packing API to support this use case.
> 
> It turns out that we can keep the design in terms of groups of 4 bytes,
> but also make it work if the total length is not a multiple of 4.
> Just like before, imagine the buffer as a big number, and its most
> significant bytes (the ones that would make up to a multiple of 4) are
> missing. Thus, with a big endian (no quirks) interpretation of the
> buffer, those most significant bytes would be absent from the beginning
> of the buffer, and with a LSW32_IS_FIRST interpretation, they would be
> absent from the end of the buffer. The LITTLE_ENDIAN quirk, in the
> packing() API world, only affects byte ordering within groups of 4.
> Thus, it does not change which bytes are missing. Only the significance
> of the remaining bytes within the (smaller) group.
> 
> No change intended for buffer sizes which are multiples of 4. Tested
> with the sja1105 driver and with downstream unit tests.
> 
> Link: https://lore.kernel.org/netdev/a0338310-e66c-497c-bc1f-a597e50aa3ff@intel.com/
> Signed-off-by: Vladimir Oltean <vladimir.oltean@....com>
> Tested-by: Jacob Keller <jacob.e.keller@...el.com>
> Signed-off-by: Jacob Keller <jacob.e.keller@...el.com>
> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@...el.com>
> ---

Reviewed-by: Vladimir Oltean <vladimir.oltean@....com>