| Message ID | 20250315031550.473587-1-kees@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | slab: Introduce kmalloc_obj() and family | expand |
Hi Kees, On 3/14/25 8:15 PM, Kees Cook wrote: > diff --git a/include/linux/overflow.h b/include/linux/overflow.h > index 0c7e3dcfe867..e2b81cb5576e 100644 > --- a/include/linux/overflow.h > +++ b/include/linux/overflow.h > @@ -440,4 +440,40 @@ static inline size_t __must_check size_sub(size_t minuend, size_t subtrahend) > #define DEFINE_FLEX(TYPE, NAME, MEMBER, COUNTER, COUNT) \ > _DEFINE_FLEX(TYPE, NAME, MEMBER, COUNT, = { .obj.COUNTER = COUNT, }) > > +/** > + * typeof_flex_counter() - Return the type of the counter variable of a given > + * flexible array member annotated by __counted_by(). > + * @FAM: Pointer to the flexible array member within a given struct. > + * > + * Returns "size_t" if no annotation exists. Please use * Returns: <text> instead so that kernel-doc can make a special doc section for it. Same for patch 2/2. > + */ > +#define typeof_flex_counter(FAM) \ > + typeof(_Generic(__flex_counter(FAM), \ > + void *: (size_t)0, \ > + default: *__flex_counter(FAM))) > + > +/** can_set_flex_counter() - Check if the counter associated with the given Needs a newline between /** and the function name, as in set_flex_counter() below. > + * flexible array member can represent a value. > + * @FAM: Pointer to the flexible array member within a given struct. > + * @COUNT: Value to check against the __counted_by annotated @FAM's counter. > + */ > +#define can_set_flex_counter(FAM, COUNT) \ > + (!overflows_type(COUNT, typeof_flex_counter(FAM))) > + > +/** > + * set_flex_counter() - Set the counter associated with the given flexible > + * array member that has been annoated by __counted_by(). > + * @FAM: Pointer to the flexible array member within a given struct. > + * @COUNT: Value to store to the __counted_by annotated @FAM's counter. > + * > + * This is a no-op if no annotation exists. Count needs to be checked with > + * can_set_flex_counter(FAM, COUNT) before using this function. > + */ > +#define set_flex_counter(FAM, COUNT) \ > +({ \ > + *_Generic(__flex_counter(FAM), \ > + void *: &(size_t){ 0 }, \ > + default: __flex_counter(FAM)) = (COUNT); \ > +}) > + > #endif /* __LINUX_OVERFLOW_H */
On Fri, Mar 14, 2025 at 09:53:41PM -0700, Randy Dunlap wrote: > Hi Kees, > > > On 3/14/25 8:15 PM, Kees Cook wrote: > > > > diff --git a/include/linux/overflow.h b/include/linux/overflow.h > > index 0c7e3dcfe867..e2b81cb5576e 100644 > > --- a/include/linux/overflow.h > > +++ b/include/linux/overflow.h > > @@ -440,4 +440,40 @@ static inline size_t __must_check size_sub(size_t minuend, size_t subtrahend) > > #define DEFINE_FLEX(TYPE, NAME, MEMBER, COUNTER, COUNT) \ > > _DEFINE_FLEX(TYPE, NAME, MEMBER, COUNT, = { .obj.COUNTER = COUNT, }) > > > > +/** > > + * typeof_flex_counter() - Return the type of the counter variable of a given > > + * flexible array member annotated by __counted_by(). > > + * @FAM: Pointer to the flexible array member within a given struct. > > + * > > + * Returns "size_t" if no annotation exists. > > Please use > * Returns: <text> > instead so that kernel-doc can make a special doc section for it. Ah! Thanks -- I hadn't realized that the ":" induced special sections. I think I have a bunch of other kern-doc clean-ups to do as well. > > Same for patch 2/2. > > > + */ > > +#define typeof_flex_counter(FAM) \ > > + typeof(_Generic(__flex_counter(FAM), \ > > + void *: (size_t)0, \ > > + default: *__flex_counter(FAM))) > > + > > +/** can_set_flex_counter() - Check if the counter associated with the given > > Needs a newline between /** and the function name, as in set_flex_counter() below. Whoops, thanks! -Kees
On Sat, Mar 15, 2025 at 4:15 AM Kees Cook <kees@kernel.org> wrote: > > + * clang: https://github.com/llvm/llvm-project/pull/102549 That is an unmerged PR -- should we link to the merged one? Or, better, to the docs, since they seem to exist: https://clang.llvm.org/docs/LanguageExtensions.html#builtin-counted-by-ref ? Thanks! Cheers, Miguel
On Sat, Mar 15, 2025 at 08:47:46PM +0100, Miguel Ojeda wrote: > On Sat, Mar 15, 2025 at 4:15 AM Kees Cook <kees@kernel.org> wrote: > > > > + * clang: https://github.com/llvm/llvm-project/pull/102549 > > That is an unmerged PR -- should we link to the merged one? > > Or, better, to the docs, since they seem to exist: > > https://clang.llvm.org/docs/LanguageExtensions.html#builtin-counted-by-ref Ah! Perfect. I was looking under builtins and couldn't find it. :) I will update the link.
On 3/15/25 04:15, Kees Cook wrote: > Introduce __flex_counter() which wraps __builtin_counted_by_ref(), > as newly introduced by GCC[1] and Clang[2]. Use of __flex_counter() > allows access to the counter member of a struct's flexible array member > when it has been annotated with __counted_by(). > > Introduce typeof_flex_counter(), can_set_flex_counter(), and > set_flex_counter() to provide the needed _Generic() wrappers to get sane > results out of __flex_counter(). > > For example, with: > > struct foo { > int counter; > short array[] __counted_by(counter); > } *p; > > __flex_counter(p->array) will resolve to: &p->counter > > typeof_flex_counter(p->array) will resolve to "int". (If p->array was not > annotated, it would resolve to "size_t".) > > can_set_flex_counter(p->array, COUNT) is the same as: > > COUNT <= type_max(p->counter) && COUNT >= type_min(p->counter) > > (If p->array was not annotated it would return true since everything > fits in size_t.) > > set_flex_counter(p->array, COUNT) is the same as: > > p->counter = COUNT; > > (It is a no-op if p->array is not annotated with __counted_by().) > > Signed-off-by: Kees Cook <kees@kernel.org> I agree that there is no suitable fallback handy, but I see counter as integral part of the struct (in contrast to being merely annotation), IOW, without set_flex_counter() doing the assignment, someone will reference it later anyway, without any warning when kzalloc()'d So, maybe BUILD_BUG() instead of no-op? > +#define set_flex_counter(FAM, COUNT) \ > +({ \ > + *_Generic(__flex_counter(FAM), \ > + void *: &(size_t){ 0 }, \ > + default: __flex_counter(FAM)) = (COUNT); \ > +}) > + > #endif /* __LINUX_OVERFLOW_H */
On 3/17/25 10:26, Przemek Kitszel wrote: > On 3/15/25 04:15, Kees Cook wrote: >> Introduce __flex_counter() which wraps __builtin_counted_by_ref(), >> as newly introduced by GCC[1] and Clang[2]. Use of __flex_counter() >> allows access to the counter member of a struct's flexible array member >> when it has been annotated with __counted_by(). >> >> Introduce typeof_flex_counter(), can_set_flex_counter(), and >> set_flex_counter() to provide the needed _Generic() wrappers to get sane >> results out of __flex_counter(). >> >> For example, with: >> >> struct foo { >> int counter; >> short array[] __counted_by(counter); >> } *p; >> >> __flex_counter(p->array) will resolve to: &p->counter >> >> typeof_flex_counter(p->array) will resolve to "int". (If p->array was not >> annotated, it would resolve to "size_t".) >> >> can_set_flex_counter(p->array, COUNT) is the same as: >> >> COUNT <= type_max(p->counter) && COUNT >= type_min(p->counter) >> >> (If p->array was not annotated it would return true since everything >> fits in size_t.) >> >> set_flex_counter(p->array, COUNT) is the same as: >> >> p->counter = COUNT; >> >> (It is a no-op if p->array is not annotated with __counted_by().) >> >> Signed-off-by: Kees Cook <kees@kernel.org> > > I agree that there is no suitable fallback handy, but I see counter > as integral part of the struct (in contrast to being merely annotation), > IOW, without set_flex_counter() doing the assignment, someone will > reference it later anyway, without any warning when kzalloc()'d > > So, maybe BUILD_BUG() instead of no-op? I get that so far this is only used as an internal helper (in the next patch), so for me it would be also fine to just add __ prefix: __set_flex_counter(), at least until the following is true: "manual initialization of the flexible array counter is still required (at some point) after allocation as not all compiler versions support the __counted_by annotation yet" > >> +#define set_flex_counter(FAM, COUNT) \ >> +({ \ >> + *_Generic(__flex_counter(FAM), \ >> + void *: &(size_t){ 0 }, \ >> + default: __flex_counter(FAM)) = (COUNT); \ >> +}) >> + >> #endif /* __LINUX_OVERFLOW_H */ >
On Mon, Mar 17, 2025 at 10:43:38AM +0100, Przemek Kitszel wrote: > On 3/17/25 10:26, Przemek Kitszel wrote: > > On 3/15/25 04:15, Kees Cook wrote: > > > Introduce __flex_counter() which wraps __builtin_counted_by_ref(), > > > as newly introduced by GCC[1] and Clang[2]. Use of __flex_counter() > > > allows access to the counter member of a struct's flexible array member > > > when it has been annotated with __counted_by(). > > > > > > Introduce typeof_flex_counter(), can_set_flex_counter(), and > > > set_flex_counter() to provide the needed _Generic() wrappers to get sane > > > results out of __flex_counter(). > > > > > > For example, with: > > > > > > struct foo { > > > int counter; > > > short array[] __counted_by(counter); > > > } *p; > > > > > > __flex_counter(p->array) will resolve to: &p->counter > > > > > > typeof_flex_counter(p->array) will resolve to "int". (If p->array was not > > > annotated, it would resolve to "size_t".) > > > > > > can_set_flex_counter(p->array, COUNT) is the same as: > > > > > > COUNT <= type_max(p->counter) && COUNT >= type_min(p->counter) > > > > > > (If p->array was not annotated it would return true since everything > > > fits in size_t.) > > > > > > set_flex_counter(p->array, COUNT) is the same as: > > > > > > p->counter = COUNT; > > > > > > (It is a no-op if p->array is not annotated with __counted_by().) > > > > > > Signed-off-by: Kees Cook <kees@kernel.org> > > > > I agree that there is no suitable fallback handy, but I see counter > > as integral part of the struct (in contrast to being merely annotation), > > IOW, without set_flex_counter() doing the assignment, someone will > > reference it later anyway, without any warning when kzalloc()'d > > > > So, maybe BUILD_BUG() instead of no-op? > > I get that so far this is only used as an internal helper (in the next > patch), so for me it would be also fine to just add __ prefix: > __set_flex_counter(), at least until the following is true: > "manual initialization of the flexible array counter is still > required (at some point) after allocation as not all compiler versions > support the __counted_by annotation yet" Yeah, that's fair. I will rename set_... and can_set_... Thought FWIW I'm not sure we'll ever want a BUILD_BUG_ON() just because there will be flex arrays with future annotations that can't have their counter set (e.g. annotations that indicate globals, expressions, etc -- support for these cases is coming, if slowly[1]). -Kees [1] loooong thread https://gcc.gnu.org/pipermail/gcc-patches/2025-March/677024.html
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h index 981cc3d7e3aa..8b45ecfad5b1 100644 --- a/include/linux/compiler_types.h +++ b/include/linux/compiler_types.h @@ -453,6 +453,37 @@ struct ftrace_likely_data { #define __annotated(var, attr) (false) #endif +/* + * Optional: only supported since gcc >= 15, clang >= 19 + * + * gcc: https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#index-_005f_005fbuiltin_005fcounted_005fby_005fref + * clang: https://github.com/llvm/llvm-project/pull/102549 + */ +#if __has_builtin(__builtin_counted_by_ref) +/** + * __flex_counter() - Get pointer to counter member for the given + * flexible array, if it was annotated with __counted_by() + * @FAM: Pointer to flexible array member of an addressable struct instance + * + * For example, with: + * + * struct foo { + * int counter; + * short array[] __counted_by(counter); + * } *p; + * + * __flex_counter(p->array) will resolve to &p->counter. + * + * Note that Clang may not allow this to be assigned to a separate + * variable; it must be used directly. + * + * If p->array is unannotated, this returns (void *)NULL. + */ +#define __flex_counter(FAM) __builtin_counted_by_ref(FAM) +#else +#define __flex_counter(FAM) ((void *)NULL) +#endif + /* * Some versions of gcc do not mark 'asm goto' volatile: * diff --git a/include/linux/overflow.h b/include/linux/overflow.h index 0c7e3dcfe867..e2b81cb5576e 100644 --- a/include/linux/overflow.h +++ b/include/linux/overflow.h @@ -440,4 +440,40 @@ static inline size_t __must_check size_sub(size_t minuend, size_t subtrahend) #define DEFINE_FLEX(TYPE, NAME, MEMBER, COUNTER, COUNT) \ _DEFINE_FLEX(TYPE, NAME, MEMBER, COUNT, = { .obj.COUNTER = COUNT, }) +/** + * typeof_flex_counter() - Return the type of the counter variable of a given + * flexible array member annotated by __counted_by(). + * @FAM: Pointer to the flexible array member within a given struct. + * + * Returns "size_t" if no annotation exists. + */ +#define typeof_flex_counter(FAM) \ + typeof(_Generic(__flex_counter(FAM), \ + void *: (size_t)0, \ + default: *__flex_counter(FAM))) + +/** can_set_flex_counter() - Check if the counter associated with the given + * flexible array member can represent a value. + * @FAM: Pointer to the flexible array member within a given struct. + * @COUNT: Value to check against the __counted_by annotated @FAM's counter. + */ +#define can_set_flex_counter(FAM, COUNT) \ + (!overflows_type(COUNT, typeof_flex_counter(FAM))) + +/** + * set_flex_counter() - Set the counter associated with the given flexible + * array member that has been annoated by __counted_by(). + * @FAM: Pointer to the flexible array member within a given struct. + * @COUNT: Value to store to the __counted_by annotated @FAM's counter. + * + * This is a no-op if no annotation exists. Count needs to be checked with + * can_set_flex_counter(FAM, COUNT) before using this function. + */ +#define set_flex_counter(FAM, COUNT) \ +({ \ + *_Generic(__flex_counter(FAM), \ + void *: &(size_t){ 0 }, \ + default: __flex_counter(FAM)) = (COUNT); \ +}) + #endif /* __LINUX_OVERFLOW_H */
Introduce __flex_counter() which wraps __builtin_counted_by_ref(), as newly introduced by GCC[1] and Clang[2]. Use of __flex_counter() allows access to the counter member of a struct's flexible array member when it has been annotated with __counted_by(). Introduce typeof_flex_counter(), can_set_flex_counter(), and set_flex_counter() to provide the needed _Generic() wrappers to get sane results out of __flex_counter(). For example, with: struct foo { int counter; short array[] __counted_by(counter); } *p; __flex_counter(p->array) will resolve to: &p->counter typeof_flex_counter(p->array) will resolve to "int". (If p->array was not annotated, it would resolve to "size_t".) can_set_flex_counter(p->array, COUNT) is the same as: COUNT <= type_max(p->counter) && COUNT >= type_min(p->counter) (If p->array was not annotated it would return true since everything fits in size_t.) set_flex_counter(p->array, COUNT) is the same as: p->counter = COUNT; (It is a no-op if p->array is not annotated with __counted_by().) Signed-off-by: Kees Cook <kees@kernel.org> --- Cc: Miguel Ojeda <ojeda@kernel.org> Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org> Cc: Nathan Chancellor <nathan@kernel.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Nick Desaulniers <nick.desaulniers+lkml@gmail.com> Cc: Marco Elver <elver@google.com> Cc: Przemek Kitszel <przemyslaw.kitszel@intel.com> Cc: linux-hardening@vger.kernel.org --- include/linux/compiler_types.h | 31 +++++++++++++++++++++++++++++ include/linux/overflow.h | 36 ++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+)