| Message ID | 20250228174130.it.875-kees@kernel.org (mailing list archive) |
|---|---|
| State | In Next |
| Commit | 76fe1a22e4f58db10dd14af024b86feeac1dcca0 |
| Headers | show |
| Series | [v2] compiler.h: Introduce __must_be_noncstr() | expand |
From: Kees Cook <kees@kernel.org> Sent: Friday, February 28, 2025 9:42 AM > > In preparation for adding more type checking to the memtostr/strtomem*() > helpers, introduce the ability to check for the "nonstring" attribute. > This is the reverse of what was added to strscpy*() in commit 559048d156ff > ("string: Check for "nonstring" attribute on strscpy() arguments"). > > Note that __annotated() must be explicitly tested for, as GCC added > __builtin_has_attribute() after it added the "nonstring" attribute. Do > so here to avoid the !__annotated() test triggering build failures > when __builtin_has_attribute() was missing but __nonstring was defined. > (I've opted to squash this fix into this patch so we don't end up with > a possible bisection target that would leave the kernel unbuildable.) > > Reported-by: Venkat Rao Bagalkote <venkat88@linux.vnet.ibm.com> > Reported-by: Stephen Rothwell <sfr@canb.auug.org.au> > Reported-by: Christophe Leroy <christophe.leroy@csgroup.eu> > Reported-by: Michael Kelley <mhklinux@outlook.com> > Closes: https://lore.kernel.org/all/adbe8dd1-a725-4811-ae7e-76fe770cf096@linux.vnet.ibm.com/ > Signed-off-by: Kees Cook <kees@kernel.org> > --- > include/linux/compiler.h | 18 +++++++++++++++++- > include/linux/compiler_types.h | 9 ++++++--- > 2 files changed, 23 insertions(+), 4 deletions(-) > > diff --git a/include/linux/compiler.h b/include/linux/compiler.h > index 200fd3c5bc70..d5201464c5e6 100644 > --- a/include/linux/compiler.h > +++ b/include/linux/compiler.h > @@ -206,9 +206,25 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val, > #define __must_be_byte_array(a) > __BUILD_BUG_ON_ZERO_MSG(!__is_byte_array(a), \ > "must be byte array") > > +/* > + * If the "nonstring" attribute isn't available, we have to return true > + * so the __must_*() checks pass when "nonstring" isn't supported. > + */ > +#if __has_attribute(__nonstring__) && defined(__annotated) > +#define __is_cstr(a) (!__annotated(a, nonstring)) > +#define __is_noncstr(a) (__annotated(a, nonstring)) > +#else > +#define __is_cstr(a) (true) > +#define __is_noncstr(a) (true) > +#endif > + > /* Require C Strings (i.e. NUL-terminated) lack the "nonstring" attribute. */ > #define __must_be_cstr(p) \ > - __BUILD_BUG_ON_ZERO_MSG(__annotated(p, nonstring), "must be cstr (NUL- > terminated)") > + __BUILD_BUG_ON_ZERO_MSG(!__is_cstr(p), \ > + "must be C-string (NUL-terminated)") > +#define __must_be_noncstr(p) \ > + __BUILD_BUG_ON_ZERO_MSG(!__is_noncstr(p), \ > + "must be non-C-string (not NUL-terminated)") > > #endif /* __KERNEL__ */ > > diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h > index 981cc3d7e3aa..f59393464ea7 100644 > --- a/include/linux/compiler_types.h > +++ b/include/linux/compiler_types.h > @@ -446,11 +446,14 @@ struct ftrace_likely_data { > #define __member_size(p) __builtin_object_size(p, 1) > #endif > > -/* Determine if an attribute has been applied to a variable. */ > +/* > + * Determine if an attribute has been applied to a variable. > + * Using __annotated needs to check for __annotated being available, > + * or negative tests may fail when annotation cannot be checked. For > + * example, see the definition of __is_cstr(). > + */ > #if __has_builtin(__builtin_has_attribute) > #define __annotated(var, attr) __builtin_has_attribute(var, attr) > -#else > -#define __annotated(var, attr) (false) > #endif > > /* > -- > 2.34.1 > Compile tested on x86 with gcc 9.4.0, and it resolves the compile problem I was originally seeing with linux-next-20250226. Tested-by: Michael Kelley <mhklinux@outlook.com>
On Sat, Mar 01, 2025 at 05:46:24AM +0000, Michael Kelley wrote: > From: Kees Cook <kees@kernel.org> Sent: Friday, February 28, 2025 9:42 AM > > > > In preparation for adding more type checking to the memtostr/strtomem*() > > helpers, introduce the ability to check for the "nonstring" attribute. > > This is the reverse of what was added to strscpy*() in commit 559048d156ff > > ("string: Check for "nonstring" attribute on strscpy() arguments"). > > > > Note that __annotated() must be explicitly tested for, as GCC added > > __builtin_has_attribute() after it added the "nonstring" attribute. Do > > so here to avoid the !__annotated() test triggering build failures > > when __builtin_has_attribute() was missing but __nonstring was defined. > > (I've opted to squash this fix into this patch so we don't end up with > > a possible bisection target that would leave the kernel unbuildable.) > > > > Reported-by: Venkat Rao Bagalkote <venkat88@linux.vnet.ibm.com> > > Reported-by: Stephen Rothwell <sfr@canb.auug.org.au> > > Reported-by: Christophe Leroy <christophe.leroy@csgroup.eu> > > Reported-by: Michael Kelley <mhklinux@outlook.com> > > Closes: https://lore.kernel.org/all/adbe8dd1-a725-4811-ae7e-76fe770cf096@linux.vnet.ibm.com/ > > Signed-off-by: Kees Cook <kees@kernel.org> > > --- > > include/linux/compiler.h | 18 +++++++++++++++++- > > include/linux/compiler_types.h | 9 ++++++--- > > 2 files changed, 23 insertions(+), 4 deletions(-) > > > > diff --git a/include/linux/compiler.h b/include/linux/compiler.h > > index 200fd3c5bc70..d5201464c5e6 100644 > > --- a/include/linux/compiler.h > > +++ b/include/linux/compiler.h > > @@ -206,9 +206,25 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val, > > #define __must_be_byte_array(a) > > __BUILD_BUG_ON_ZERO_MSG(!__is_byte_array(a), \ > > "must be byte array") > > > > +/* > > + * If the "nonstring" attribute isn't available, we have to return true > > + * so the __must_*() checks pass when "nonstring" isn't supported. > > + */ > > +#if __has_attribute(__nonstring__) && defined(__annotated) > > +#define __is_cstr(a) (!__annotated(a, nonstring)) > > +#define __is_noncstr(a) (__annotated(a, nonstring)) > > +#else > > +#define __is_cstr(a) (true) > > +#define __is_noncstr(a) (true) > > +#endif > > + > > /* Require C Strings (i.e. NUL-terminated) lack the "nonstring" attribute. */ > > #define __must_be_cstr(p) \ > > - __BUILD_BUG_ON_ZERO_MSG(__annotated(p, nonstring), "must be cstr (NUL- > > terminated)") > > + __BUILD_BUG_ON_ZERO_MSG(!__is_cstr(p), \ > > + "must be C-string (NUL-terminated)") > > +#define __must_be_noncstr(p) \ > > + __BUILD_BUG_ON_ZERO_MSG(!__is_noncstr(p), \ > > + "must be non-C-string (not NUL-terminated)") > > > > #endif /* __KERNEL__ */ > > > > diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h > > index 981cc3d7e3aa..f59393464ea7 100644 > > --- a/include/linux/compiler_types.h > > +++ b/include/linux/compiler_types.h > > @@ -446,11 +446,14 @@ struct ftrace_likely_data { > > #define __member_size(p) __builtin_object_size(p, 1) > > #endif > > > > -/* Determine if an attribute has been applied to a variable. */ > > +/* > > + * Determine if an attribute has been applied to a variable. > > + * Using __annotated needs to check for __annotated being available, > > + * or negative tests may fail when annotation cannot be checked. For > > + * example, see the definition of __is_cstr(). > > + */ > > #if __has_builtin(__builtin_has_attribute) > > #define __annotated(var, attr) __builtin_has_attribute(var, attr) > > -#else > > -#define __annotated(var, attr) (false) > > #endif > > > > /* > > -- > > 2.34.1 > > > > Compile tested on x86 with gcc 9.4.0, and it resolves the compile > problem I was originally seeing with linux-next-20250226. > > Tested-by: Michael Kelley <mhklinux@outlook.com> Great! Thanks for testing this. :)
diff --git a/include/linux/compiler.h b/include/linux/compiler.h index 200fd3c5bc70..d5201464c5e6 100644 --- a/include/linux/compiler.h +++ b/include/linux/compiler.h @@ -206,9 +206,25 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val, #define __must_be_byte_array(a) __BUILD_BUG_ON_ZERO_MSG(!__is_byte_array(a), \ "must be byte array") +/* + * If the "nonstring" attribute isn't available, we have to return true + * so the __must_*() checks pass when "nonstring" isn't supported. + */ +#if __has_attribute(__nonstring__) && defined(__annotated) +#define __is_cstr(a) (!__annotated(a, nonstring)) +#define __is_noncstr(a) (__annotated(a, nonstring)) +#else +#define __is_cstr(a) (true) +#define __is_noncstr(a) (true) +#endif + /* Require C Strings (i.e. NUL-terminated) lack the "nonstring" attribute. */ #define __must_be_cstr(p) \ - __BUILD_BUG_ON_ZERO_MSG(__annotated(p, nonstring), "must be cstr (NUL-terminated)") + __BUILD_BUG_ON_ZERO_MSG(!__is_cstr(p), \ + "must be C-string (NUL-terminated)") +#define __must_be_noncstr(p) \ + __BUILD_BUG_ON_ZERO_MSG(!__is_noncstr(p), \ + "must be non-C-string (not NUL-terminated)") #endif /* __KERNEL__ */ diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h index 981cc3d7e3aa..f59393464ea7 100644 --- a/include/linux/compiler_types.h +++ b/include/linux/compiler_types.h @@ -446,11 +446,14 @@ struct ftrace_likely_data { #define __member_size(p) __builtin_object_size(p, 1) #endif -/* Determine if an attribute has been applied to a variable. */ +/* + * Determine if an attribute has been applied to a variable. + * Using __annotated needs to check for __annotated being available, + * or negative tests may fail when annotation cannot be checked. For + * example, see the definition of __is_cstr(). + */ #if __has_builtin(__builtin_has_attribute) #define __annotated(var, attr) __builtin_has_attribute(var, attr) -#else -#define __annotated(var, attr) (false) #endif /*
In preparation for adding more type checking to the memtostr/strtomem*() helpers, introduce the ability to check for the "nonstring" attribute. This is the reverse of what was added to strscpy*() in commit 559048d156ff ("string: Check for "nonstring" attribute on strscpy() arguments"). Note that __annotated() must be explicitly tested for, as GCC added __builtin_has_attribute() after it added the "nonstring" attribute. Do so here to avoid the !__annotated() test triggering build failures when __builtin_has_attribute() was missing but __nonstring was defined. (I've opted to squash this fix into this patch so we don't end up with a possible bisection target that would leave the kernel unbuildable.) Reported-by: Venkat Rao Bagalkote <venkat88@linux.vnet.ibm.com> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au> Reported-by: Christophe Leroy <christophe.leroy@csgroup.eu> Reported-by: Michael Kelley <mhklinux@outlook.com> Closes: https://lore.kernel.org/all/adbe8dd1-a725-4811-ae7e-76fe770cf096@linux.vnet.ibm.com/ Signed-off-by: Kees Cook <kees@kernel.org> --- include/linux/compiler.h | 18 +++++++++++++++++- include/linux/compiler_types.h | 9 ++++++--- 2 files changed, 23 insertions(+), 4 deletions(-)