Certain modules call copy_user_highpage(), which calls
kmsan_copy_page_meta() under KMSAN, so we need to export the latter.
Cc: Andrew Morton <[email protected]>
Fixes: b073d7f8aee4 ("mm: kmsan: maintain KMSAN metadata for page operations")
Link: https://github.com/google/kmsan/issues/89
Signed-off-by: Alexander Potapenko <[email protected]>
---
mm/kmsan/shadow.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/mm/kmsan/shadow.c b/mm/kmsan/shadow.c
index 21e3e196ec3cf..a787c04e9583c 100644
--- a/mm/kmsan/shadow.c
+++ b/mm/kmsan/shadow.c
@@ -167,6 +167,7 @@ void kmsan_copy_page_meta(struct page *dst, struct page *src)
__memcpy(origin_ptr_for(dst), origin_ptr_for(src), PAGE_SIZE);
kmsan_leave_runtime();
}
+EXPORT_SYMBOL(kmsan_copy_page_meta);
void kmsan_alloc_page(struct page *page, unsigned int order, gfp_t flags)
{
--
2.38.0.135.g90850a2211-goog
The stand-alone purgatory.ro does not contain the KMSAN runtime,
therefore it can't be built with KMSAN compiler instrumentation.
Cc: Andrew Morton <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: [email protected]
Link: https://github.com/google/kmsan/issues/89
Signed-off-by: Alexander Potapenko <[email protected]>
---
arch/x86/purgatory/Makefile | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/x86/purgatory/Makefile b/arch/x86/purgatory/Makefile
index 58a200dc762d6..17f09dc263811 100644
--- a/arch/x86/purgatory/Makefile
+++ b/arch/x86/purgatory/Makefile
@@ -26,6 +26,7 @@ GCOV_PROFILE := n
KASAN_SANITIZE := n
UBSAN_SANITIZE := n
KCSAN_SANITIZE := n
+KMSAN_SANITIZE := n
KCOV_INSTRUMENT := n
# These are adjustments to the compiler flags used for objects that
--
2.38.0.135.g90850a2211-goog
KMSAN adds a lot of instrumentation to the code, which results in
increased stack usage (up to 2048 bytes and more in some cases).
It's hard to predict how big the stack frames can be, so we disable
the warnings for KMSAN instead.
Cc: Andrew Morton <[email protected]>
Cc: Kees Cook <[email protected]>
Cc: Masahiro Yamada <[email protected]>
Cc: Nick Desaulniers <[email protected]>
Cc: [email protected]
Link: https://github.com/google/kmsan/issues/89
Signed-off-by: Alexander Potapenko <[email protected]>
---
lib/Kconfig.debug | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 3fc7abffc7aa2..29280072dc0e4 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -400,8 +400,9 @@ config FRAME_WARN
default 1536 if (!64BIT && XTENSA)
default 1024 if !64BIT
default 2048 if 64BIT
+ default 0 if KMSAN
help
- Tell gcc to warn at build time for stack frames larger than this.
+ Tell the compiler to warn at build time for stack frames larger than this.
Setting this too low will cause a lot of warnings.
Setting it to 0 disables the warning.
--
2.38.0.135.g90850a2211-goog
Ensure that KMSAN builds replace memset/memcpy/memmove calls with the
respective __msan_XXX functions, and that none of the macros are
redefined twice. This should allow building kernel with both
CONFIG_KMSAN and CONFIG_FORTIFY_SOURCE.
Cc: Andrew Morton <[email protected]>
Cc: Nathan Chancellor <[email protected]>
Cc: Nick Desaulniers <[email protected]>
Cc: Kees Cook <[email protected]>
Cc: [email protected]
Reported-by: Tamas K Lengyel <[email protected]>
Link: https://github.com/google/kmsan/issues/89
Signed-off-by: Alexander Potapenko <[email protected]>
---
arch/x86/include/asm/string_64.h | 11 +++++++----
include/linux/fortify-string.h | 17 +++++++++++++++--
include/linux/kmsan_string.h | 21 +++++++++++++++++++++
mm/kmsan/instrumentation.c | 1 +
4 files changed, 44 insertions(+), 6 deletions(-)
create mode 100644 include/linux/kmsan_string.h
diff --git a/arch/x86/include/asm/string_64.h b/arch/x86/include/asm/string_64.h
index 3b87d889b6e16..888731ccf1f67 100644
--- a/arch/x86/include/asm/string_64.h
+++ b/arch/x86/include/asm/string_64.h
@@ -10,10 +10,13 @@
/* Even with __builtin_ the compiler may decide to use the out of line
function. */
+#if defined(__SANITIZE_MEMORY__) && defined(__NO_FORTIFY)
+#include <linux/kmsan_string.h>
+#endif
+
#define __HAVE_ARCH_MEMCPY 1
-#if defined(__SANITIZE_MEMORY__)
+#if defined(__SANITIZE_MEMORY__) && defined(__NO_FORTIFY)
#undef memcpy
-void *__msan_memcpy(void *dst, const void *src, size_t size);
#define memcpy __msan_memcpy
#else
extern void *memcpy(void *to, const void *from, size_t len);
@@ -21,7 +24,7 @@ extern void *memcpy(void *to, const void *from, size_t len);
extern void *__memcpy(void *to, const void *from, size_t len);
#define __HAVE_ARCH_MEMSET
-#if defined(__SANITIZE_MEMORY__)
+#if defined(__SANITIZE_MEMORY__) && defined(__NO_FORTIFY)
extern void *__msan_memset(void *s, int c, size_t n);
#undef memset
#define memset __msan_memset
@@ -67,7 +70,7 @@ static inline void *memset64(uint64_t *s, uint64_t v, size_t n)
}
#define __HAVE_ARCH_MEMMOVE
-#if defined(__SANITIZE_MEMORY__)
+#if defined(__SANITIZE_MEMORY__) && defined(__NO_FORTIFY)
#undef memmove
void *__msan_memmove(void *dest, const void *src, size_t len);
#define memmove __msan_memmove
diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h
index 4029fe368a4f6..18a31b125f9d6 100644
--- a/include/linux/fortify-string.h
+++ b/include/linux/fortify-string.h
@@ -43,11 +43,24 @@ 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
-#define __underlying_memchr __builtin_memchr
-#define __underlying_memcmp __builtin_memcmp
+
+#if defined(__SANITIZE_MEMORY__)
+/*
+ * For KMSAN builds all memcpy/memset/memmove calls should be replaced by the
+ * corresponding __msan_XXX functions.
+ */
+#include <linux/kmsan_string.h>
+#define __underlying_memcpy __msan_memcpy
+#define __underlying_memmove __msan_memmove
+#define __underlying_memset __msan_memset
+#else
#define __underlying_memcpy __builtin_memcpy
#define __underlying_memmove __builtin_memmove
#define __underlying_memset __builtin_memset
+#endif
+
+#define __underlying_memchr __builtin_memchr
+#define __underlying_memcmp __builtin_memcmp
#define __underlying_strcat __builtin_strcat
#define __underlying_strcpy __builtin_strcpy
#define __underlying_strlen __builtin_strlen
diff --git a/include/linux/kmsan_string.h b/include/linux/kmsan_string.h
new file mode 100644
index 0000000000000..7287da6f52eff
--- /dev/null
+++ b/include/linux/kmsan_string.h
@@ -0,0 +1,21 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * KMSAN string functions API used in other headers.
+ *
+ * Copyright (C) 2022 Google LLC
+ * Author: Alexander Potapenko <[email protected]>
+ *
+ */
+#ifndef _LINUX_KMSAN_STRING_H
+#define _LINUX_KMSAN_STRING_H
+
+/*
+ * KMSAN overrides the default memcpy/memset/memmove implementations in the
+ * kernel, which requires having __msan_XXX function prototypes in several other
+ * headers. Keep them in one place instead of open-coding.
+ */
+void *__msan_memcpy(void *dst, const void *src, size_t size);
+void *__msan_memset(void *s, int c, size_t n);
+void *__msan_memmove(void *dest, const void *src, size_t len);
+
+#endif /* _LINUX_KMSAN_STRING_H */
diff --git a/mm/kmsan/instrumentation.c b/mm/kmsan/instrumentation.c
index 280d154132684..271f135f97a16 100644
--- a/mm/kmsan/instrumentation.c
+++ b/mm/kmsan/instrumentation.c
@@ -14,6 +14,7 @@
#include "kmsan.h"
#include <linux/gfp.h>
+#include <linux/kmsan_string.h>
#include <linux/mm.h>
#include <linux/uaccess.h>
--
2.38.0.135.g90850a2211-goog
User access macros must ensure their arguments are evaluated only once
if they are used more than once in the macro body. Adding
instrument_put_user() to __put_user_size() resulted in double evaluation
of the `ptr` argument, which led to correctness issues when performing
e.g. unsafe_put_user(..., p++, ...).
To fix those issues, evaluate the `ptr` argument of __put_user_size()
at the beginning of the macro.
Cc: Andrew Morton <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: [email protected]
Reported-by: youling257 <[email protected]>
Signed-off-by: Alexander Potapenko <[email protected]>
Fixes: 888f84a6da4d ("x86: asm: instrument usercopy in get_user() and put_user()")
---
arch/x86/include/asm/uaccess.h | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
index 8bc614cfe21b9..1cc756eafa447 100644
--- a/arch/x86/include/asm/uaccess.h
+++ b/arch/x86/include/asm/uaccess.h
@@ -254,24 +254,25 @@ extern void __put_user_nocheck_8(void);
#define __put_user_size(x, ptr, size, label) \
do { \
__typeof__(*(ptr)) __x = (x); /* eval x once */ \
- __chk_user_ptr(ptr); \
+ __typeof__(ptr) __ptr = (ptr); /* eval ptr once */ \
+ __chk_user_ptr(__ptr); \
switch (size) { \
case 1: \
- __put_user_goto(__x, ptr, "b", "iq", label); \
+ __put_user_goto(__x, __ptr, "b", "iq", label); \
break; \
case 2: \
- __put_user_goto(__x, ptr, "w", "ir", label); \
+ __put_user_goto(__x, __ptr, "w", "ir", label); \
break; \
case 4: \
- __put_user_goto(__x, ptr, "l", "ir", label); \
+ __put_user_goto(__x, __ptr, "l", "ir", label); \
break; \
case 8: \
- __put_user_goto_u64(__x, ptr, label); \
+ __put_user_goto_u64(__x, __ptr, label); \
break; \
default: \
__put_user_bad(); \
} \
- instrument_put_user(__x, ptr, size); \
+ instrument_put_user(__x, __ptr, size); \
} while (0)
#ifdef CONFIG_CC_HAS_ASM_GOTO_OUTPUT
--
2.38.0.135.g90850a2211-goog
On Tue, Oct 25, 2022 at 6:22 AM Alexander Potapenko <[email protected]> wrote:
>
> KMSAN adds a lot of instrumentation to the code, which results in
> increased stack usage (up to 2048 bytes and more in some cases).
> It's hard to predict how big the stack frames can be, so we disable
> the warnings for KMSAN instead.
>
> Cc: Andrew Morton <[email protected]>
> Cc: Kees Cook <[email protected]>
> Cc: Masahiro Yamada <[email protected]>
> Cc: Nick Desaulniers <[email protected]>
> Cc: [email protected]
> Link: https://github.com/google/kmsan/issues/89
> Signed-off-by: Alexander Potapenko <[email protected]>
> ---
> lib/Kconfig.debug | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 3fc7abffc7aa2..29280072dc0e4 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -400,8 +400,9 @@ config FRAME_WARN
> default 1536 if (!64BIT && XTENSA)
> default 1024 if !64BIT
> default 2048 if 64BIT
> + default 0 if KMSAN
This is wrong.
Kconfig picks up the first default entry which has
true 'if' condition.
Since (!64BIT || 64BIT) covers all the possible cases,
this patch is just adding dead code.
> help
> - Tell gcc to warn at build time for stack frames larger than this.
> + Tell the compiler to warn at build time for stack frames larger than this.
> Setting this too low will cause a lot of warnings.
> Setting it to 0 disables the warning.
>
> --
> 2.38.0.135.g90850a2211-goog
>
--
Best Regards
Masahiro Yamada
On Thu, Oct 27, 2022 at 7:33 AM Masahiro Yamada <[email protected]> wrote:
>
> On Tue, Oct 25, 2022 at 6:22 AM Alexander Potapenko <[email protected]> wrote:
> >
> > KMSAN adds a lot of instrumentation to the code, which results in
> > increased stack usage (up to 2048 bytes and more in some cases).
> > It's hard to predict how big the stack frames can be, so we disable
> > the warnings for KMSAN instead.
> >
> > Cc: Andrew Morton <[email protected]>
> > Cc: Kees Cook <[email protected]>
> > Cc: Masahiro Yamada <[email protected]>
> > Cc: Nick Desaulniers <[email protected]>
> > Cc: [email protected]
> > Link: https://github.com/google/kmsan/issues/89
> > Signed-off-by: Alexander Potapenko <[email protected]>
> > ---
> > lib/Kconfig.debug | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> > index 3fc7abffc7aa2..29280072dc0e4 100644
> > --- a/lib/Kconfig.debug
> > +++ b/lib/Kconfig.debug
> > @@ -400,8 +400,9 @@ config FRAME_WARN
> > default 1536 if (!64BIT && XTENSA)
> > default 1024 if !64BIT
> > default 2048 if 64BIT
> > + default 0 if KMSAN
>
>
>
> This is wrong.
>
> Kconfig picks up the first default entry which has
> true 'if' condition.
>
>
> Since (!64BIT || 64BIT) covers all the possible cases,
> this patch is just adding dead code.
>
Thanks for clarifying!
What we actually need is to forcefully disable CONFIG_FRAME_WARN under
KMSAN, but adding constructs like "select FRAME_WARN 0" or "select
FRAME_WARN=0" doesn't work, right?
I'll move "default 0 if KMSAN" to the beginning of FRAME_WARN declaration then.
>
>
>
>
>
> > help
> > - Tell gcc to warn at build time for stack frames larger than this.
> > + Tell the compiler to warn at build time for stack frames larger than this.
> > Setting this too low will cause a lot of warnings.
> > Setting it to 0 disables the warning.
> >
> > --
> > 2.38.0.135.g90850a2211-goog
> >
>
>
> --
> Best Regards
> Masahiro Yamada
--
Alexander Potapenko
Software Engineer
Google Germany GmbH
Erika-Mann-Straße, 33
80636 München
Geschäftsführer: Paul Manicle, Liana Sebastian
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg