From e36299efe7d749976fbdaaf756dee6ef32543c2c Mon Sep 17 00:00:00 2001 From: Marco Elver Date: Wed, 3 Mar 2021 10:38:45 +0100 Subject: [PATCH 1/4] kcsan, debugfs: Move debugfs file creation out of early init Commit 56348560d495 ("debugfs: do not attempt to create a new file before the filesystem is initalized") forbids creating new debugfs files until debugfs is fully initialized. This means that KCSAN's debugfs file creation, which happened at the end of __init(), no longer works. And was apparently never supposed to work! However, there is no reason to create KCSAN's debugfs file so early. This commit therefore moves its creation to a late_initcall() callback. Cc: "Rafael J. Wysocki" Cc: stable Fixes: 56348560d495 ("debugfs: do not attempt to create a new file before the filesystem is initalized") Reviewed-by: Greg Kroah-Hartman Signed-off-by: Marco Elver Signed-off-by: Paul E. McKenney --- kernel/kcsan/core.c | 2 -- kernel/kcsan/debugfs.c | 4 +++- kernel/kcsan/kcsan.h | 5 ----- 3 files changed, 3 insertions(+), 8 deletions(-) diff --git a/kernel/kcsan/core.c b/kernel/kcsan/core.c index 3bf98db9c702..23e7acb5c667 100644 --- a/kernel/kcsan/core.c +++ b/kernel/kcsan/core.c @@ -639,8 +639,6 @@ void __init kcsan_init(void) BUG_ON(!in_task()); - kcsan_debugfs_init(); - for_each_possible_cpu(cpu) per_cpu(kcsan_rand_state, cpu) = (u32)get_cycles(); diff --git a/kernel/kcsan/debugfs.c b/kernel/kcsan/debugfs.c index 3c8093a371b1..209ad8dcfcec 100644 --- a/kernel/kcsan/debugfs.c +++ b/kernel/kcsan/debugfs.c @@ -261,7 +261,9 @@ static const struct file_operations debugfs_ops = .release = single_release }; -void __init kcsan_debugfs_init(void) +static void __init kcsan_debugfs_init(void) { debugfs_create_file("kcsan", 0644, NULL, NULL, &debugfs_ops); } + +late_initcall(kcsan_debugfs_init); diff --git a/kernel/kcsan/kcsan.h b/kernel/kcsan/kcsan.h index 8d4bf3431b3c..87ccdb3b051f 100644 --- a/kernel/kcsan/kcsan.h +++ b/kernel/kcsan/kcsan.h @@ -30,11 +30,6 @@ extern bool kcsan_enabled; void kcsan_save_irqtrace(struct task_struct *task); void kcsan_restore_irqtrace(struct task_struct *task); -/* - * Initialize debugfs file. - */ -void kcsan_debugfs_init(void); - /* * Statistics counters displayed via debugfs; should only be modified in * slow-paths. From a146fed56f8a06a6f17ac11ebdc7ca3f396bcb55 Mon Sep 17 00:00:00 2001 From: Marco Elver Date: Wed, 13 Jan 2021 17:05:56 +0100 Subject: [PATCH 2/4] kcsan: Make test follow KUnit style recommendations Per recently added KUnit style recommendations at Documentation/dev-tools/kunit/style.rst, make the following changes to the KCSAN test: 1. Rename 'kcsan-test.c' to 'kcsan_test.c'. 2. Rename suite name 'kcsan-test' to 'kcsan'. 3. Rename CONFIG_KCSAN_TEST to CONFIG_KCSAN_KUNIT_TEST and default to KUNIT_ALL_TESTS. Reviewed-by: David Gow Signed-off-by: Marco Elver Signed-off-by: Paul E. McKenney --- kernel/kcsan/Makefile | 4 ++-- kernel/kcsan/{kcsan-test.c => kcsan_test.c} | 2 +- lib/Kconfig.kcsan | 5 +++-- 3 files changed, 6 insertions(+), 5 deletions(-) rename kernel/kcsan/{kcsan-test.c => kcsan_test.c} (99%) diff --git a/kernel/kcsan/Makefile b/kernel/kcsan/Makefile index 65ca5539c470..c2bb07f5bcc7 100644 --- a/kernel/kcsan/Makefile +++ b/kernel/kcsan/Makefile @@ -13,5 +13,5 @@ CFLAGS_core.o := $(call cc-option,-fno-conserve-stack) \ obj-y := core.o debugfs.o report.o obj-$(CONFIG_KCSAN_SELFTEST) += selftest.o -CFLAGS_kcsan-test.o := $(CFLAGS_KCSAN) -g -fno-omit-frame-pointer -obj-$(CONFIG_KCSAN_TEST) += kcsan-test.o +CFLAGS_kcsan_test.o := $(CFLAGS_KCSAN) -g -fno-omit-frame-pointer +obj-$(CONFIG_KCSAN_KUNIT_TEST) += kcsan_test.o diff --git a/kernel/kcsan/kcsan-test.c b/kernel/kcsan/kcsan_test.c similarity index 99% rename from kernel/kcsan/kcsan-test.c rename to kernel/kcsan/kcsan_test.c index ebe7fd245104..f16f632eb416 100644 --- a/kernel/kcsan/kcsan-test.c +++ b/kernel/kcsan/kcsan_test.c @@ -1156,7 +1156,7 @@ static void test_exit(struct kunit *test) } static struct kunit_suite kcsan_test_suite = { - .name = "kcsan-test", + .name = "kcsan", .test_cases = kcsan_test_cases, .init = test_init, .exit = test_exit, diff --git a/lib/Kconfig.kcsan b/lib/Kconfig.kcsan index f271ff5fbb5a..0440f373248e 100644 --- a/lib/Kconfig.kcsan +++ b/lib/Kconfig.kcsan @@ -69,8 +69,9 @@ config KCSAN_SELFTEST panic. Recommended to be enabled, ensuring critical functionality works as intended. -config KCSAN_TEST - tristate "KCSAN test for integrated runtime behaviour" +config KCSAN_KUNIT_TEST + tristate "KCSAN test for integrated runtime behaviour" if !KUNIT_ALL_TESTS + default KUNIT_ALL_TESTS depends on TRACEPOINTS && KUNIT select TORTURE_TEST help From f6a149140321274cbd955dee50798fe191841f94 Mon Sep 17 00:00:00 2001 From: Marco Elver Date: Wed, 13 Jan 2021 17:05:57 +0100 Subject: [PATCH 3/4] kcsan: Switch to KUNIT_CASE_PARAM for parameterized tests Since KUnit now support parameterized tests via KUNIT_CASE_PARAM, update KCSAN's test to switch to it for parameterized tests. This simplifies parameterized tests and gets rid of the "parameters in case name" workaround (hack). At the same time, we can increase the maximum number of threads used, because on systems with too few CPUs, KUnit allows us to now stop at the maximum useful threads and not unnecessarily execute redundant test cases with (the same) limited threads as had been the case before. Reviewed-by: David Gow Signed-off-by: Marco Elver Signed-off-by: Paul E. McKenney --- kernel/kcsan/kcsan_test.c | 118 ++++++++++++++++++-------------------- 1 file changed, 55 insertions(+), 63 deletions(-) diff --git a/kernel/kcsan/kcsan_test.c b/kernel/kcsan/kcsan_test.c index f16f632eb416..b71751fc9f4f 100644 --- a/kernel/kcsan/kcsan_test.c +++ b/kernel/kcsan/kcsan_test.c @@ -13,6 +13,8 @@ * Author: Marco Elver */ +#define pr_fmt(fmt) "kcsan_test: " fmt + #include #include #include @@ -951,22 +953,53 @@ static void test_atomic_builtins(struct kunit *test) } /* - * Each test case is run with different numbers of threads. Until KUnit supports - * passing arguments for each test case, we encode #threads in the test case - * name (read by get_num_threads()). [The '-' was chosen as a stylistic - * preference to separate test name and #threads.] + * Generate thread counts for all test cases. Values generated are in interval + * [2, 5] followed by exponentially increasing thread counts from 8 to 32. * * The thread counts are chosen to cover potentially interesting boundaries and - * corner cases (range 2-5), and then stress the system with larger counts. + * corner cases (2 to 5), and then stress the system with larger counts. */ -#define KCSAN_KUNIT_CASE(test_name) \ - { .run_case = test_name, .name = #test_name "-02" }, \ - { .run_case = test_name, .name = #test_name "-03" }, \ - { .run_case = test_name, .name = #test_name "-04" }, \ - { .run_case = test_name, .name = #test_name "-05" }, \ - { .run_case = test_name, .name = #test_name "-08" }, \ - { .run_case = test_name, .name = #test_name "-16" } +static const void *nthreads_gen_params(const void *prev, char *desc) +{ + long nthreads = (long)prev; + if (nthreads < 0 || nthreads >= 32) + nthreads = 0; /* stop */ + else if (!nthreads) + nthreads = 2; /* initial value */ + else if (nthreads < 5) + nthreads++; + else if (nthreads == 5) + nthreads = 8; + else + nthreads *= 2; + + if (!IS_ENABLED(CONFIG_PREEMPT) || !IS_ENABLED(CONFIG_KCSAN_INTERRUPT_WATCHER)) { + /* + * Without any preemption, keep 2 CPUs free for other tasks, one + * of which is the main test case function checking for + * completion or failure. + */ + const long min_unused_cpus = IS_ENABLED(CONFIG_PREEMPT_NONE) ? 2 : 0; + const long min_required_cpus = 2 + min_unused_cpus; + + if (num_online_cpus() < min_required_cpus) { + pr_err_once("Too few online CPUs (%u < %d) for test\n", + num_online_cpus(), min_required_cpus); + nthreads = 0; + } else if (nthreads >= num_online_cpus() - min_unused_cpus) { + /* Use negative value to indicate last param. */ + nthreads = -(num_online_cpus() - min_unused_cpus); + pr_warn_once("Limiting number of threads to %ld (only %d online CPUs)\n", + -nthreads, num_online_cpus()); + } + } + + snprintf(desc, KUNIT_PARAM_DESC_SIZE, "threads=%ld", abs(nthreads)); + return (void *)nthreads; +} + +#define KCSAN_KUNIT_CASE(test_name) KUNIT_CASE_PARAM(test_name, nthreads_gen_params) static struct kunit_case kcsan_test_cases[] = { KCSAN_KUNIT_CASE(test_basic), KCSAN_KUNIT_CASE(test_concurrent_races), @@ -996,24 +1029,6 @@ static struct kunit_case kcsan_test_cases[] = { /* ===== End test cases ===== */ -/* Get number of threads encoded in test name. */ -static bool __no_kcsan -get_num_threads(const char *test, int *nthreads) -{ - int len = strlen(test); - - if (WARN_ON(len < 3)) - return false; - - *nthreads = test[len - 1] - '0'; - *nthreads += (test[len - 2] - '0') * 10; - - if (WARN_ON(*nthreads < 0)) - return false; - - return true; -} - /* Concurrent accesses from interrupts. */ __no_kcsan static void access_thread_timer(struct timer_list *timer) @@ -1076,9 +1091,6 @@ static int test_init(struct kunit *test) if (!torture_init_begin((char *)test->name, 1)) return -EBUSY; - if (!get_num_threads(test->name, &nthreads)) - goto err; - if (WARN_ON(threads)) goto err; @@ -1087,38 +1099,18 @@ static int test_init(struct kunit *test) goto err; } - if (!IS_ENABLED(CONFIG_PREEMPT) || !IS_ENABLED(CONFIG_KCSAN_INTERRUPT_WATCHER)) { - /* - * Without any preemption, keep 2 CPUs free for other tasks, one - * of which is the main test case function checking for - * completion or failure. - */ - const int min_unused_cpus = IS_ENABLED(CONFIG_PREEMPT_NONE) ? 2 : 0; - const int min_required_cpus = 2 + min_unused_cpus; + nthreads = abs((long)test->param_value); + if (WARN_ON(!nthreads)) + goto err; - if (num_online_cpus() < min_required_cpus) { - pr_err("%s: too few online CPUs (%u < %d) for test", - test->name, num_online_cpus(), min_required_cpus); + threads = kcalloc(nthreads + 1, sizeof(struct task_struct *), GFP_KERNEL); + if (WARN_ON(!threads)) + goto err; + + threads[nthreads] = NULL; + for (i = 0; i < nthreads; ++i) { + if (torture_create_kthread(access_thread, NULL, threads[i])) goto err; - } else if (nthreads > num_online_cpus() - min_unused_cpus) { - nthreads = num_online_cpus() - min_unused_cpus; - pr_warn("%s: limiting number of threads to %d\n", - test->name, nthreads); - } - } - - if (nthreads) { - threads = kcalloc(nthreads + 1, sizeof(struct task_struct *), - GFP_KERNEL); - if (WARN_ON(!threads)) - goto err; - - threads[nthreads] = NULL; - for (i = 0; i < nthreads; ++i) { - if (torture_create_kthread(access_thread, NULL, - threads[i])) - goto err; - } } torture_init_end(); From bd0ccc4afca2d6ae0029cae35c4f1d2e2ade7579 Mon Sep 17 00:00:00 2001 From: Marco Elver Date: Fri, 15 Jan 2021 18:09:53 +0100 Subject: [PATCH 4/4] kcsan: Add missing license and copyright headers Adds missing license and/or copyright headers for KCSAN source files. Signed-off-by: Marco Elver Signed-off-by: Paul E. McKenney --- Documentation/dev-tools/kcsan.rst | 3 +++ include/linux/kcsan-checks.h | 6 ++++++ include/linux/kcsan.h | 7 +++++++ kernel/kcsan/atomic.h | 5 +++++ kernel/kcsan/core.c | 5 +++++ kernel/kcsan/debugfs.c | 5 +++++ kernel/kcsan/encoding.h | 5 +++++ kernel/kcsan/kcsan.h | 3 ++- kernel/kcsan/report.c | 5 +++++ kernel/kcsan/selftest.c | 5 +++++ 10 files changed, 48 insertions(+), 1 deletion(-) diff --git a/Documentation/dev-tools/kcsan.rst b/Documentation/dev-tools/kcsan.rst index be7a0b0e1f28..d85ce238ace7 100644 --- a/Documentation/dev-tools/kcsan.rst +++ b/Documentation/dev-tools/kcsan.rst @@ -1,3 +1,6 @@ +.. SPDX-License-Identifier: GPL-2.0 +.. Copyright (C) 2019, Google LLC. + The Kernel Concurrency Sanitizer (KCSAN) ======================================== diff --git a/include/linux/kcsan-checks.h b/include/linux/kcsan-checks.h index cf14840609ce..9fd0ad80fef6 100644 --- a/include/linux/kcsan-checks.h +++ b/include/linux/kcsan-checks.h @@ -1,4 +1,10 @@ /* SPDX-License-Identifier: GPL-2.0 */ +/* + * KCSAN access checks and modifiers. These can be used to explicitly check + * uninstrumented accesses, or change KCSAN checking behaviour of accesses. + * + * Copyright (C) 2019, Google LLC. + */ #ifndef _LINUX_KCSAN_CHECKS_H #define _LINUX_KCSAN_CHECKS_H diff --git a/include/linux/kcsan.h b/include/linux/kcsan.h index 53340d8789f9..fc266ecb2a4d 100644 --- a/include/linux/kcsan.h +++ b/include/linux/kcsan.h @@ -1,4 +1,11 @@ /* SPDX-License-Identifier: GPL-2.0 */ +/* + * The Kernel Concurrency Sanitizer (KCSAN) infrastructure. Public interface and + * data structures to set up runtime. See kcsan-checks.h for explicit checks and + * modifiers. For more info please see Documentation/dev-tools/kcsan.rst. + * + * Copyright (C) 2019, Google LLC. + */ #ifndef _LINUX_KCSAN_H #define _LINUX_KCSAN_H diff --git a/kernel/kcsan/atomic.h b/kernel/kcsan/atomic.h index 75fe701f4127..530ae1bda8e7 100644 --- a/kernel/kcsan/atomic.h +++ b/kernel/kcsan/atomic.h @@ -1,4 +1,9 @@ /* SPDX-License-Identifier: GPL-2.0 */ +/* + * Rules for implicitly atomic memory accesses. + * + * Copyright (C) 2019, Google LLC. + */ #ifndef _KERNEL_KCSAN_ATOMIC_H #define _KERNEL_KCSAN_ATOMIC_H diff --git a/kernel/kcsan/core.c b/kernel/kcsan/core.c index 23e7acb5c667..45c821d4e8bd 100644 --- a/kernel/kcsan/core.c +++ b/kernel/kcsan/core.c @@ -1,4 +1,9 @@ // SPDX-License-Identifier: GPL-2.0 +/* + * KCSAN core runtime. + * + * Copyright (C) 2019, Google LLC. + */ #define pr_fmt(fmt) "kcsan: " fmt diff --git a/kernel/kcsan/debugfs.c b/kernel/kcsan/debugfs.c index 209ad8dcfcec..c1dd02f3be8b 100644 --- a/kernel/kcsan/debugfs.c +++ b/kernel/kcsan/debugfs.c @@ -1,4 +1,9 @@ // SPDX-License-Identifier: GPL-2.0 +/* + * KCSAN debugfs interface. + * + * Copyright (C) 2019, Google LLC. + */ #define pr_fmt(fmt) "kcsan: " fmt diff --git a/kernel/kcsan/encoding.h b/kernel/kcsan/encoding.h index 7ee405524904..170a2bb22f53 100644 --- a/kernel/kcsan/encoding.h +++ b/kernel/kcsan/encoding.h @@ -1,4 +1,9 @@ /* SPDX-License-Identifier: GPL-2.0 */ +/* + * KCSAN watchpoint encoding. + * + * Copyright (C) 2019, Google LLC. + */ #ifndef _KERNEL_KCSAN_ENCODING_H #define _KERNEL_KCSAN_ENCODING_H diff --git a/kernel/kcsan/kcsan.h b/kernel/kcsan/kcsan.h index 87ccdb3b051f..9881099d4179 100644 --- a/kernel/kcsan/kcsan.h +++ b/kernel/kcsan/kcsan.h @@ -1,8 +1,9 @@ /* SPDX-License-Identifier: GPL-2.0 */ - /* * The Kernel Concurrency Sanitizer (KCSAN) infrastructure. For more info please * see Documentation/dev-tools/kcsan.rst. + * + * Copyright (C) 2019, Google LLC. */ #ifndef _KERNEL_KCSAN_KCSAN_H diff --git a/kernel/kcsan/report.c b/kernel/kcsan/report.c index d3bf87e6007c..13dce3c664d6 100644 --- a/kernel/kcsan/report.c +++ b/kernel/kcsan/report.c @@ -1,4 +1,9 @@ // SPDX-License-Identifier: GPL-2.0 +/* + * KCSAN reporting. + * + * Copyright (C) 2019, Google LLC. + */ #include #include diff --git a/kernel/kcsan/selftest.c b/kernel/kcsan/selftest.c index 9014a3a82cf9..7f29cb0f5e63 100644 --- a/kernel/kcsan/selftest.c +++ b/kernel/kcsan/selftest.c @@ -1,4 +1,9 @@ // SPDX-License-Identifier: GPL-2.0 +/* + * KCSAN short boot-time selftests. + * + * Copyright (C) 2019, Google LLC. + */ #define pr_fmt(fmt) "kcsan: " fmt