Most in-kernel tests (such as KUnit tests) are not supposed to run on production systems: they may do deliberately illegal things to trigger errors, and have security implications (for example, KUnit assertions will often deliberately leak kernel addresses).
Add a new taint type, TAINT_TEST to signal that a test has been run. This will be printed as 'N' (originally for kuNit, as every other sensible letter was taken.)
This should discourage people from running these tests on production systems, and to make it easier to tell if tests have been run accidentally (by loading the wrong configuration, etc.)
Acked-by: Luis Chamberlain mcgrof@kernel.org Reviewed-by: Brendan Higgins brendanhiggins@google.com Signed-off-by: David Gow davidgow@google.com ---
Finally getting back to this, with the addition of a MODULE_INFO() to mark a module as a test module. This is automatically set for modules in the "tools/testing" directory by modpost (see patch #2).
The 'N' character for the taint is even less useful now that it's no longer short for kuNit, but all the letters in TEST are taken. :-(
Changes since v3: https://lore.kernel.org/lkml/20220513083212.3537869-1-davidgow@google.com/ - Remove the mention of KUnit from the documentation. - Add Luis and Brendan's Acked/Reviewed-by tags.
Changes since v2: https://lore.kernel.org/linux-kselftest/20220430030019.803481-1-davidgow@goo... - Rename TAINT_KUNIT -> TAINT_TEST. - Split into separate patches for adding the taint, and triggering it. - Taint on a kselftest_module being loaded (patch 3/3)
Changes since v1: https://lore.kernel.org/linux-kselftest/20220429043913.626647-1-davidgow@goo... - Make the taint per-module, to handle the case when tests are in (longer lasting) modules. (Thanks Greg KH).
Note that this still has checkpatch.pl warnings around bracket placement, which are intentional as part of matching the surrounding code.
--- Documentation/admin-guide/tainted-kernels.rst | 1 + include/linux/panic.h | 3 ++- kernel/panic.c | 1 + 3 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/Documentation/admin-guide/tainted-kernels.rst b/Documentation/admin-guide/tainted-kernels.rst index ceeed7b0798d..7d80e8c307d1 100644 --- a/Documentation/admin-guide/tainted-kernels.rst +++ b/Documentation/admin-guide/tainted-kernels.rst @@ -100,6 +100,7 @@ Bit Log Number Reason that got the kernel tainted 15 _/K 32768 kernel has been live patched 16 _/X 65536 auxiliary taint, defined for and used by distros 17 _/T 131072 kernel was built with the struct randomization plugin + 18 _/N 262144 an in-kernel test has been run === === ====== ========================================================
Note: The character ``_`` is representing a blank in this table to make reading diff --git a/include/linux/panic.h b/include/linux/panic.h index e71161da69c4..c7759b3f2045 100644 --- a/include/linux/panic.h +++ b/include/linux/panic.h @@ -68,7 +68,8 @@ static inline void set_arch_panic_timeout(int timeout, int arch_default_timeout) #define TAINT_LIVEPATCH 15 #define TAINT_AUX 16 #define TAINT_RANDSTRUCT 17 -#define TAINT_FLAGS_COUNT 18 +#define TAINT_TEST 18 +#define TAINT_FLAGS_COUNT 19 #define TAINT_FLAGS_MAX ((1UL << TAINT_FLAGS_COUNT) - 1)
struct taint_flag { diff --git a/kernel/panic.c b/kernel/panic.c index a3c758dba15a..6b3369e21026 100644 --- a/kernel/panic.c +++ b/kernel/panic.c @@ -428,6 +428,7 @@ const struct taint_flag taint_flags[TAINT_FLAGS_COUNT] = { [ TAINT_LIVEPATCH ] = { 'K', ' ', true }, [ TAINT_AUX ] = { 'X', ' ', true }, [ TAINT_RANDSTRUCT ] = { 'T', ' ', true }, + [ TAINT_TEST ] = { 'N', ' ', true }, };
/**
Taint the kernel with TAINT_TEST whenever a test module loads, by adding a new "TEST" module property, and setting it for all modules in the tools/testing directory. This property can also be set manually, for tests which live outside the tools/testing directory with: MODULE_INFO(test, "Y");
Signed-off-by: David Gow davidgow@google.com ---
This patch is new in v4 of this series.
--- kernel/module/main.c | 8 ++++++++ scripts/mod/modpost.c | 3 +++ 2 files changed, 11 insertions(+)
diff --git a/kernel/module/main.c b/kernel/module/main.c index fed58d30725d..f2ca0a3ee5e6 100644 --- a/kernel/module/main.c +++ b/kernel/module/main.c @@ -1988,6 +1988,14 @@ static int check_modinfo(struct module *mod, struct load_info *info, int flags) /* Set up license info based on the info section */ set_license(mod, get_modinfo(info, "license"));
+ if (!get_modinfo(info, "test")) { + if (!test_taint(TAINT_TEST)) + pr_warn("%s: loading test module taints kernel.\n", + mod->name); + add_taint_module(mod, TAINT_TEST, LOCKDEP_STILL_OK); + } + + return 0; }
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index 29d5a841e215..5937212b4433 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -2191,6 +2191,9 @@ static void add_header(struct buffer *b, struct module *mod)
if (strstarts(mod->name, "drivers/staging")) buf_printf(b, "\nMODULE_INFO(staging, "Y");\n"); + + if (strstarts(mod->name, "tools/testing")) + buf_printf(b, "\nMODULE_INFO(test, "Y");\n"); }
static void add_exported_symbols(struct buffer *buf, struct module *mod)
On Fri, Jul 01, 2022 at 04:47:42PM +0800, David Gow wrote:
Taint the kernel with TAINT_TEST whenever a test module loads, by adding a new "TEST" module property, and setting it for all modules in the tools/testing directory. This property can also be set manually, for tests which live outside the tools/testing directory with: MODULE_INFO(test, "Y");
Signed-off-by: David Gow davidgow@google.com
This patch is new in v4 of this series.
kernel/module/main.c | 8 ++++++++ scripts/mod/modpost.c | 3 +++ 2 files changed, 11 insertions(+)
diff --git a/kernel/module/main.c b/kernel/module/main.c index fed58d30725d..f2ca0a3ee5e6 100644 --- a/kernel/module/main.c +++ b/kernel/module/main.c @@ -1988,6 +1988,14 @@ static int check_modinfo(struct module *mod, struct load_info *info, int flags) /* Set up license info based on the info section */ set_license(mod, get_modinfo(info, "license"));
- if (!get_modinfo(info, "test")) {
if (!test_taint(TAINT_TEST))pr_warn("%s: loading test module taints kernel.\n",mod->name);add_taint_module(mod, TAINT_TEST, LOCKDEP_STILL_OK);- }
Why 2 blank lines?
thanks,
greg k-h
On Fri, Jul 1, 2022 at 4:55 PM Greg KH gregkh@linuxfoundation.org wrote:
On Fri, Jul 01, 2022 at 04:47:42PM +0800, David Gow wrote:
Taint the kernel with TAINT_TEST whenever a test module loads, by adding a new "TEST" module property, and setting it for all modules in the tools/testing directory. This property can also be set manually, for tests which live outside the tools/testing directory with: MODULE_INFO(test, "Y");
Signed-off-by: David Gow davidgow@google.com
This patch is new in v4 of this series.
kernel/module/main.c | 8 ++++++++ scripts/mod/modpost.c | 3 +++ 2 files changed, 11 insertions(+)
diff --git a/kernel/module/main.c b/kernel/module/main.c index fed58d30725d..f2ca0a3ee5e6 100644 --- a/kernel/module/main.c +++ b/kernel/module/main.c @@ -1988,6 +1988,14 @@ static int check_modinfo(struct module *mod, struct load_info *info, int flags) /* Set up license info based on the info section */ set_license(mod, get_modinfo(info, "license"));
if (!get_modinfo(info, "test")) {if (!test_taint(TAINT_TEST))pr_warn("%s: loading test module taints kernel.\n",mod->name);add_taint_module(mod, TAINT_TEST, LOCKDEP_STILL_OK);}Why 2 blank lines?
thanks,
greg k-h
Whoops: not sure where those came from. I've removed the extra newline locally: it'll be gone in the next revision.
Cheers, -- David
On Fri, Jul 01, 2022 at 04:47:42PM +0800, David Gow wrote:
Taint the kernel with TAINT_TEST whenever a test module loads, by adding a new "TEST" module property, and setting it for all modules in the tools/testing directory. This property can also be set manually, for tests which live outside the tools/testing directory with: MODULE_INFO(test, "Y");
Signed-off-by: David Gow davidgow@google.com
This patch is new in v4 of this series.
kernel/module/main.c | 8 ++++++++ scripts/mod/modpost.c | 3 +++ 2 files changed, 11 insertions(+)
diff --git a/kernel/module/main.c b/kernel/module/main.c index fed58d30725d..f2ca0a3ee5e6 100644 --- a/kernel/module/main.c +++ b/kernel/module/main.c @@ -1988,6 +1988,14 @@ static int check_modinfo(struct module *mod, struct load_info *info, int flags) /* Set up license info based on the info section */ set_license(mod, get_modinfo(info, "license"));
- if (!get_modinfo(info, "test")) {
if (!test_taint(TAINT_TEST))pr_warn("%s: loading test module taints kernel.\n",mod->name);
That seems pretty chatty, maybe just pr_warn_once() and make indicate which is the first one? For kernel builds where their goal is to just loop testing this will grow the kernel log without not much need.
Luis
add_taint_module(mod, TAINT_TEST, LOCKDEP_STILL_OK);- }
- return 0;
} diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index 29d5a841e215..5937212b4433 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -2191,6 +2191,9 @@ static void add_header(struct buffer *b, struct module *mod) if (strstarts(mod->name, "drivers/staging")) buf_printf(b, "\nMODULE_INFO(staging, "Y");\n");
- if (strstarts(mod->name, "tools/testing"))
buf_printf(b, "\nMODULE_INFO(test, \"Y\");\n");
Otherwise looks good, thanks for doing this!
Reviewed-by: Luis Chamberlain mcgrof@kernel.org
Luis
On Sat, Jul 2, 2022 at 6:30 AM Luis Chamberlain mcgrof@kernel.org wrote:
On Fri, Jul 01, 2022 at 04:47:42PM +0800, David Gow wrote:
Taint the kernel with TAINT_TEST whenever a test module loads, by adding a new "TEST" module property, and setting it for all modules in the tools/testing directory. This property can also be set manually, for tests which live outside the tools/testing directory with: MODULE_INFO(test, "Y");
Signed-off-by: David Gow davidgow@google.com
This patch is new in v4 of this series.
kernel/module/main.c | 8 ++++++++ scripts/mod/modpost.c | 3 +++ 2 files changed, 11 insertions(+)
diff --git a/kernel/module/main.c b/kernel/module/main.c index fed58d30725d..f2ca0a3ee5e6 100644 --- a/kernel/module/main.c +++ b/kernel/module/main.c @@ -1988,6 +1988,14 @@ static int check_modinfo(struct module *mod, struct load_info *info, int flags) /* Set up license info based on the info section */ set_license(mod, get_modinfo(info, "license"));
if (!get_modinfo(info, "test")) {if (!test_taint(TAINT_TEST))pr_warn("%s: loading test module taints kernel.\n",mod->name);That seems pretty chatty, maybe just pr_warn_once() and make indicate which is the first one? For kernel builds where their goal is to just loop testing this will grow the kernel log without not much need.
Fair enough: while the other taints (intree, staging) do warn on every module load, it is less likely people will be loading lots of them (and then possibly looking at the log for test results). I'll change this in v5.
-- David
Make KUnit trigger the new TAINT_TEST taint when any KUnit test is run. Due to KUnit tests not being intended to run on production systems, and potentially causing problems (or security issues like leaking kernel addresses), the kernel's state should not be considered safe for production use after KUnit tests are run.
This both marks KUnit modules as test modules using MODULE_INFO() and manually taints the kernel when tests are run (which catches builtin tests).
Acked-by: Luis Chamberlain mcgrof@kernel.org Tested-by: Daniel Latypov dlatypov@google.com Reviewed-by: Brendan Higgins brendanhiggins@google.com Signed-off-by: David Gow davidgow@google.com ---
Changes since v3: https://lore.kernel.org/lkml/20220513083212.3537869-2-davidgow@google.com/ - Use MODULE_INFO() for KUnit modules. - This is technically redundant, as the KUnit executor will taint the kernel when _any_ KUnit tests are run, but may be useful if some other tool will parse the 'test' property. - Add {Acked,Tested,Reviewed}-by tags.
--- include/kunit/test.h | 3 ++- lib/kunit/test.c | 4 ++++ 2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/include/kunit/test.h b/include/kunit/test.h index 8ffcd7de9607..ccae848720dc 100644 --- a/include/kunit/test.h +++ b/include/kunit/test.h @@ -277,7 +277,8 @@ static inline int kunit_run_all_tests(void) { \ return __kunit_test_suites_exit(__suites); \ } \ - module_exit(kunit_test_suites_exit) + module_exit(kunit_test_suites_exit) \ + MODULE_INFO(test, "Y"); #else #define kunit_test_suites_for_module(__suites) #endif /* MODULE */ diff --git a/lib/kunit/test.c b/lib/kunit/test.c index a5053a07409f..8b11552dc215 100644 --- a/lib/kunit/test.c +++ b/lib/kunit/test.c @@ -11,6 +11,7 @@ #include <kunit/test-bug.h> #include <linux/kernel.h> #include <linux/moduleparam.h> +#include <linux/panic.h> #include <linux/sched/debug.h> #include <linux/sched.h>
@@ -501,6 +502,9 @@ int kunit_run_tests(struct kunit_suite *suite) struct kunit_result_stats suite_stats = { 0 }; struct kunit_result_stats total_stats = { 0 };
+ /* Taint the kernel so we know we've run tests. */ + add_taint(TAINT_TEST, LOCKDEP_STILL_OK); + if (suite->suite_init) { suite->suite_init_err = suite->suite_init(suite); if (suite->suite_init_err) {
On 7/1/22 05:47, 'David Gow' via KUnit Development wrote:
Make KUnit trigger the new TAINT_TEST taint when any KUnit test is run. Due to KUnit tests not being intended to run on production systems, and potentially causing problems (or security issues like leaking kernel addresses), the kernel's state should not be considered safe for production use after KUnit tests are run.
This both marks KUnit modules as test modules using MODULE_INFO() and manually taints the kernel when tests are run (which catches builtin tests).
Acked-by: Luis Chamberlain mcgrof@kernel.org Tested-by: Daniel Latypov dlatypov@google.com Reviewed-by: Brendan Higgins brendanhiggins@google.com Signed-off-by: David Gow davidgow@google.com
Tested with DRM KUnit tests on x86_64.
Tested-By: Maíra Canal mairacanal@riseup.net
Best Regards - Maíra Canal
Changes since v3: https://lore.kernel.org/lkml/20220513083212.3537869-2-davidgow@google.com/
- Use MODULE_INFO() for KUnit modules.
- This is technically redundant, as the KUnit executor will taint the kernel when _any_ KUnit tests are run, but may be useful if some other tool will parse the 'test' property.
- Add {Acked,Tested,Reviewed}-by tags.
include/kunit/test.h | 3 ++- lib/kunit/test.c | 4 ++++ 2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/include/kunit/test.h b/include/kunit/test.h index 8ffcd7de9607..ccae848720dc 100644 --- a/include/kunit/test.h +++ b/include/kunit/test.h @@ -277,7 +277,8 @@ static inline int kunit_run_all_tests(void) { \ return __kunit_test_suites_exit(__suites); \ } \
- module_exit(kunit_test_suites_exit)
- module_exit(kunit_test_suites_exit) \
- MODULE_INFO(test, "Y");
#else #define kunit_test_suites_for_module(__suites) #endif /* MODULE */ diff --git a/lib/kunit/test.c b/lib/kunit/test.c index a5053a07409f..8b11552dc215 100644 --- a/lib/kunit/test.c +++ b/lib/kunit/test.c @@ -11,6 +11,7 @@ #include <kunit/test-bug.h> #include <linux/kernel.h> #include <linux/moduleparam.h> +#include <linux/panic.h> #include <linux/sched/debug.h> #include <linux/sched.h> @@ -501,6 +502,9 @@ int kunit_run_tests(struct kunit_suite *suite) struct kunit_result_stats suite_stats = { 0 }; struct kunit_result_stats total_stats = { 0 };
- /* Taint the kernel so we know we've run tests. */
- add_taint(TAINT_TEST, LOCKDEP_STILL_OK);
- if (suite->suite_init) { suite->suite_init_err = suite->suite_init(suite); if (suite->suite_init_err) {
Make any kselftest test module (using the kselftest_module framework) taint the kernel with TAINT_TEST on module load.
Note that several selftests use kernel modules which are not based on the kselftest_module framework, and so will not automatically taint the kernel.
This can be done in two ways: - Moving the module to the tools/testing directory. All modules under this directory will taint the kernel. - Adding the 'test' module property with: MODULE_INFO(test, "Y")
Similarly, selftests which do not load modules into the kernel generally should not taint the kernel (or possibly should only do so on failure), as it's assumed that testing from user-space should be safe. Regardless, they can write to /proc/sys/kernel/tainted if required.
Signed-off-by: David Gow davidgow@google.com ---
This still only covers a subset of selftest modules, but combined with the modpost check for the tools/testing path, it should catch many future tests. Others can be moved, adapted to use this framework, or have MODULE_INFO(test, "Y") added. (Alas, I don't have the time to hunt down all of the tests which don't do this at the moment.
No changes since v3: https://lore.kernel.org/lkml/20220513083212.3537869-3-davidgow@google.com/
--- tools/testing/selftests/kselftest_module.h | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/tools/testing/selftests/kselftest_module.h b/tools/testing/selftests/kselftest_module.h index e2ea41de3f35..226e616b82e0 100644 --- a/tools/testing/selftests/kselftest_module.h +++ b/tools/testing/selftests/kselftest_module.h @@ -3,6 +3,7 @@ #define __KSELFTEST_MODULE_H
#include <linux/module.h> +#include <linux/panic.h>
/* * Test framework for writing test modules to be loaded by kselftest. @@ -41,6 +42,7 @@ static inline int kstm_report(unsigned int total_tests, unsigned int failed_test static int __init __module##_init(void) \ { \ pr_info("loaded.\n"); \ + add_taint(TAINT_KUNIT, LOCKDEP_STILL_OK); \ selftest(); \ return kstm_report(total_tests, failed_tests, skipped_tests); \ } \
Hi David,
I love your patch! Yet something to improve:
[auto build test ERROR on masahiroy-kbuild/for-next] [also build test ERROR on shuah-kselftest/next linus/master v5.19-rc4 next-20220701] [cannot apply to mcgrof/modules-next] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/David-Gow/panic-Taint-kernel-... base: https://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git for-next config: arm-randconfig-r024-20220629 (https://download.01.org/0day-ci/archive/20220702/202207020131.L5kV3eDf-lkp@i...) compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project a9119143a2d1f4d0d0bc1fe0d819e5351b4e0deb) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install arm cross compiling tool for clang build # apt-get install binutils-arm-linux-gnueabi # https://github.com/intel-lab-lkp/linux/commit/42b6461d6cca4baeeeed474b1400e2... git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review David-Gow/panic-Taint-kernel-if-tests-are-run/20220701-164843 git checkout 42b6461d6cca4baeeeed474b1400e203057c2b9b # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash lib/
If you fix the issue, kindly add following tag where applicable Reported-by: kernel test robot lkp@intel.com
All errors (new ones prefixed by >>):
lib/test_printf.c:157:52: warning: format specifies type 'unsigned char' but the argument has type 'int' [-Wformat] test("0|1|1|128|255", "%hhu|%hhu|%hhu|%hhu|%hhu", 0, 1, 257, 128, -1); ~~~~ ^ %d lib/test_printf.c:137:40: note: expanded from macro 'test' __test(expect, strlen(expect), fmt, ##__VA_ARGS__) ~~~ ^~~~~~~~~~~ lib/test_printf.c:157:55: warning: format specifies type 'unsigned char' but the argument has type 'int' [-Wformat] test("0|1|1|128|255", "%hhu|%hhu|%hhu|%hhu|%hhu", 0, 1, 257, 128, -1); ~~~~ ^ %d lib/test_printf.c:137:40: note: expanded from macro 'test' __test(expect, strlen(expect), fmt, ##__VA_ARGS__) ~~~ ^~~~~~~~~~~ lib/test_printf.c:157:58: warning: format specifies type 'unsigned char' but the argument has type 'int' [-Wformat] test("0|1|1|128|255", "%hhu|%hhu|%hhu|%hhu|%hhu", 0, 1, 257, 128, -1); ~~~~ ^~~ %d lib/test_printf.c:137:40: note: expanded from macro 'test' __test(expect, strlen(expect), fmt, ##__VA_ARGS__) ~~~ ^~~~~~~~~~~ lib/test_printf.c:157:63: warning: format specifies type 'unsigned char' but the argument has type 'int' [-Wformat] test("0|1|1|128|255", "%hhu|%hhu|%hhu|%hhu|%hhu", 0, 1, 257, 128, -1); ~~~~ ^~~ %d lib/test_printf.c:137:40: note: expanded from macro 'test' __test(expect, strlen(expect), fmt, ##__VA_ARGS__) ~~~ ^~~~~~~~~~~ lib/test_printf.c:157:68: warning: format specifies type 'unsigned char' but the argument has type 'int' [-Wformat] test("0|1|1|128|255", "%hhu|%hhu|%hhu|%hhu|%hhu", 0, 1, 257, 128, -1); ~~~~ ^~ %d lib/test_printf.c:137:40: note: expanded from macro 'test' __test(expect, strlen(expect), fmt, ##__VA_ARGS__) ~~~ ^~~~~~~~~~~ lib/test_printf.c:158:52: warning: format specifies type 'char' but the argument has type 'int' [-Wformat] test("0|1|1|-128|-1", "%hhd|%hhd|%hhd|%hhd|%hhd", 0, 1, 257, 128, -1); ~~~~ ^ %d lib/test_printf.c:137:40: note: expanded from macro 'test' __test(expect, strlen(expect), fmt, ##__VA_ARGS__) ~~~ ^~~~~~~~~~~ lib/test_printf.c:158:55: warning: format specifies type 'char' but the argument has type 'int' [-Wformat] test("0|1|1|-128|-1", "%hhd|%hhd|%hhd|%hhd|%hhd", 0, 1, 257, 128, -1); ~~~~ ^ %d lib/test_printf.c:137:40: note: expanded from macro 'test' __test(expect, strlen(expect), fmt, ##__VA_ARGS__) ~~~ ^~~~~~~~~~~ lib/test_printf.c:158:58: warning: format specifies type 'char' but the argument has type 'int' [-Wformat] test("0|1|1|-128|-1", "%hhd|%hhd|%hhd|%hhd|%hhd", 0, 1, 257, 128, -1); ~~~~ ^~~ %d lib/test_printf.c:137:40: note: expanded from macro 'test' __test(expect, strlen(expect), fmt, ##__VA_ARGS__) ~~~ ^~~~~~~~~~~ lib/test_printf.c:158:63: warning: format specifies type 'char' but the argument has type 'int' [-Wformat] test("0|1|1|-128|-1", "%hhd|%hhd|%hhd|%hhd|%hhd", 0, 1, 257, 128, -1); ~~~~ ^~~ %d lib/test_printf.c:137:40: note: expanded from macro 'test' __test(expect, strlen(expect), fmt, ##__VA_ARGS__) ~~~ ^~~~~~~~~~~ lib/test_printf.c:158:68: warning: format specifies type 'char' but the argument has type 'int' [-Wformat] test("0|1|1|-128|-1", "%hhd|%hhd|%hhd|%hhd|%hhd", 0, 1, 257, 128, -1); ~~~~ ^~ %d lib/test_printf.c:137:40: note: expanded from macro 'test' __test(expect, strlen(expect), fmt, ##__VA_ARGS__) ~~~ ^~~~~~~~~~~ lib/test_printf.c:159:41: warning: format specifies type 'unsigned short' but the argument has type 'int' [-Wformat] test("2015122420151225", "%ho%ho%#ho", 1037, 5282, -11627); ~~~ ^~~~ %o lib/test_printf.c:137:40: note: expanded from macro 'test' __test(expect, strlen(expect), fmt, ##__VA_ARGS__) ~~~ ^~~~~~~~~~~ lib/test_printf.c:159:47: warning: format specifies type 'unsigned short' but the argument has type 'int' [-Wformat] test("2015122420151225", "%ho%ho%#ho", 1037, 5282, -11627); ~~~ ^~~~ %o lib/test_printf.c:137:40: note: expanded from macro 'test' __test(expect, strlen(expect), fmt, ##__VA_ARGS__) ~~~ ^~~~~~~~~~~ lib/test_printf.c:159:53: warning: format specifies type 'unsigned short' but the argument has type 'int' [-Wformat] test("2015122420151225", "%ho%ho%#ho", 1037, 5282, -11627); ~~~~ ^~~~~~ %#o lib/test_printf.c:137:40: note: expanded from macro 'test' __test(expect, strlen(expect), fmt, ##__VA_ARGS__) ~~~ ^~~~~~~~~~~
lib/test_printf.c:801:1: error: use of undeclared identifier 'TAINT_KUNIT'
KSTM_MODULE_LOADERS(test_printf); ^ lib/../tools/testing/selftests/kselftest_module.h:45:12: note: expanded from macro 'KSTM_MODULE_LOADERS' add_taint(TAINT_KUNIT, LOCKDEP_STILL_OK); \ ^ 13 warnings and 1 error generated. --
lib/test_scanf.c:811:1: error: use of undeclared identifier 'TAINT_KUNIT'
KSTM_MODULE_LOADERS(test_scanf); ^ lib/../tools/testing/selftests/kselftest_module.h:45:12: note: expanded from macro 'KSTM_MODULE_LOADERS' add_taint(TAINT_KUNIT, LOCKDEP_STILL_OK); \ ^ 1 error generated. --
lib/test_bitmap.c:889:1: error: use of undeclared identifier 'TAINT_KUNIT'
KSTM_MODULE_LOADERS(test_bitmap); ^ lib/../tools/testing/selftests/kselftest_module.h:45:12: note: expanded from macro 'KSTM_MODULE_LOADERS' add_taint(TAINT_KUNIT, LOCKDEP_STILL_OK); \ ^ 1 error generated.
vim +/TAINT_KUNIT +801 lib/test_printf.c
707cc7280f452a1 Rasmus Villemoes 2015-11-06 800 6b1a4d5b1a26ae8 Tobin C. Harding 2019-04-05 @801 KSTM_MODULE_LOADERS(test_printf);
Hi David,
I love your patch! Yet something to improve:
[auto build test ERROR on masahiroy-kbuild/for-next] [also build test ERROR on shuah-kselftest/next linus/master v5.19-rc4 next-20220701] [cannot apply to mcgrof/modules-next] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/David-Gow/panic-Taint-kernel-... base: https://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git for-next config: alpha-randconfig-r006-20220629 (https://download.01.org/0day-ci/archive/20220702/202207020132.SKDpQP9D-lkp@i...) compiler: alpha-linux-gcc (GCC) 11.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/intel-lab-lkp/linux/commit/42b6461d6cca4baeeeed474b1400e2... git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review David-Gow/panic-Taint-kernel-if-tests-are-run/20220701-164843 git checkout 42b6461d6cca4baeeeed474b1400e203057c2b9b # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=alpha SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable Reported-by: kernel test robot lkp@intel.com
All errors (new ones prefixed by >>):
In file included from lib/test_printf.c:27: lib/test_printf.c: In function 'test_printf_init':
lib/../tools/testing/selftests/kselftest_module.h:45:19: error: 'TAINT_KUNIT' undeclared (first use in this function)
45 | add_taint(TAINT_KUNIT, LOCKDEP_STILL_OK); \ | ^~~~~~~~~~~ lib/test_printf.c:801:1: note: in expansion of macro 'KSTM_MODULE_LOADERS' 801 | KSTM_MODULE_LOADERS(test_printf); | ^~~~~~~~~~~~~~~~~~~ lib/../tools/testing/selftests/kselftest_module.h:45:19: note: each undeclared identifier is reported only once for each function it appears in 45 | add_taint(TAINT_KUNIT, LOCKDEP_STILL_OK); \ | ^~~~~~~~~~~ lib/test_printf.c:801:1: note: in expansion of macro 'KSTM_MODULE_LOADERS' 801 | KSTM_MODULE_LOADERS(test_printf); | ^~~~~~~~~~~~~~~~~~~
vim +/TAINT_KUNIT +45 lib/../tools/testing/selftests/kselftest_module.h
40 41 #define KSTM_MODULE_LOADERS(__module) \ 42 static int __init __module##_init(void) \ 43 { \ 44 pr_info("loaded.\n"); \
45 add_taint(TAINT_KUNIT, LOCKDEP_STILL_OK); \
46 selftest(); \ 47 return kstm_report(total_tests, failed_tests, skipped_tests); \ 48 } \ 49 static void __exit __module##_exit(void) \ 50 { \ 51 pr_info("unloaded.\n"); \ 52 } \ 53 module_init(__module##_init); \ 54 module_exit(__module##_exit) 55
On Fri, Jul 01, 2022 at 04:47:44PM +0800, David Gow wrote:
Make any kselftest test module (using the kselftest_module framework) taint the kernel with TAINT_TEST on module load.
Note that several selftests use kernel modules which are not based on the kselftest_module framework, and so will not automatically taint the kernel.
This can be done in two ways:
- Moving the module to the tools/testing directory. All modules under this directory will taint the kernel.
- Adding the 'test' module property with: MODULE_INFO(test, "Y")
This just needs to be documented somewhere other than a commit log. Otherwise I am not sure how we can be sure it will catch on.
Similarly, selftests which do not load modules into the kernel generally should not taint the kernel (or possibly should only do so on failure), as it's assumed that testing from user-space should be safe. Regardless, they can write to /proc/sys/kernel/tainted if required.
Signed-off-by: David Gow davidgow@google.com
Looks good otherwise!
Reviewed-by: Luis Chamberlain mcgrof@kernel.org
Do we want this to go through selftest / kunit / modules tree? Happy for it to through any. I can't predict a conflict.
Luis
On Sat, Jul 2, 2022 at 6:33 AM Luis Chamberlain mcgrof@kernel.org wrote:
On Fri, Jul 01, 2022 at 04:47:44PM +0800, David Gow wrote:
Make any kselftest test module (using the kselftest_module framework) taint the kernel with TAINT_TEST on module load.
Note that several selftests use kernel modules which are not based on the kselftest_module framework, and so will not automatically taint the kernel.
This can be done in two ways:
- Moving the module to the tools/testing directory. All modules under this directory will taint the kernel.
- Adding the 'test' module property with: MODULE_INFO(test, "Y")
This just needs to be documented somewhere other than a commit log. Otherwise I am not sure how we can be sure it will catch on.
I've updated the kselftest documentation for v5.
Similarly, selftests which do not load modules into the kernel generally should not taint the kernel (or possibly should only do so on failure), as it's assumed that testing from user-space should be safe. Regardless, they can write to /proc/sys/kernel/tainted if required.
Signed-off-by: David Gow davidgow@google.com
Looks good otherwise!
Reviewed-by: Luis Chamberlain mcgrof@kernel.org
Do we want this to go through selftest / kunit / modules tree? Happy for it to through any. I can't predict a conflict.
I don't mind which tree it goes through either -- I'm not aware of anything which would depend on it. I do have it on the list of things pending for the KUnit tree, but it's much less KUnit-specific now compared to v1. Regardless, I'll leave in the KUnit to-do list, and we'll pick it up if no-one else particularly wants to.
Cheers, -- David
On Sat, Jul 2, 2022 at 12:06 PM David Gow davidgow@google.com wrote:
On Sat, Jul 2, 2022 at 6:33 AM Luis Chamberlain mcgrof@kernel.org wrote:
On Fri, Jul 01, 2022 at 04:47:44PM +0800, David Gow wrote:
Make any kselftest test module (using the kselftest_module framework) taint the kernel with TAINT_TEST on module load.
Note that several selftests use kernel modules which are not based on the kselftest_module framework, and so will not automatically taint the kernel.
This can be done in two ways:
- Moving the module to the tools/testing directory. All modules under this directory will taint the kernel.
- Adding the 'test' module property with: MODULE_INFO(test, "Y")
This just needs to be documented somewhere other than a commit log. Otherwise I am not sure how we can be sure it will catch on.
I've updated the kselftest documentation for v5.
Similarly, selftests which do not load modules into the kernel generally should not taint the kernel (or possibly should only do so on failure), as it's assumed that testing from user-space should be safe. Regardless, they can write to /proc/sys/kernel/tainted if required.
Signed-off-by: David Gow davidgow@google.com
Looks good otherwise!
Reviewed-by: Luis Chamberlain mcgrof@kernel.org
Do we want this to go through selftest / kunit / modules tree? Happy for it to through any. I can't predict a conflict.
I don't mind which tree it goes through either -- I'm not aware of anything which would depend on it. I do have it on the list of things pending for the KUnit tree, but it's much less KUnit-specific now compared to v1. Regardless, I'll leave in the KUnit to-do list, and we'll pick it up if no-one else particularly wants to.
FYI: It looks like patches 1 & 3 are already in the kunit tree, so it makes sense to take the rest of them, too: https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git/lo...
Cheers, -- David
linux-kselftest-mirror@lists.linaro.org
-
David Gow -
Greg KH -
kernel test robot -
Luis Chamberlain -
Maíra Canal