| Message ID | 20240517130118.759301-1-andrey.konovalov@linux.dev (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | kasan, fortify: properly rename memintrinsics | expand |
On Fri, 17 May 2024 at 15:01, <andrey.konovalov@linux.dev> wrote: > > From: Andrey Konovalov <andreyknvl@gmail.com> > > After commit 69d4c0d32186 ("entry, kasan, x86: Disallow overriding mem*() > functions") and the follow-up fixes, with CONFIG_FORTIFY_SOURCE enabled, > even though the compiler instruments meminstrinsics by generating calls > to __asan/__hwasan_ prefixed functions, FORTIFY_SOURCE still uses > uninstrumented memset/memmove/memcpy as the underlying functions. > > As a result, KASAN cannot detect bad accesses in memset/memmove/memcpy. > This also makes KASAN tests corrupt kernel memory and cause crashes. > > To fix this, use __asan_/__hwasan_memset/memmove/memcpy as the underlying > functions whenever appropriate. Do this only for the instrumented code > (as indicated by __SANITIZE_ADDRESS__). > > Reported-by: Erhard Furtner <erhard_f@mailbox.org> > Reported-by: Nico Pache <npache@redhat.com> > Closes: https://lore.kernel.org/all/20240501144156.17e65021@outsider.home/ > Fixes: 69d4c0d32186 ("entry, kasan, x86: Disallow overriding mem*() functions") > Fixes: 51287dcb00cc ("kasan: emit different calls for instrumentable memintrinsics") > Fixes: 36be5cba99f6 ("kasan: treat meminstrinsic as builtins in uninstrumented files") > Signed-off-by: Andrey Konovalov <andreyknvl@gmail.com> Reviewed-by: Marco Elver <elver@google.com> This is getting rather complex, but I don't see a better way either. > --- > include/linux/fortify-string.h | 22 ++++++++++++++++++---- > 1 file changed, 18 insertions(+), 4 deletions(-) > > diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h > index 85fc0e6f0f7f..bac010cfc42f 100644 > --- a/include/linux/fortify-string.h > +++ b/include/linux/fortify-string.h > @@ -75,17 +75,30 @@ void __write_overflow_field(size_t avail, size_t wanted) __compiletime_warning(" > __ret; \ > }) > > -#if defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS) > +#if defined(__SANITIZE_ADDRESS__) > + > +#if !defined(CONFIG_CC_HAS_KASAN_MEMINTRINSIC_PREFIX) && !defined(CONFIG_GENERIC_ENTRY) > +extern void *__underlying_memset(void *p, int c, __kernel_size_t size) __RENAME(memset); > +extern void *__underlying_memmove(void *p, const void *q, __kernel_size_t size) __RENAME(memmove); > +extern void *__underlying_memcpy(void *p, const void *q, __kernel_size_t size) __RENAME(memcpy); > +#elif defined(CONFIG_KASAN_GENERIC) > +extern void *__underlying_memset(void *p, int c, __kernel_size_t size) __RENAME(__asan_memset); > +extern void *__underlying_memmove(void *p, const void *q, __kernel_size_t size) __RENAME(__asan_memmove); > +extern void *__underlying_memcpy(void *p, const void *q, __kernel_size_t size) __RENAME(__asan_memcpy); > +#else /* CONFIG_KASAN_SW_TAGS */ > +extern void *__underlying_memset(void *p, int c, __kernel_size_t size) __RENAME(__hwasan_memset); > +extern void *__underlying_memmove(void *p, const void *q, __kernel_size_t size) __RENAME(__hwasan_memmove); > +extern void *__underlying_memcpy(void *p, const void *q, __kernel_size_t size) __RENAME(__hwasan_memcpy); > +#endif > + > extern void *__underlying_memchr(const void *p, int c, __kernel_size_t size) __RENAME(memchr); > extern int __underlying_memcmp(const void *p, const void *q, __kernel_size_t size) __RENAME(memcmp); > -extern void *__underlying_memcpy(void *p, const void *q, __kernel_size_t size) __RENAME(memcpy); > -extern void *__underlying_memmove(void *p, const void *q, __kernel_size_t size) __RENAME(memmove); > -extern void *__underlying_memset(void *p, int c, __kernel_size_t size) __RENAME(memset); > extern char *__underlying_strcat(char *p, const char *q) __RENAME(strcat); > extern char *__underlying_strcpy(char *p, const char *q) __RENAME(strcpy); > extern __kernel_size_t __underlying_strlen(const char *p) __RENAME(strlen); > extern char *__underlying_strncat(char *p, const char *q, __kernel_size_t count) __RENAME(strncat); > extern char *__underlying_strncpy(char *p, const char *q, __kernel_size_t size) __RENAME(strncpy); > + > #else > > #if defined(__SANITIZE_MEMORY__) > @@ -110,6 +123,7 @@ extern char *__underlying_strncpy(char *p, const char *q, __kernel_size_t size) > #define __underlying_strlen __builtin_strlen > #define __underlying_strncat __builtin_strncat > #define __underlying_strncpy __builtin_strncpy > + > #endif > > /** > -- > 2.25.1 >
On Fri, May 17, 2024 at 7:02 AM <andrey.konovalov@linux.dev> wrote: > > From: Andrey Konovalov <andreyknvl@gmail.com> > > After commit 69d4c0d32186 ("entry, kasan, x86: Disallow overriding mem*() > functions") and the follow-up fixes, with CONFIG_FORTIFY_SOURCE enabled, > even though the compiler instruments meminstrinsics by generating calls > to __asan/__hwasan_ prefixed functions, FORTIFY_SOURCE still uses > uninstrumented memset/memmove/memcpy as the underlying functions. > > As a result, KASAN cannot detect bad accesses in memset/memmove/memcpy. > This also makes KASAN tests corrupt kernel memory and cause crashes. > > To fix this, use __asan_/__hwasan_memset/memmove/memcpy as the underlying > functions whenever appropriate. Do this only for the instrumented code > (as indicated by __SANITIZE_ADDRESS__). > > Reported-by: Erhard Furtner <erhard_f@mailbox.org> > Reported-by: Nico Pache <npache@redhat.com> > Closes: https://lore.kernel.org/all/20240501144156.17e65021@outsider.home/ > Fixes: 69d4c0d32186 ("entry, kasan, x86: Disallow overriding mem*() functions") > Fixes: 51287dcb00cc ("kasan: emit different calls for instrumentable memintrinsics") > Fixes: 36be5cba99f6 ("kasan: treat meminstrinsic as builtins in uninstrumented files") > Signed-off-by: Andrey Konovalov <andreyknvl@gmail.com> Thank you for fixing this !! The test no longer panics :) Now that the test progresses I also see rcu_uaf failing. # rcu_uaf: EXPECTATION FAILED at mm/kasan/kasan_test.c:870 KASAN failure expected in "call_rcu(&global_rcu_ptr->rcu, rcu_uaf_reclaim); rcu_barrier()", but none occurred not ok 31 rcu_uaf I can open a new thread for that if you'd like. Tested-by: Nico Pache <npache@redhat.com> Acked-by: Nico Pache <npache@redhat.com> > --- > include/linux/fortify-string.h | 22 ++++++++++++++++++---- > 1 file changed, 18 insertions(+), 4 deletions(-) > > diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h > index 85fc0e6f0f7f..bac010cfc42f 100644 > --- a/include/linux/fortify-string.h > +++ b/include/linux/fortify-string.h > @@ -75,17 +75,30 @@ void __write_overflow_field(size_t avail, size_t wanted) __compiletime_warning(" > __ret; \ > }) > > -#if defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS) > +#if defined(__SANITIZE_ADDRESS__) > + > +#if !defined(CONFIG_CC_HAS_KASAN_MEMINTRINSIC_PREFIX) && !defined(CONFIG_GENERIC_ENTRY) > +extern void *__underlying_memset(void *p, int c, __kernel_size_t size) __RENAME(memset); > +extern void *__underlying_memmove(void *p, const void *q, __kernel_size_t size) __RENAME(memmove); > +extern void *__underlying_memcpy(void *p, const void *q, __kernel_size_t size) __RENAME(memcpy); > +#elif defined(CONFIG_KASAN_GENERIC) > +extern void *__underlying_memset(void *p, int c, __kernel_size_t size) __RENAME(__asan_memset); > +extern void *__underlying_memmove(void *p, const void *q, __kernel_size_t size) __RENAME(__asan_memmove); > +extern void *__underlying_memcpy(void *p, const void *q, __kernel_size_t size) __RENAME(__asan_memcpy); > +#else /* CONFIG_KASAN_SW_TAGS */ > +extern void *__underlying_memset(void *p, int c, __kernel_size_t size) __RENAME(__hwasan_memset); > +extern void *__underlying_memmove(void *p, const void *q, __kernel_size_t size) __RENAME(__hwasan_memmove); > +extern void *__underlying_memcpy(void *p, const void *q, __kernel_size_t size) __RENAME(__hwasan_memcpy); > +#endif > + > extern void *__underlying_memchr(const void *p, int c, __kernel_size_t size) __RENAME(memchr); > extern int __underlying_memcmp(const void *p, const void *q, __kernel_size_t size) __RENAME(memcmp); > -extern void *__underlying_memcpy(void *p, const void *q, __kernel_size_t size) __RENAME(memcpy); > -extern void *__underlying_memmove(void *p, const void *q, __kernel_size_t size) __RENAME(memmove); > -extern void *__underlying_memset(void *p, int c, __kernel_size_t size) __RENAME(memset); > extern char *__underlying_strcat(char *p, const char *q) __RENAME(strcat); > extern char *__underlying_strcpy(char *p, const char *q) __RENAME(strcpy); > extern __kernel_size_t __underlying_strlen(const char *p) __RENAME(strlen); > extern char *__underlying_strncat(char *p, const char *q, __kernel_size_t count) __RENAME(strncat); > extern char *__underlying_strncpy(char *p, const char *q, __kernel_size_t size) __RENAME(strncpy); > + > #else > > #if defined(__SANITIZE_MEMORY__) > @@ -110,6 +123,7 @@ extern char *__underlying_strncpy(char *p, const char *q, __kernel_size_t size) > #define __underlying_strlen __builtin_strlen > #define __underlying_strncat __builtin_strncat > #define __underlying_strncpy __builtin_strncpy > + > #endif > > /** > -- > 2.25.1 >
On Fri, May 17, 2024 at 9:50 PM Nico Pache <npache@redhat.com> wrote: > > Thank you for fixing this !! The test no longer panics :) Awesome, thank you for testing! > Now that the test progresses I also see rcu_uaf failing. > # rcu_uaf: EXPECTATION FAILED at mm/kasan/kasan_test.c:870 > KASAN failure expected in "call_rcu(&global_rcu_ptr->rcu, > rcu_uaf_reclaim); rcu_barrier()", but none occurred > not ok 31 rcu_uaf > I can open a new thread for that if you'd like. Looks like something else is broken :( Unfortunately, I failed to boot a kernel built with the config that you provided in QEMU. If you can provide a config that boots in QEMU or instructions on how to reproduce the issue, I can take a look. Thanks!
diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h index 85fc0e6f0f7f..bac010cfc42f 100644 --- a/include/linux/fortify-string.h +++ b/include/linux/fortify-string.h @@ -75,17 +75,30 @@ void __write_overflow_field(size_t avail, size_t wanted) __compiletime_warning(" __ret; \ }) -#if defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS) +#if defined(__SANITIZE_ADDRESS__) + +#if !defined(CONFIG_CC_HAS_KASAN_MEMINTRINSIC_PREFIX) && !defined(CONFIG_GENERIC_ENTRY) +extern void *__underlying_memset(void *p, int c, __kernel_size_t size) __RENAME(memset); +extern void *__underlying_memmove(void *p, const void *q, __kernel_size_t size) __RENAME(memmove); +extern void *__underlying_memcpy(void *p, const void *q, __kernel_size_t size) __RENAME(memcpy); +#elif defined(CONFIG_KASAN_GENERIC) +extern void *__underlying_memset(void *p, int c, __kernel_size_t size) __RENAME(__asan_memset); +extern void *__underlying_memmove(void *p, const void *q, __kernel_size_t size) __RENAME(__asan_memmove); +extern void *__underlying_memcpy(void *p, const void *q, __kernel_size_t size) __RENAME(__asan_memcpy); +#else /* CONFIG_KASAN_SW_TAGS */ +extern void *__underlying_memset(void *p, int c, __kernel_size_t size) __RENAME(__hwasan_memset); +extern void *__underlying_memmove(void *p, const void *q, __kernel_size_t size) __RENAME(__hwasan_memmove); +extern void *__underlying_memcpy(void *p, const void *q, __kernel_size_t size) __RENAME(__hwasan_memcpy); +#endif + extern void *__underlying_memchr(const void *p, int c, __kernel_size_t size) __RENAME(memchr); extern int __underlying_memcmp(const void *p, const void *q, __kernel_size_t size) __RENAME(memcmp); -extern void *__underlying_memcpy(void *p, const void *q, __kernel_size_t size) __RENAME(memcpy); -extern void *__underlying_memmove(void *p, const void *q, __kernel_size_t size) __RENAME(memmove); -extern void *__underlying_memset(void *p, int c, __kernel_size_t size) __RENAME(memset); extern char *__underlying_strcat(char *p, const char *q) __RENAME(strcat); extern char *__underlying_strcpy(char *p, const char *q) __RENAME(strcpy); extern __kernel_size_t __underlying_strlen(const char *p) __RENAME(strlen); extern char *__underlying_strncat(char *p, const char *q, __kernel_size_t count) __RENAME(strncat); extern char *__underlying_strncpy(char *p, const char *q, __kernel_size_t size) __RENAME(strncpy); + #else #if defined(__SANITIZE_MEMORY__) @@ -110,6 +123,7 @@ extern char *__underlying_strncpy(char *p, const char *q, __kernel_size_t size) #define __underlying_strlen __builtin_strlen #define __underlying_strncat __builtin_strncat #define __underlying_strncpy __builtin_strncpy + #endif /**