[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <202503030936.E7B8B4F3E@keescook>
Date: Mon, 3 Mar 2025 09:36:42 -0800
From: Kees Cook <kees@...nel.org>
To: Michael Kelley <mhklinux@...look.com>
Cc: Venkat Rao Bagalkote <venkat88@...ux.vnet.ibm.com>,
Stephen Rothwell <sfr@...b.auug.org.au>,
Christophe Leroy <christophe.leroy@...roup.eu>,
Luc Van Oostenryck <luc.vanoostenryck@...il.com>,
Nathan Chancellor <nathan@...nel.org>,
Miguel Ojeda <ojeda@...nel.org>, Jakub Kicinski <kuba@...nel.org>,
Marco Elver <elver@...gle.com>,
Alexander Potapenko <glider@...gle.com>,
Tony Ambardar <tony.ambardar@...il.com>,
Jan Hendrik Farr <kernel@...rr.cc>,
Alexander Lobakin <aleksander.lobakin@...el.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-hardening@...r.kernel.org" <linux-hardening@...r.kernel.org>
Subject: Re: [PATCH v2] compiler.h: Introduce __must_be_noncstr()
On Sat, Mar 01, 2025 at 05:46:24AM +0000, Michael Kelley wrote:
> From: Kees Cook <kees@...nel.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@...ux.vnet.ibm.com>
> > Reported-by: Stephen Rothwell <sfr@...b.auug.org.au>
> > Reported-by: Christophe Leroy <christophe.leroy@...roup.eu>
> > Reported-by: Michael Kelley <mhklinux@...look.com>
> > Closes: https://lore.kernel.org/all/adbe8dd1-a725-4811-ae7e-76fe770cf096@linux.vnet.ibm.com/
> > Signed-off-by: Kees Cook <kees@...nel.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@...look.com>
Great! Thanks for testing this. :)
--
Kees Cook
Powered by blists - more mailing lists