netdev - Re: [PATCH net-next v5 3/9] lib: packing: add pack_fields() and unpack

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: <5a666ac7-4026-4d4f-b2dc-74a124055f21@intel.com>
Date: Wed, 13 Nov 2024 13:04:16 -0800
From: Jacob Keller <jacob.e.keller@...el.com>
To: Vladimir Oltean <olteanv@...il.com>
CC: Andrew Morton <akpm@...ux-foundation.org>, Eric Dumazet
	<edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni
	<pabeni@...hat.com>, Tony Nguyen <anthony.l.nguyen@...el.com>, "Przemek
 Kitszel" <przemyslaw.kitszel@...el.com>, netdev <netdev@...r.kernel.org>,
	<linux-kbuild@...r.kernel.org>, Vladimir Oltean <vladimir.oltean@....com>,
	Masahiro Yamada <masahiroy@...nel.org>
Subject: Re: [PATCH net-next v5 3/9] lib: packing: add pack_fields() and
 unpack_fields()



On 11/13/2024 12:32 PM, Masahiro Yamada wrote:
> On Mon, Nov 11, 2024 at 5:08 PM Jacob Keller <jacob.e.keller@...el.com> wrote:
>>
>> From: Vladimir Oltean <vladimir.oltean@....com>
>>
>> This is new API which caters to the following requirements:
>>
>> - Pack or unpack a large number of fields to/from a buffer with a small
>>   code footprint. The current alternative is to open-code a large number
>>   of calls to pack() and unpack(), or to use packing() to reduce that
>>   number to half. But packing() is not const-correct.
>>
>> - Use unpacked numbers stored in variables smaller than u64. This
>>   reduces the rodata footprint of the stored field arrays.
>>
>> - Perform error checking at compile time, rather than runtime, and return
>>   void from the API functions. Because the C preprocessor can't generat
>>   variable length code (loops), we can't easily use macros to implement the
>>   overlap checks at compile time.
>>
>>   Instead, check for field ordering and overlap in modpost.
> 
> This is over-engineering.
> 
> modpost should not be bothered just for a small library like this.
> 
> Please do sanity checks within lib/packing.c
> 

With the goal of maintaining compile time checks, we end up either
needing to use generated macros which are O(N^2) if we allow arbitrary
overlap. If we instead allow only only ascending or descending order,
this would drop to O(N) which would avoid needing to have 20k lines of
generated code for the case with 50. I think we could implement them
without forcing drivers to specifically call the correct macro by using
something like __builtin_choose_expr(), tho implementing that macro to
select could be quite long.

Otherwise we can fall back to either module load time checks, or go all
the way back to only sanity checking at executing of pack_fields or
unpack_fields.