| Message ID | 20210818060533.3569517-28-keescook@chromium.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | Introduce strict memcpy() bounds checking | expand |
Hi. Le mercredi 18 août 2021, 08:04:57 CEST Kees Cook a écrit : > When commit a28a6e860c6c ("string.h: move fortified functions definitions > in a dedicated header.") moved the fortify-specific code, some helpers > were left behind. Moves the remaining fortify-specific helpers into > fortify-string.h so they're together where they're used. This requires > that any FORTIFY helper function prototypes be conditionally built to > avoid "no prototype" warnings. Additionally removes unused helpers. > > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: Francis Laniel <laniel_francis@privacyrequired.com> > Cc: Daniel Axtens <dja@axtens.net> > Cc: Vincenzo Frascino <vincenzo.frascino@arm.com> > Cc: Andrey Konovalov <andreyknvl@google.com> > Cc: Dan Williams <dan.j.williams@intel.com> > Signed-off-by: Kees Cook <keescook@chromium.org> > --- > include/linux/fortify-string.h | 7 +++++++ > include/linux/string.h | 9 --------- > lib/string_helpers.c | 2 ++ > 3 files changed, 9 insertions(+), 9 deletions(-) > > diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h > index c1be37437e77..7e67d02764db 100644 > --- a/include/linux/fortify-string.h > +++ b/include/linux/fortify-string.h > @@ -2,6 +2,13 @@ > #ifndef _LINUX_FORTIFY_STRING_H_ > #define _LINUX_FORTIFY_STRING_H_ > > +#define __FORTIFY_INLINE extern __always_inline __attribute__((gnu_inline)) > +#define __RENAME(x) __asm__(#x) > + > +void fortify_panic(const char *name) __noreturn __cold; > +void __read_overflow(void) __compiletime_error("detected read beyond size > of object (1st parameter)"); +void __read_overflow2(void) > __compiletime_error("detected read beyond size of object (2nd parameter)"); > +void __write_overflow(void) __compiletime_error("detected write beyond > size of object (1st parameter)"); > > #if defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS) > extern void *__underlying_memchr(const void *p, int c, __kernel_size_t > size) __RENAME(memchr); diff --git a/include/linux/string.h > b/include/linux/string.h > index b48d2d28e0b1..9473f81b9db2 100644 > --- a/include/linux/string.h > +++ b/include/linux/string.h > @@ -249,15 +249,6 @@ static inline const char *kbasename(const char *path) > return tail ? tail + 1 : path; > } > > -#define __FORTIFY_INLINE extern __always_inline __attribute__((gnu_inline)) > -#define __RENAME(x) __asm__(#x) > - > -void fortify_panic(const char *name) __noreturn __cold; > -void __read_overflow(void) __compiletime_error("detected read beyond size > of object passed as 1st parameter"); -void __read_overflow2(void) > __compiletime_error("detected read beyond size of object passed as 2nd > parameter"); -void __read_overflow3(void) __compiletime_error("detected > read beyond size of object passed as 3rd parameter"); -void > __write_overflow(void) __compiletime_error("detected write beyond size of > object passed as 1st parameter"); - > #if !defined(__NO_FORTIFY) && defined(__OPTIMIZE__) && > defined(CONFIG_FORTIFY_SOURCE) #include <linux/fortify-string.h> > #endif > diff --git a/lib/string_helpers.c b/lib/string_helpers.c > index bde13612c25d..faa9d8e4e2c5 100644 > --- a/lib/string_helpers.c > +++ b/lib/string_helpers.c > @@ -883,9 +883,11 @@ char *strreplace(char *s, char old, char new) > } > EXPORT_SYMBOL(strreplace); > > +#ifdef CONFIG_FORTIFY_SOURCE > void fortify_panic(const char *name) > { > pr_emerg("detected buffer overflow in %s\n", name); > BUG(); > } > EXPORT_SYMBOL(fortify_panic); > +#endif /* CONFIG_FORTIFY_SOURCE */ If I remember correctly, I let these helpers in string.h because I thought they could be used by code not related to fortify-string.h. But you are right and I think it is better to have all the code related to one feature in the same place. I am happy to see the kernel is fortifying, and this contribution is good, so here is what I can give: Acked-by: Francis Laniel <laniel_francis@privacyrequired.com> Best regards.
diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h index c1be37437e77..7e67d02764db 100644 --- a/include/linux/fortify-string.h +++ b/include/linux/fortify-string.h @@ -2,6 +2,13 @@ #ifndef _LINUX_FORTIFY_STRING_H_ #define _LINUX_FORTIFY_STRING_H_ +#define __FORTIFY_INLINE extern __always_inline __attribute__((gnu_inline)) +#define __RENAME(x) __asm__(#x) + +void fortify_panic(const char *name) __noreturn __cold; +void __read_overflow(void) __compiletime_error("detected read beyond size of object (1st parameter)"); +void __read_overflow2(void) __compiletime_error("detected read beyond size of object (2nd parameter)"); +void __write_overflow(void) __compiletime_error("detected write beyond size of object (1st parameter)"); #if defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS) extern void *__underlying_memchr(const void *p, int c, __kernel_size_t size) __RENAME(memchr); diff --git a/include/linux/string.h b/include/linux/string.h index b48d2d28e0b1..9473f81b9db2 100644 --- a/include/linux/string.h +++ b/include/linux/string.h @@ -249,15 +249,6 @@ static inline const char *kbasename(const char *path) return tail ? tail + 1 : path; } -#define __FORTIFY_INLINE extern __always_inline __attribute__((gnu_inline)) -#define __RENAME(x) __asm__(#x) - -void fortify_panic(const char *name) __noreturn __cold; -void __read_overflow(void) __compiletime_error("detected read beyond size of object passed as 1st parameter"); -void __read_overflow2(void) __compiletime_error("detected read beyond size of object passed as 2nd parameter"); -void __read_overflow3(void) __compiletime_error("detected read beyond size of object passed as 3rd parameter"); -void __write_overflow(void) __compiletime_error("detected write beyond size of object passed as 1st parameter"); - #if !defined(__NO_FORTIFY) && defined(__OPTIMIZE__) && defined(CONFIG_FORTIFY_SOURCE) #include <linux/fortify-string.h> #endif diff --git a/lib/string_helpers.c b/lib/string_helpers.c index bde13612c25d..faa9d8e4e2c5 100644 --- a/lib/string_helpers.c +++ b/lib/string_helpers.c @@ -883,9 +883,11 @@ char *strreplace(char *s, char old, char new) } EXPORT_SYMBOL(strreplace); +#ifdef CONFIG_FORTIFY_SOURCE void fortify_panic(const char *name) { pr_emerg("detected buffer overflow in %s\n", name); BUG(); } EXPORT_SYMBOL(fortify_panic); +#endif /* CONFIG_FORTIFY_SOURCE */
When commit a28a6e860c6c ("string.h: move fortified functions definitions in a dedicated header.") moved the fortify-specific code, some helpers were left behind. Moves the remaining fortify-specific helpers into fortify-string.h so they're together where they're used. This requires that any FORTIFY helper function prototypes be conditionally built to avoid "no prototype" warnings. Additionally removes unused helpers. Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Francis Laniel <laniel_francis@privacyrequired.com> Cc: Daniel Axtens <dja@axtens.net> Cc: Vincenzo Frascino <vincenzo.frascino@arm.com> Cc: Andrey Konovalov <andreyknvl@google.com> Cc: Dan Williams <dan.j.williams@intel.com> Signed-off-by: Kees Cook <keescook@chromium.org> --- include/linux/fortify-string.h | 7 +++++++ include/linux/string.h | 9 --------- lib/string_helpers.c | 2 ++ 3 files changed, 9 insertions(+), 9 deletions(-)