| Message ID | 20230301143933.2374658-1-glider@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/4] x86: kmsan: Don't rename memintrinsics in uninstrumented files | expand |
On Wed, 1 Mar 2023 at 15:39, Alexander Potapenko <glider@google.com> wrote: > > KMSAN should be overriding calls to memset/memcpy/memmove and their You mean that the compiler will override calls? All supported compilers that have fsanitize=kernel-memory replace memintrinsics with __msan_mem*() calls, right? > __builtin_ versions in instrumented files, so there is no need to > override them. In non-instrumented versions we are now required to > leave memset() and friends intact, so we cannot replace them with > __msan_XXX() functions. > > Cc: Kees Cook <keescook@chromium.org> > Suggested-by: Marco Elver <elver@google.com> > Signed-off-by: Alexander Potapenko <glider@google.com> Other than that, Reviewed-by: Marco Elver <elver@google.com> > --- > arch/x86/include/asm/string_64.h | 17 ----------------- > 1 file changed, 17 deletions(-) > > diff --git a/arch/x86/include/asm/string_64.h b/arch/x86/include/asm/string_64.h > index 888731ccf1f67..9be401d971a99 100644 > --- a/arch/x86/include/asm/string_64.h > +++ b/arch/x86/include/asm/string_64.h > @@ -15,22 +15,11 @@ > #endif > > #define __HAVE_ARCH_MEMCPY 1 > -#if defined(__SANITIZE_MEMORY__) && defined(__NO_FORTIFY) > -#undef memcpy > -#define memcpy __msan_memcpy > -#else > extern void *memcpy(void *to, const void *from, size_t len); > -#endif > extern void *__memcpy(void *to, const void *from, size_t len); > > #define __HAVE_ARCH_MEMSET > -#if defined(__SANITIZE_MEMORY__) && defined(__NO_FORTIFY) > -extern void *__msan_memset(void *s, int c, size_t n); > -#undef memset > -#define memset __msan_memset > -#else > void *memset(void *s, int c, size_t n); > -#endif > void *__memset(void *s, int c, size_t n); > > #define __HAVE_ARCH_MEMSET16 > @@ -70,13 +59,7 @@ static inline void *memset64(uint64_t *s, uint64_t v, size_t n) > } > > #define __HAVE_ARCH_MEMMOVE > -#if defined(__SANITIZE_MEMORY__) && defined(__NO_FORTIFY) > -#undef memmove > -void *__msan_memmove(void *dest, const void *src, size_t len); > -#define memmove __msan_memmove > -#else > void *memmove(void *dest, const void *src, size_t count); > -#endif > void *__memmove(void *dest, const void *src, size_t count); > > int memcmp(const void *cs, const void *ct, size_t count); > -- > 2.39.2.722.g9855ee24e9-goog >
On Thu, Mar 2, 2023 at 12:14 PM Marco Elver <elver@google.com> wrote: > > On Wed, 1 Mar 2023 at 15:39, Alexander Potapenko <glider@google.com> wrote: > > > > KMSAN should be overriding calls to memset/memcpy/memmove and their > > You mean that the compiler will override calls? > All supported compilers that have fsanitize=kernel-memory replace > memintrinsics with __msan_mem*() calls, right? Right. Changed to: KMSAN already replaces calls to to memset/memcpy/memmove and their __builtin_ versions with __msan_memset/__msan_memcpy/__msan_memmove in instrumented files, so there is no need to override them. > > > __builtin_ versions in instrumented files, so there is no need to > > override them. In non-instrumented versions we are now required to > > leave memset() and friends intact, so we cannot replace them with > > __msan_XXX() functions. > > > > Cc: Kees Cook <keescook@chromium.org> > > Suggested-by: Marco Elver <elver@google.com> > > Signed-off-by: Alexander Potapenko <glider@google.com> > > Other than that, > > Reviewed-by: Marco Elver <elver@google.com> Thanks!
On Thu, 2 Mar 2023 at 15:28, Alexander Potapenko <glider@google.com> wrote: > > On Thu, Mar 2, 2023 at 12:14 PM Marco Elver <elver@google.com> wrote: > > > > On Wed, 1 Mar 2023 at 15:39, Alexander Potapenko <glider@google.com> wrote: > > > > > > KMSAN should be overriding calls to memset/memcpy/memmove and their > > > > You mean that the compiler will override calls? > > All supported compilers that have fsanitize=kernel-memory replace > > memintrinsics with __msan_mem*() calls, right? > > Right. Changed to: > > KMSAN already replaces calls to to memset/memcpy/memmove and their > __builtin_ versions with __msan_memset/__msan_memcpy/__msan_memmove in > instrumented files, so there is no need to override them. But it's not KMSAN - KMSAN is the combined end result of runtime and compiler - in this case we need to be specific and point out it's the compiler that's doing it. There is no code in the Linux kernel that does this replacement. > > > > > > __builtin_ versions in instrumented files, so there is no need to > > > override them. In non-instrumented versions we are now required to > > > leave memset() and friends intact, so we cannot replace them with > > > __msan_XXX() functions. > > > > > > Cc: Kees Cook <keescook@chromium.org> > > > Suggested-by: Marco Elver <elver@google.com> > > > Signed-off-by: Alexander Potapenko <glider@google.com> > > > > Other than that, > > > > Reviewed-by: Marco Elver <elver@google.com> > Thanks!
On Thu, Mar 2, 2023 at 4:13 PM Marco Elver <elver@google.com> wrote: > > On Thu, 2 Mar 2023 at 15:28, Alexander Potapenko <glider@google.com> wrote: > > > > On Thu, Mar 2, 2023 at 12:14 PM Marco Elver <elver@google.com> wrote: > > > > > > On Wed, 1 Mar 2023 at 15:39, Alexander Potapenko <glider@google.com> wrote: > > > > > > > > KMSAN should be overriding calls to memset/memcpy/memmove and their > > > > > > You mean that the compiler will override calls? > > > All supported compilers that have fsanitize=kernel-memory replace > > > memintrinsics with __msan_mem*() calls, right? > > > > Right. Changed to: > > > > KMSAN already replaces calls to to memset/memcpy/memmove and their > > __builtin_ versions with __msan_memset/__msan_memcpy/__msan_memmove in > > instrumented files, so there is no need to override them. > > But it's not KMSAN - KMSAN is the combined end result of runtime and > compiler - in this case we need to be specific and point out it's the > compiler that's doing it. There is no code in the Linux kernel that > does this replacement. Agreed. I'll replace with "clang -fsanitize=kernel-memory"
diff --git a/arch/x86/include/asm/string_64.h b/arch/x86/include/asm/string_64.h index 888731ccf1f67..9be401d971a99 100644 --- a/arch/x86/include/asm/string_64.h +++ b/arch/x86/include/asm/string_64.h @@ -15,22 +15,11 @@ #endif #define __HAVE_ARCH_MEMCPY 1 -#if defined(__SANITIZE_MEMORY__) && defined(__NO_FORTIFY) -#undef memcpy -#define memcpy __msan_memcpy -#else extern void *memcpy(void *to, const void *from, size_t len); -#endif extern void *__memcpy(void *to, const void *from, size_t len); #define __HAVE_ARCH_MEMSET -#if defined(__SANITIZE_MEMORY__) && defined(__NO_FORTIFY) -extern void *__msan_memset(void *s, int c, size_t n); -#undef memset -#define memset __msan_memset -#else void *memset(void *s, int c, size_t n); -#endif void *__memset(void *s, int c, size_t n); #define __HAVE_ARCH_MEMSET16 @@ -70,13 +59,7 @@ static inline void *memset64(uint64_t *s, uint64_t v, size_t n) } #define __HAVE_ARCH_MEMMOVE -#if defined(__SANITIZE_MEMORY__) && defined(__NO_FORTIFY) -#undef memmove -void *__msan_memmove(void *dest, const void *src, size_t len); -#define memmove __msan_memmove -#else void *memmove(void *dest, const void *src, size_t count); -#endif void *__memmove(void *dest, const void *src, size_t count); int memcmp(const void *cs, const void *ct, size_t count);
KMSAN should be overriding calls to memset/memcpy/memmove and their __builtin_ versions in instrumented files, so there is no need to override them. In non-instrumented versions we are now required to leave memset() and friends intact, so we cannot replace them with __msan_XXX() functions. Cc: Kees Cook <keescook@chromium.org> Suggested-by: Marco Elver <elver@google.com> Signed-off-by: Alexander Potapenko <glider@google.com> --- arch/x86/include/asm/string_64.h | 17 ----------------- 1 file changed, 17 deletions(-)