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
| ||
|
Message-ID: <12cda14d-a360-f4b0-14f1-11d74f3262f9@intel.com> Date: Fri, 11 Aug 2023 11:10:56 +0200 From: Przemek Kitszel <przemyslaw.kitszel@...el.com> To: Kees Cook <keescook@...omium.org> CC: <netdev@...r.kernel.org>, Jacob Keller <jacob.e.keller@...el.com>, <intel-wired-lan@...ts.osuosl.org>, Alexander Lobakin <aleksander.lobakin@...el.com>, <linux-hardening@...r.kernel.org>, Steven Zou <steven.zou@...el.com> Subject: Re: [PATCH net-next v1 1/7] overflow: add DEFINE_FLEX() for on-stack allocs On 8/10/23 20:46, Kees Cook wrote: > On Thu, Aug 10, 2023 at 06:35:03AM -0400, Przemek Kitszel wrote: >> Add DEFINE_FLEX() macro for on-stack allocations of structs with >> flexible array member. >> >> Add also const_flex_size() macro, that reads size of structs >> allocated by DEFINE_FLEX(). >> >> Using underlying array for on-stack storage lets us to declare >> known-at-compile-time structures without kzalloc(). >> >> Actual usage for ice driver is in following patches of the series. >> >> Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@...el.com> >> --- >> v1: change macro name; add macro for size read; >> accept struct type instead of ptr to it; change alignment; >> --- >> include/linux/overflow.h | 27 +++++++++++++++++++++++++++ >> 1 file changed, 27 insertions(+) >> >> diff --git a/include/linux/overflow.h b/include/linux/overflow.h >> index f9b60313eaea..21a4410799eb 100644 >> --- a/include/linux/overflow.h >> +++ b/include/linux/overflow.h >> @@ -309,4 +309,31 @@ 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) >> >> +/** >> + * DEFINE_FLEX() - Define a zeroed, on-stack, instance of @type structure with >> + * a trailing flexible array member. >> + * >> + * @type: structure type name, including "struct" keyword. >> + * @name: Name for a variable to define. >> + * @member: Name of the array member. >> + * @count: Number of elements in the array; must be compile-time const. >> + */ >> +#define DEFINE_FLEX(type, name, member, count) \ >> + union { \ >> + u8 bytes[struct_size_t(type, member, count)]; \ >> + type obj; \ >> + } name##_u __aligned(_Alignof(type)) = {}; \ >> + type *name = (type *)&name##_u > > We'll need another macro when __counted_by is needed, but yes, if all of > these structs use non-native endian counters, we can't require it in the > base macro. (i.e. not now -- this is fine as-is.) > >> + >> +/** >> + * const_flex_size() - Get size of on-stack instance of structure with >> + * a trailing flexible array member. >> + * >> + * @name: Name of the variable, the one defined by DEFINE_FLEX() macro above. >> + * >> + * Get size of @name, which is equivalent to struct_size(name, array, count), >> + * but does not require (repeating) last two arguments. >> + */ >> +#define const_flex_size(name) __builtin_object_size(name, 1) > > Naming is hard. ;) I don't like "const" here (it's not a storage > class). But more importantly, this calculation ("how big is this thing > actually?") gets used a lot in the fortify routines, so I'd prefer > exposing those macros (from fortify-string.h): > > > diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h > index c88488715a39..4b788fa0c576 100644 > --- a/include/linux/compiler_types.h > +++ b/include/linux/compiler_types.h > @@ -352,6 +352,18 @@ struct ftrace_likely_data { > # define __realloc_size(x, ...) > #endif > > +/* > + * When the size of an allocated object is needed, use the best available > + * mechanism to find it. (For cases where sizeof() cannot be used.) > + */ > +#if __has_builtin(__builtin_dynamic_object_size) > +#define __struct_size(p) __builtin_dynamic_object_size(p, 0) > +#define __member_size(p) __builtin_dynamic_object_size(p, 1) > +#else > +#define __struct_size(p) __builtin_object_size(p, 0) > +#define __member_size(p) __builtin_object_size(p, 1) > +#endif > + > #ifndef asm_volatile_goto > #define asm_volatile_goto(x...) asm goto(x) > #endif > diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h > index da51a83b2829..1e7711185ec6 100644 > --- a/include/linux/fortify-string.h > +++ b/include/linux/fortify-string.h > @@ -93,13 +93,9 @@ extern char *__underlying_strncpy(char *p, const char *q, __kernel_size_t size) > #if __has_builtin(__builtin_dynamic_object_size) > #define POS __pass_dynamic_object_size(1) > #define POS0 __pass_dynamic_object_size(0) > -#define __struct_size(p) __builtin_dynamic_object_size(p, 0) > -#define __member_size(p) __builtin_dynamic_object_size(p, 1) > #else > #define POS __pass_object_size(1) > #define POS0 __pass_object_size(0) > -#define __struct_size(p) __builtin_object_size(p, 0) > -#define __member_size(p) __builtin_object_size(p, 1) > #endif > > #define __compiletime_lessthan(bounds, length) ( \ > > > And the way DEFINE_FLEX is built, __struct_size() and __member_size() > will give the same result (which is what I was concerned about for > FORTIFY_SOURCE's use of __member_size not "seeing" the flexible array > members). > > In this case, I think using __struct_size() in place of const_flex_size() > in the patch series is the way to go. > Thanks! I was a bit too afraid to move it out of fortify-string.h, I guess whole family have to go together (to keep related code close to each other)?
Powered by blists - more mailing lists