| Message ID | 20250206181133.3450635-2-kees@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | string.h: Use ARRAY_SIZE() for memtostr*()/strtomem*() | expand |
On Thu, 6 Feb 2025 10:11:29 -0800 Kees Cook <kees@kernel.org> wrote: > In preparation for adding stricter type checking to the str/mem*() > helpers, provide a way to check that a variable is a character array > via __must_be_char_array(). > ... > +#define __is_char_array(a) (__is_array(a) && sizeof((a)[0]) == 1) > +#define __must_be_char_array(a) __BUILD_BUG_ON_ZERO_MSG(!__is_char_array(a), \ > + "must be byte array") If you are only checking the size, perhaps __is_byte_array() would be a better name. (You've even used 'byte' in the error message.) David
On Thu, Feb 06, 2025 at 10:11:29AM -0800, Kees Cook wrote: > In preparation for adding stricter type checking to the str/mem*() > helpers, provide a way to check that a variable is a character array > via __must_be_char_array(). > > Signed-off-by: Kees Cook <kees@kernel.org> Suggested-by? :) > --- > include/linux/compiler.h | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/include/linux/compiler.h b/include/linux/compiler.h > index 7af999a131cb..a577fe0b1f8a 100644 > --- a/include/linux/compiler.h > +++ b/include/linux/compiler.h > @@ -221,7 +221,13 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val, > #endif /* __CHECKER__ */ > > /* &a[0] degrades to a pointer: a different type from an array */ > -#define __must_be_array(a) __BUILD_BUG_ON_ZERO_MSG(__same_type((a), &(a)[0]), "must be array") > +#define __is_array(a) (!__same_type((a), &(a)[0])) > +#define __must_be_array(a) __BUILD_BUG_ON_ZERO_MSG(!__is_array(a), \ > + "must be array") > + > +#define __is_char_array(a) (__is_array(a) && sizeof((a)[0]) == 1) > +#define __must_be_char_array(a) __BUILD_BUG_ON_ZERO_MSG(!__is_char_array(a), \ > + "must be byte array") > > /* Require C Strings (i.e. NUL-terminated) lack the "nonstring" attribute. */ > #define __must_be_cstr(p) \ > -- > 2.34.1 >
On Thu, Feb 06, 2025 at 03:50:47PM -0500, Kent Overstreet wrote: > On Thu, Feb 06, 2025 at 10:11:29AM -0800, Kees Cook wrote: > > In preparation for adding stricter type checking to the str/mem*() > > helpers, provide a way to check that a variable is a character array > > via __must_be_char_array(). > > > > Signed-off-by: Kees Cook <kees@kernel.org> > > Suggested-by? :) Sure, I'll add that. I did it for the ARRAY_SIZE() and forgot which pieces came from where when I split up the patches. :)
On Thu, Feb 06, 2025 at 07:56:58PM +0000, David Laight wrote: > On Thu, 6 Feb 2025 10:11:29 -0800 > Kees Cook <kees@kernel.org> wrote: > > > In preparation for adding stricter type checking to the str/mem*() > > helpers, provide a way to check that a variable is a character array > > via __must_be_char_array(). > > > ... > > +#define __is_char_array(a) (__is_array(a) && sizeof((a)[0]) == 1) > > +#define __must_be_char_array(a) __BUILD_BUG_ON_ZERO_MSG(!__is_char_array(a), \ > > + "must be byte array") > > If you are only checking the size, perhaps __is_byte_array() would > be a better name. > (You've even used 'byte' in the error message.) Yeah, fair point. I went and forth on naming. Fixed for v2.
On Thu, Feb 06 2025, Kees Cook <kees@kernel.org> wrote: > In preparation for adding stricter type checking to the str/mem*() > helpers, provide a way to check that a variable is a character array > via __must_be_char_array(). > > Signed-off-by: Kees Cook <kees@kernel.org> > --- > include/linux/compiler.h | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/include/linux/compiler.h b/include/linux/compiler.h > index 7af999a131cb..a577fe0b1f8a 100644 > --- a/include/linux/compiler.h > +++ b/include/linux/compiler.h > @@ -221,7 +221,13 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val, > #endif /* __CHECKER__ */ > > /* &a[0] degrades to a pointer: a different type from an array */ > -#define __must_be_array(a) __BUILD_BUG_ON_ZERO_MSG(__same_type((a), &(a)[0]), "must be array") > +#define __is_array(a) (!__same_type((a), &(a)[0])) > +#define __must_be_array(a) __BUILD_BUG_ON_ZERO_MSG(!__is_array(a), \ > + "must be array") > + > +#define __is_char_array(a) (__is_array(a) && sizeof((a)[0]) == 1) > +#define __must_be_char_array(a) __BUILD_BUG_ON_ZERO_MSG(!__is_char_array(a), \ > + "must be byte array") > It's probably unlikely to ever encounter an array of _Bool or array of structs-with-a-single-char-member in the wild, but it does seem a bit odd to base the test on sizeof(). Why not add a __is_character_type(t) (__same_type(t, char) || __same_type(t, signed char) || __same_type(t, unsigned char) ) helper and write the test using __is_character_type((a)[0])? Or if you really mean that it must be an array of char, not any of the three "character types", simply replace the sizeof() by __same_type(a[0], char) Rasmus
On Fri, 07 Feb 2025 09:55:00 +0100 Rasmus Villemoes <ravi@prevas.dk> wrote: > On Thu, Feb 06 2025, Kees Cook <kees@kernel.org> wrote: > > > In preparation for adding stricter type checking to the str/mem*() > > helpers, provide a way to check that a variable is a character array > > via __must_be_char_array(). > > > > Signed-off-by: Kees Cook <kees@kernel.org> > > --- > > include/linux/compiler.h | 8 +++++++- > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/include/linux/compiler.h b/include/linux/compiler.h > > index 7af999a131cb..a577fe0b1f8a 100644 > > --- a/include/linux/compiler.h > > +++ b/include/linux/compiler.h > > @@ -221,7 +221,13 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val, > > #endif /* __CHECKER__ */ > > > > /* &a[0] degrades to a pointer: a different type from an array */ > > -#define __must_be_array(a) __BUILD_BUG_ON_ZERO_MSG(__same_type((a), &(a)[0]), "must be array") > > +#define __is_array(a) (!__same_type((a), &(a)[0])) > > +#define __must_be_array(a) __BUILD_BUG_ON_ZERO_MSG(!__is_array(a), \ > > + "must be array") > > + > > +#define __is_char_array(a) (__is_array(a) && sizeof((a)[0]) == 1) > > +#define __must_be_char_array(a) __BUILD_BUG_ON_ZERO_MSG(!__is_char_array(a), \ > > + "must be byte array") > > > > It's probably unlikely to ever encounter an array of _Bool or array of > structs-with-a-single-char-member in the wild, but it does seem a bit > odd to base the test on sizeof(). Why not add a > > __is_character_type(t) (__same_type(t, char) || > __same_type(t, signed char) || > __same_type(t, unsigned char) ) > > helper and write the test using __is_character_type((a)[0])? > > Or if you really mean that it must be an array of char, not any of the > three "character types", simply replace the sizeof() by > __same_type(a[0], char) I'm not sure it matters enough to slow down the compilation by that much cpp bloat. David
On Thu, Feb 06, 2025 at 10:11:29AM -0800, Kees Cook wrote: > In preparation for adding stricter type checking to the str/mem*() > helpers, provide a way to check that a variable is a character array > via __must_be_char_array(). > > Signed-off-by: Kees Cook <kees@kernel.org> > --- > include/linux/compiler.h | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/include/linux/compiler.h b/include/linux/compiler.h > index 7af999a131cb..a577fe0b1f8a 100644 > --- a/include/linux/compiler.h > +++ b/include/linux/compiler.h > @@ -221,7 +221,13 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val, > #endif /* __CHECKER__ */ > > /* &a[0] degrades to a pointer: a different type from an array */ > -#define __must_be_array(a) __BUILD_BUG_ON_ZERO_MSG(__same_type((a), &(a)[0]), "must be array") > +#define __is_array(a) (!__same_type((a), &(a)[0])) > +#define __must_be_array(a) __BUILD_BUG_ON_ZERO_MSG(!__is_array(a), \ > + "must be array") > + > +#define __is_char_array(a) (__is_array(a) && sizeof((a)[0]) == 1) > +#define __must_be_char_array(a) __BUILD_BUG_ON_ZERO_MSG(!__is_char_array(a), \ > + "must be byte array") > > /* Require C Strings (i.e. NUL-terminated) lack the "nonstring" attribute. */ > #define __must_be_cstr(p) \ > -- > 2.34.1 > Why not just use a combination of ARRAY_SIZE() and a function call (to verify that it's a character array) for the typechecking?
diff --git a/include/linux/compiler.h b/include/linux/compiler.h index 7af999a131cb..a577fe0b1f8a 100644 --- a/include/linux/compiler.h +++ b/include/linux/compiler.h @@ -221,7 +221,13 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val, #endif /* __CHECKER__ */ /* &a[0] degrades to a pointer: a different type from an array */ -#define __must_be_array(a) __BUILD_BUG_ON_ZERO_MSG(__same_type((a), &(a)[0]), "must be array") +#define __is_array(a) (!__same_type((a), &(a)[0])) +#define __must_be_array(a) __BUILD_BUG_ON_ZERO_MSG(!__is_array(a), \ + "must be array") + +#define __is_char_array(a) (__is_array(a) && sizeof((a)[0]) == 1) +#define __must_be_char_array(a) __BUILD_BUG_ON_ZERO_MSG(!__is_char_array(a), \ + "must be byte array") /* Require C Strings (i.e. NUL-terminated) lack the "nonstring" attribute. */ #define __must_be_cstr(p) \
In preparation for adding stricter type checking to the str/mem*() helpers, provide a way to check that a variable is a character array via __must_be_char_array(). Signed-off-by: Kees Cook <kees@kernel.org> --- include/linux/compiler.h | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)