netdev - Re: [PATCH] overflow: Change DEFINE_FLEX to take __counted

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: <bd21f7dc-9f89-40ee-895e-601c80165225@intel.com>
Date: Wed, 6 Mar 2024 08:06:29 +0100
From: Przemek Kitszel <przemyslaw.kitszel@...el.com>
To: "Gustavo A. R. Silva" <gustavo@...eddedor.com>, Kees Cook
	<keescook@...omium.org>
CC: Jesse Brandeburg <jesse.brandeburg@...el.com>, Tony Nguyen
	<anthony.l.nguyen@...el.com>, "David S. Miller" <davem@...emloft.net>, "Eric
 Dumazet" <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni
	<pabeni@...hat.com>, "Gustavo A. R. Silva" <gustavoars@...nel.org>,
	<intel-wired-lan@...ts.osuosl.org>, <netdev@...r.kernel.org>,
	<linux-hardening@...r.kernel.org>, Andrew Morton <akpm@...ux-foundation.org>,
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] overflow: Change DEFINE_FLEX to take __counted_by member

On 3/6/24 04:25, Gustavo A. R. Silva wrote:
> 
> 
> On 05/03/24 19:07, Kees Cook wrote:
>> The norm should be flexible array structures with __counted_by
>> annotations, so DEFINE_FLEX() is updated to expect that. Rename
>> the non-annotated version to DEFINE_RAW_FLEX(), and update the few
>> existing users. Additionally add self-tests to validate syntax and
>> size calculations.
>>
>> Signed-off-by: Kees Cook <keescook@...omium.org>
>> ---
> 
> [..]

Just a note that ice changes are purely mechanical, so this seems ok
to go via linux-hardening tree. And changes per-se are fine too :)

> 
>> +/**
>> + * DEFINE_FLEX() - Define an on-stack instance of structure with a 
>> trailing
>> + * flexible array member.
>> + *
>> + * @TYPE: structure type name, including "struct" keyword.
>> + * @NAME: Name for a variable to define.
>> + * @COUNTER: Name of the __counted_by member.
>> + * @MEMBER: Name of the array member.
>> + * @COUNT: Number of elements in the array; must be compile-time const.
>> + *
>> + * Define a zeroed, on-stack, instance of @TYPE structure with a 
>> trailing
>> + * flexible array member.
>> + * Use __struct_size(@NAME) to get compile-time size of it afterwards.
>> + */
>> +#define DEFINE_FLEX(TYPE, NAME, COUNTER, MEMBER, COUNT)    \
> 
> Probably, swapping COUNTER and MEMBER is better?

right now we have usage scenario (from Kunits):
	DEFINE_FLEX(struct foo, eight, counter, array, 8);

> 
>      DEFINE_FLEX(TYPE, NAME, MEMBER, COUNTER, COUNT)

usage would become:
	DEFINE_FLEX(struct foo, eight, array, counter, 8);

which reads a bit better indeed, with the added benefit that we
go from broader to more specific:
whole struct -> array -> array size variable -> given array size

so +1 from me for the params swap

> 
> Thanks
> -- 
> Gustavo