netdev - Re: [RFC net-next 1/2] overflow: add DECLARE

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: <d4d5324c-8d4b-b2e1-78c8-5c3015b5c03d@intel.com>
Date: Tue, 1 Aug 2023 15:54:29 +0200
From: Alexander Lobakin <aleksander.lobakin@...el.com>
To: Przemek Kitszel <przemyslaw.kitszel@...el.com>
CC: Kees Cook <keescook@...omium.org>, Jacob Keller
	<jacob.e.keller@...el.com>, <intel-wired-lan@...ts.osuosl.org>,
	<netdev@...r.kernel.org>
Subject: Re: [RFC net-next 1/2] overflow: add DECLARE_FLEX() for on-stack
 allocs

From: Przemek Kitszel <przemyslaw.kitszel@...el.com>
Date: Tue, 1 Aug 2023 13:19:22 +0200

> Add DECLARE_FLEX() macro for on-stack allocations of structs with
> flexible array member.
> 
> Using underlying array for on-stack storage lets us to declare known
> on compile-time structures without kzalloc().
> 
> Actual usage for ice driver is in next patch of the series.
> 
> Note that "struct" kw and "*" char is moved to the caller, to both:
> have shorter macro name, and have more natural type specification
> in the driver code (IOW not hiding an actual type of var).
> 
> Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@...el.com>
> ---
>  include/linux/overflow.h | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/include/linux/overflow.h b/include/linux/overflow.h
> index f9b60313eaea..403b7ec120a2 100644
> --- a/include/linux/overflow.h
> +++ b/include/linux/overflow.h
> @@ -309,4 +309,18 @@ static inline size_t __must_check size_sub(size_t minuend, size_t subtrahend)
>  #define struct_size_t(type, member, count)					\
>  	struct_size((type *)NULL, member, count)
>  
> +/**
> + * DECLARE_FLEX() - Declare an on-stack instance of structure with trailing
> + * flexible array.
> + * @type: Pointer to structure type, including "struct" keyword and "*" char.
> + * @name: Name for a (pointer) variable to create.
> + * @member: Name of the array member.
> + * @count: Number of elements in the array; must be compile-time const.
> + *
> + * Declare an instance of structure *@...e with trailing flexible array.
> + */
> +#define DECLARE_FLEX(type, name, member, count)					\
> +	u8 name##_buf[struct_size((type)NULL, member, count)] __aligned(8) = {};\

1. You can use struct_size_t() instead of open-coding it.
2. Maybe use alignof(type) instead of 8? Some structures have larger
   alignment requirements.

> +	type name = (type)&name##_buf

In general, I still think DECLARE_FLEX(struct foo) is better than
DECLARE_FLEX(struct foo *). Looking at container_of(), struct_size_t()
etc., they all take `type`, not `type *`, so even from the consistency
perspective your solution is not optimal to me.

> +
>  #endif /* __LINUX_OVERFLOW_H */

Thanks,
Olek