clang -fsanitize=kernel-memory already replaces calls 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.
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 <[email protected]>
Suggested-by: Marco Elver <[email protected]>
Signed-off-by: Alexander Potapenko <[email protected]>
Reviewed-by: Marco Elver <[email protected]>
---
v2:
- updated patch description
---
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.40.0.rc0.216.gc4246ad0f0-goog
commit 5478afc55a21 ("kmsan: fix memcpy tests") uses OPTIMIZER_HIDE_VAR()
to hide the uninitialized var from the compiler optimizations.
However OPTIMIZER_HIDE_VAR(uninit) enforces an immediate check of
@uninit, so memcpy tests did not actually check the behavior of memcpy(),
because they always contained a KMSAN report.
Replace OPTIMIZER_HIDE_VAR() with a file-local macro that just clobbers
the memory with a barrier(), and add a test case for memcpy() that does not
expect an error report.
Also reflow kmsan_test.c with clang-format.
Signed-off-by: Alexander Potapenko <[email protected]>
---
v2:
- replace inline assembly with a barrier(), update comments
---
mm/kmsan/kmsan_test.c | 44 +++++++++++++++++++++++++++++++++++++------
1 file changed, 38 insertions(+), 6 deletions(-)
diff --git a/mm/kmsan/kmsan_test.c b/mm/kmsan/kmsan_test.c
index 088e21a48dc4b..aeddfdd4f679f 100644
--- a/mm/kmsan/kmsan_test.c
+++ b/mm/kmsan/kmsan_test.c
@@ -407,6 +407,37 @@ static void test_printk(struct kunit *test)
KUNIT_EXPECT_TRUE(test, report_matches(&expect));
}
+/*
+ * Prevent the compiler from optimizing @var away. Without this, Clang may
+ * notice that @var is uninitialized and drop memcpy() calls that use it.
+ *
+ * There is OPTIMIZER_HIDE_VAR() in linux/compier.h that we cannot use here,
+ * because it is implemented as inline assembly receiving @var as a parameter
+ * and will enforce a KMSAN check. Same is true for e.g. barrier_data(var).
+ */
+#define DO_NOT_OPTIMIZE(var) barrier()
+
+/*
+ * Test case: ensure that memcpy() correctly copies initialized values.
+ * Also serves as a regression test to ensure DO_NOT_OPTIMIZE() does not cause
+ * extra checks.
+ */
+static void test_init_memcpy(struct kunit *test)
+{
+ EXPECTATION_NO_REPORT(expect);
+ volatile int src;
+ volatile int dst = 0;
+
+ DO_NOT_OPTIMIZE(src);
+ src = 1;
+ kunit_info(
+ test,
+ "memcpy()ing aligned initialized src to aligned dst (no reports)\n");
+ memcpy((void *)&dst, (void *)&src, sizeof(src));
+ kmsan_check_memory((void *)&dst, sizeof(dst));
+ KUNIT_EXPECT_TRUE(test, report_matches(&expect));
+}
+
/*
* Test case: ensure that memcpy() correctly copies uninitialized values between
* aligned `src` and `dst`.
@@ -420,7 +451,7 @@ static void test_memcpy_aligned_to_aligned(struct kunit *test)
kunit_info(
test,
"memcpy()ing aligned uninit src to aligned dst (UMR report)\n");
- OPTIMIZER_HIDE_VAR(uninit_src);
+ DO_NOT_OPTIMIZE(uninit_src);
memcpy((void *)&dst, (void *)&uninit_src, sizeof(uninit_src));
kmsan_check_memory((void *)&dst, sizeof(dst));
KUNIT_EXPECT_TRUE(test, report_matches(&expect));
@@ -443,7 +474,7 @@ static void test_memcpy_aligned_to_unaligned(struct kunit *test)
kunit_info(
test,
"memcpy()ing aligned uninit src to unaligned dst (UMR report)\n");
- OPTIMIZER_HIDE_VAR(uninit_src);
+ DO_NOT_OPTIMIZE(uninit_src);
memcpy((void *)&dst[1], (void *)&uninit_src, sizeof(uninit_src));
kmsan_check_memory((void *)dst, 4);
KUNIT_EXPECT_TRUE(test, report_matches(&expect));
@@ -467,13 +498,14 @@ static void test_memcpy_aligned_to_unaligned2(struct kunit *test)
kunit_info(
test,
"memcpy()ing aligned uninit src to unaligned dst - part 2 (UMR report)\n");
- OPTIMIZER_HIDE_VAR(uninit_src);
+ DO_NOT_OPTIMIZE(uninit_src);
memcpy((void *)&dst[1], (void *)&uninit_src, sizeof(uninit_src));
kmsan_check_memory((void *)&dst[4], sizeof(uninit_src));
KUNIT_EXPECT_TRUE(test, report_matches(&expect));
}
-static noinline void fibonacci(int *array, int size, int start) {
+static noinline void fibonacci(int *array, int size, int start)
+{
if (start < 2 || (start == size))
return;
array[start] = array[start - 1] + array[start - 2];
@@ -482,8 +514,7 @@ static noinline void fibonacci(int *array, int size, int start) {
static void test_long_origin_chain(struct kunit *test)
{
- EXPECTATION_UNINIT_VALUE_FN(expect,
- "test_long_origin_chain");
+ EXPECTATION_UNINIT_VALUE_FN(expect, "test_long_origin_chain");
/* (KMSAN_MAX_ORIGIN_DEPTH * 2) recursive calls to fibonacci(). */
volatile int accum[KMSAN_MAX_ORIGIN_DEPTH * 2 + 2];
int last = ARRAY_SIZE(accum) - 1;
@@ -515,6 +546,7 @@ static struct kunit_case kmsan_test_cases[] = {
KUNIT_CASE(test_uaf),
KUNIT_CASE(test_percpu_propagate),
KUNIT_CASE(test_printk),
+ KUNIT_CASE(test_init_memcpy),
KUNIT_CASE(test_memcpy_aligned_to_aligned),
KUNIT_CASE(test_memcpy_aligned_to_unaligned),
KUNIT_CASE(test_memcpy_aligned_to_unaligned2),
--
2.40.0.rc0.216.gc4246ad0f0-goog
KMSAN must see as many memory accesses as possible to prevent false
positive reports. Fall back to versions of
memset16()/memset32()/memset64() implemented in lib/string.c instead of
those written in assembly.
Cc: Geert Uytterhoeven <[email protected]>
Cc: Daniel Vetter <[email protected]>
Cc: Helge Deller <[email protected]>
Suggested-by: Tetsuo Handa <[email protected]>
Signed-off-by: Alexander Potapenko <[email protected]>
Reviewed-by: Marco Elver <[email protected]>
---
arch/x86/include/asm/string_64.h | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/arch/x86/include/asm/string_64.h b/arch/x86/include/asm/string_64.h
index 9be401d971a99..e9c736f4686f5 100644
--- a/arch/x86/include/asm/string_64.h
+++ b/arch/x86/include/asm/string_64.h
@@ -22,6 +22,11 @@ extern void *__memcpy(void *to, const void *from, size_t len);
void *memset(void *s, int c, size_t n);
void *__memset(void *s, int c, size_t n);
+/*
+ * KMSAN needs to instrument as much code as possible. Use C versions of
+ * memsetXX() from lib/string.c under KMSAN.
+ */
+#if !defined(CONFIG_KMSAN)
#define __HAVE_ARCH_MEMSET16
static inline void *memset16(uint16_t *s, uint16_t v, size_t n)
{
@@ -57,6 +62,7 @@ static inline void *memset64(uint64_t *s, uint64_t v, size_t n)
: "memory");
return s;
}
+#endif
#define __HAVE_ARCH_MEMMOVE
void *memmove(void *dest, const void *src, size_t count);
--
2.40.0.rc0.216.gc4246ad0f0-goog
Add tests ensuring that memset16()/memset32()/memset64() are
instrumented by KMSAN and correctly initialize the memory.
Signed-off-by: Alexander Potapenko <[email protected]>
---
v2:
- drop a redundant parameter of DEFINE_TEST_MEMSETXX()
---
mm/kmsan/kmsan_test.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/mm/kmsan/kmsan_test.c b/mm/kmsan/kmsan_test.c
index aeddfdd4f679f..7095d3fbb23ac 100644
--- a/mm/kmsan/kmsan_test.c
+++ b/mm/kmsan/kmsan_test.c
@@ -504,6 +504,25 @@ static void test_memcpy_aligned_to_unaligned2(struct kunit *test)
KUNIT_EXPECT_TRUE(test, report_matches(&expect));
}
+/* Generate test cases for memset16(), memset32(), memset64(). */
+#define DEFINE_TEST_MEMSETXX(size) \
+ static void test_memset##size(struct kunit *test) \
+ { \
+ EXPECTATION_NO_REPORT(expect); \
+ volatile uint##size##_t uninit; \
+ \
+ kunit_info(test, \
+ "memset" #size "() should initialize memory\n"); \
+ DO_NOT_OPTIMIZE(uninit); \
+ memset##size((uint##size##_t *)&uninit, 0, 1); \
+ kmsan_check_memory((void *)&uninit, sizeof(uninit)); \
+ KUNIT_EXPECT_TRUE(test, report_matches(&expect)); \
+ }
+
+DEFINE_TEST_MEMSETXX(16)
+DEFINE_TEST_MEMSETXX(32)
+DEFINE_TEST_MEMSETXX(64)
+
static noinline void fibonacci(int *array, int size, int start)
{
if (start < 2 || (start == size))
@@ -550,6 +569,9 @@ static struct kunit_case kmsan_test_cases[] = {
KUNIT_CASE(test_memcpy_aligned_to_aligned),
KUNIT_CASE(test_memcpy_aligned_to_unaligned),
KUNIT_CASE(test_memcpy_aligned_to_unaligned2),
+ KUNIT_CASE(test_memset16),
+ KUNIT_CASE(test_memset32),
+ KUNIT_CASE(test_memset64),
KUNIT_CASE(test_long_origin_chain),
{},
};
--
2.40.0.rc0.216.gc4246ad0f0-goog
This is the second version of the patch. Sorry for the inconvenience.
This is the second version of the patch. Sorry for the inconvenience.
This is the second version of the patch. Sorry for the inconvenience.
This is the second version of the patch. Sorry for the inconvenience.
On Fri, 3 Mar 2023 at 15:14, Alexander Potapenko <[email protected]> wrote:
>
> commit 5478afc55a21 ("kmsan: fix memcpy tests") uses OPTIMIZER_HIDE_VAR()
> to hide the uninitialized var from the compiler optimizations.
>
> However OPTIMIZER_HIDE_VAR(uninit) enforces an immediate check of
> @uninit, so memcpy tests did not actually check the behavior of memcpy(),
> because they always contained a KMSAN report.
>
> Replace OPTIMIZER_HIDE_VAR() with a file-local macro that just clobbers
> the memory with a barrier(), and add a test case for memcpy() that does not
> expect an error report.
>
> Also reflow kmsan_test.c with clang-format.
>
> Signed-off-by: Alexander Potapenko <[email protected]>
> ---
> v2:
> - replace inline assembly with a barrier(), update comments
Reviewed-by: Marco Elver <[email protected]>
> ---
> mm/kmsan/kmsan_test.c | 44 +++++++++++++++++++++++++++++++++++++------
> 1 file changed, 38 insertions(+), 6 deletions(-)
>
> diff --git a/mm/kmsan/kmsan_test.c b/mm/kmsan/kmsan_test.c
> index 088e21a48dc4b..aeddfdd4f679f 100644
> --- a/mm/kmsan/kmsan_test.c
> +++ b/mm/kmsan/kmsan_test.c
> @@ -407,6 +407,37 @@ static void test_printk(struct kunit *test)
> KUNIT_EXPECT_TRUE(test, report_matches(&expect));
> }
>
> +/*
> + * Prevent the compiler from optimizing @var away. Without this, Clang may
> + * notice that @var is uninitialized and drop memcpy() calls that use it.
> + *
> + * There is OPTIMIZER_HIDE_VAR() in linux/compier.h that we cannot use here,
> + * because it is implemented as inline assembly receiving @var as a parameter
> + * and will enforce a KMSAN check. Same is true for e.g. barrier_data(var).
> + */
> +#define DO_NOT_OPTIMIZE(var) barrier()
> +
> +/*
> + * Test case: ensure that memcpy() correctly copies initialized values.
> + * Also serves as a regression test to ensure DO_NOT_OPTIMIZE() does not cause
> + * extra checks.
> + */
> +static void test_init_memcpy(struct kunit *test)
> +{
> + EXPECTATION_NO_REPORT(expect);
> + volatile int src;
> + volatile int dst = 0;
> +
> + DO_NOT_OPTIMIZE(src);
> + src = 1;
> + kunit_info(
> + test,
> + "memcpy()ing aligned initialized src to aligned dst (no reports)\n");
> + memcpy((void *)&dst, (void *)&src, sizeof(src));
> + kmsan_check_memory((void *)&dst, sizeof(dst));
> + KUNIT_EXPECT_TRUE(test, report_matches(&expect));
> +}
> +
> /*
> * Test case: ensure that memcpy() correctly copies uninitialized values between
> * aligned `src` and `dst`.
> @@ -420,7 +451,7 @@ static void test_memcpy_aligned_to_aligned(struct kunit *test)
> kunit_info(
> test,
> "memcpy()ing aligned uninit src to aligned dst (UMR report)\n");
> - OPTIMIZER_HIDE_VAR(uninit_src);
> + DO_NOT_OPTIMIZE(uninit_src);
> memcpy((void *)&dst, (void *)&uninit_src, sizeof(uninit_src));
> kmsan_check_memory((void *)&dst, sizeof(dst));
> KUNIT_EXPECT_TRUE(test, report_matches(&expect));
> @@ -443,7 +474,7 @@ static void test_memcpy_aligned_to_unaligned(struct kunit *test)
> kunit_info(
> test,
> "memcpy()ing aligned uninit src to unaligned dst (UMR report)\n");
> - OPTIMIZER_HIDE_VAR(uninit_src);
> + DO_NOT_OPTIMIZE(uninit_src);
> memcpy((void *)&dst[1], (void *)&uninit_src, sizeof(uninit_src));
> kmsan_check_memory((void *)dst, 4);
> KUNIT_EXPECT_TRUE(test, report_matches(&expect));
> @@ -467,13 +498,14 @@ static void test_memcpy_aligned_to_unaligned2(struct kunit *test)
> kunit_info(
> test,
> "memcpy()ing aligned uninit src to unaligned dst - part 2 (UMR report)\n");
> - OPTIMIZER_HIDE_VAR(uninit_src);
> + DO_NOT_OPTIMIZE(uninit_src);
> memcpy((void *)&dst[1], (void *)&uninit_src, sizeof(uninit_src));
> kmsan_check_memory((void *)&dst[4], sizeof(uninit_src));
> KUNIT_EXPECT_TRUE(test, report_matches(&expect));
> }
>
> -static noinline void fibonacci(int *array, int size, int start) {
> +static noinline void fibonacci(int *array, int size, int start)
> +{
> if (start < 2 || (start == size))
> return;
> array[start] = array[start - 1] + array[start - 2];
> @@ -482,8 +514,7 @@ static noinline void fibonacci(int *array, int size, int start) {
>
> static void test_long_origin_chain(struct kunit *test)
> {
> - EXPECTATION_UNINIT_VALUE_FN(expect,
> - "test_long_origin_chain");
> + EXPECTATION_UNINIT_VALUE_FN(expect, "test_long_origin_chain");
> /* (KMSAN_MAX_ORIGIN_DEPTH * 2) recursive calls to fibonacci(). */
> volatile int accum[KMSAN_MAX_ORIGIN_DEPTH * 2 + 2];
> int last = ARRAY_SIZE(accum) - 1;
> @@ -515,6 +546,7 @@ static struct kunit_case kmsan_test_cases[] = {
> KUNIT_CASE(test_uaf),
> KUNIT_CASE(test_percpu_propagate),
> KUNIT_CASE(test_printk),
> + KUNIT_CASE(test_init_memcpy),
> KUNIT_CASE(test_memcpy_aligned_to_aligned),
> KUNIT_CASE(test_memcpy_aligned_to_unaligned),
> KUNIT_CASE(test_memcpy_aligned_to_unaligned2),
> --
> 2.40.0.rc0.216.gc4246ad0f0-goog
>
On Fri, 3 Mar 2023 at 15:14, 'Alexander Potapenko' via kasan-dev
<[email protected]> wrote:
>
> Add tests ensuring that memset16()/memset32()/memset64() are
> instrumented by KMSAN and correctly initialize the memory.
>
> Signed-off-by: Alexander Potapenko <[email protected]>
> ---
> v2:
> - drop a redundant parameter of DEFINE_TEST_MEMSETXX()
Reviewed-by: Marco Elver <[email protected]>
> ---
> mm/kmsan/kmsan_test.c | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
>
> diff --git a/mm/kmsan/kmsan_test.c b/mm/kmsan/kmsan_test.c
> index aeddfdd4f679f..7095d3fbb23ac 100644
> --- a/mm/kmsan/kmsan_test.c
> +++ b/mm/kmsan/kmsan_test.c
> @@ -504,6 +504,25 @@ static void test_memcpy_aligned_to_unaligned2(struct kunit *test)
> KUNIT_EXPECT_TRUE(test, report_matches(&expect));
> }
>
> +/* Generate test cases for memset16(), memset32(), memset64(). */
> +#define DEFINE_TEST_MEMSETXX(size) \
> + static void test_memset##size(struct kunit *test) \
> + { \
> + EXPECTATION_NO_REPORT(expect); \
> + volatile uint##size##_t uninit; \
> + \
> + kunit_info(test, \
> + "memset" #size "() should initialize memory\n"); \
> + DO_NOT_OPTIMIZE(uninit); \
> + memset##size((uint##size##_t *)&uninit, 0, 1); \
> + kmsan_check_memory((void *)&uninit, sizeof(uninit)); \
> + KUNIT_EXPECT_TRUE(test, report_matches(&expect)); \
> + }
> +
> +DEFINE_TEST_MEMSETXX(16)
> +DEFINE_TEST_MEMSETXX(32)
> +DEFINE_TEST_MEMSETXX(64)
> +
> static noinline void fibonacci(int *array, int size, int start)
> {
> if (start < 2 || (start == size))
> @@ -550,6 +569,9 @@ static struct kunit_case kmsan_test_cases[] = {
> KUNIT_CASE(test_memcpy_aligned_to_aligned),
> KUNIT_CASE(test_memcpy_aligned_to_unaligned),
> KUNIT_CASE(test_memcpy_aligned_to_unaligned2),
> + KUNIT_CASE(test_memset16),
> + KUNIT_CASE(test_memset32),
> + KUNIT_CASE(test_memset64),
> KUNIT_CASE(test_long_origin_chain),
> {},
> };
> --
> 2.40.0.rc0.216.gc4246ad0f0-goog
>
> --
> You received this message because you are subscribed to the Google Groups "kasan-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
> To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/20230303141433.3422671-4-glider%40google.com.