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 --- 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");
Reviewed-by: Luis Chamberlain mcgrof@kernel.org Signed-off-by: David Gow davidgow@google.com --- kernel/module/main.c | 7 +++++++ scripts/mod/modpost.c | 3 +++ 2 files changed, 10 insertions(+)
diff --git a/kernel/module/main.c b/kernel/module/main.c index fed58d30725d..730503561eb0 100644 --- a/kernel/module/main.c +++ b/kernel/module/main.c @@ -1988,6 +1988,13 @@ 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_once("%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 Sat, Jul 2, 2022 at 12:10 PM David Gow davidgow@google.com 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");
Reviewed-by: Luis Chamberlain mcgrof@kernel.org Signed-off-by: David Gow davidgow@google.com
I forgot the changelogs here. The only significant difference from v4 is the change from pr_warn() to pr_warn_once().
Changes since v4: https://lore.kernel.org/linux-kselftest/20220701084744.3002019-2-davidgow@go... - Use pr_warn_once() to only log a warning the first time a module taints the kernel with TAINT_TEST - Loading lots of test modules is a common usecase, and this would otherwise spam the logs too much. - Thanks Luis. - Remove a superfluous newline (Thanks Greg) - Add Luis' Reviewed-by tag.
This patch was new in v4 of the series.
On Sat 2022-07-02 12:09 +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");
Reviewed-by: Luis Chamberlain mcgrof@kernel.org Signed-off-by: David Gow davidgow@google.com
kernel/module/main.c | 7 +++++++ scripts/mod/modpost.c | 3 +++ 2 files changed, 10 insertions(+)
diff --git a/kernel/module/main.c b/kernel/module/main.c index fed58d30725d..730503561eb0 100644 --- a/kernel/module/main.c +++ b/kernel/module/main.c @@ -1988,6 +1988,13 @@ 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_once("%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) -- 2.37.0.rc0.161.g10f37bed90-goog
Hi David,
I think this looks good:
Reviewed-by: Aaron Tomlin atomlin@redhat.com
Kind regards,
On Sat, Jul 2, 2022 at 12:10 AM 'David Gow' via KUnit Development kunit-dev@googlegroups.com 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");
Reviewed-by: Luis Chamberlain mcgrof@kernel.org Signed-off-by: David Gow davidgow@google.com
Acked-by: Brendan Higgins brendanhiggins@google.com
On Sat, Jul 02, 2022 at 12:09:57PM +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");
Reviewed-by: Luis Chamberlain mcgrof@kernel.org Signed-off-by: David Gow davidgow@google.com
kernel/module/main.c | 7 +++++++ scripts/mod/modpost.c | 3 +++ 2 files changed, 10 insertions(+)
diff --git a/kernel/module/main.c b/kernel/module/main.c index fed58d30725d..730503561eb0 100644 --- a/kernel/module/main.c +++ b/kernel/module/main.c @@ -1988,6 +1988,13 @@ 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_once("%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) -- 2.37.0.rc0.161.g10f37bed90-goog
Hi David,
This change has landed in linux-next as commit e20729ede7ed ("module: panic: taint the kernel when selftest modules load") and on all of my test machines, I see this new message printed, even though as far as I am aware, I am not loading any testing modules. For example, in QEMU, I see:
[ 0.596978] serio: loading test module taints kernel.
and on my Honeycomb LX2, I see:
[ 5.400861] fuse: loading test module taints kernel.
It seems like the get_modinfo() check might be wrong? The following diff resolves it for me, I can send a formal patch if necessary (although it appears to have gone in via -mm so I assume Andrew can squash this in).
Cheers, Nathan
diff --git a/kernel/module/main.c b/kernel/module/main.c index 730503561eb0..4f91e41b8bc9 100644 --- a/kernel/module/main.c +++ b/kernel/module/main.c @@ -1988,7 +1988,7 @@ 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 (get_modinfo(info, "test")) { if (!test_taint(TAINT_TEST)) pr_warn_once("%s: loading test module taints kernel.\n", mod->name);
On Fri, Jul 8, 2022 at 8:40 AM Nathan Chancellor nathan@kernel.org wrote:
On Sat, Jul 02, 2022 at 12:09:57PM +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");
Reviewed-by: Luis Chamberlain mcgrof@kernel.org Signed-off-by: David Gow davidgow@google.com
kernel/module/main.c | 7 +++++++ scripts/mod/modpost.c | 3 +++ 2 files changed, 10 insertions(+)
diff --git a/kernel/module/main.c b/kernel/module/main.c index fed58d30725d..730503561eb0 100644 --- a/kernel/module/main.c +++ b/kernel/module/main.c @@ -1988,6 +1988,13 @@ 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_once("%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)
2.37.0.rc0.161.g10f37bed90-goog
Hi David,
This change has landed in linux-next as commit e20729ede7ed ("module: panic: taint the kernel when selftest modules load") and on all of my test machines, I see this new message printed, even though as far as I am aware, I am not loading any testing modules. For example, in QEMU, I see:
[ 0.596978] serio: loading test module taints kernel.
and on my Honeycomb LX2, I see:
[ 5.400861] fuse: loading test module taints kernel.
It seems like the get_modinfo() check might be wrong? The following diff resolves it for me, I can send a formal patch if necessary (although it appears to have gone in via -mm so I assume Andrew can squash this in).
Whoops: this is definitely the wrong way round. Thanks very much for catching it! (I'd swapped it locally at some point to test, and must've accidentally committed it.)
I've sent out v6 with this fixed (and a couple of other minor changes): https://lore.kernel.org/linux-kselftest/20220708044847.531566-2-davidgow@goo...
That being said, if just squashing your change below in is easier, I'm fine with that, too. (The other changes are minor enough that we could live without them and/or send them in separately.)
Unless there are any objections, should Andrew update this patch in his tree, and we remove the series from the kunit tree?
Cheers, -- David
diff --git a/kernel/module/main.c b/kernel/module/main.c index 730503561eb0..4f91e41b8bc9 100644 --- a/kernel/module/main.c +++ b/kernel/module/main.c @@ -1988,7 +1988,7 @@ 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 (get_modinfo(info, "test")) { if (!test_taint(TAINT_TEST)) pr_warn_once("%s: loading test module taints kernel.\n", mod->name);
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 --- 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.
Also mark the module as a test module using MODULE_INFO(test, "Y") so that other tools can tell this is a test module. We can't rely solely on this, though, as these test modules are also often built-in.
Finally, update the kselftest documentation to mention that the kernel should be tainted, and how to do so manually (as below).
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.
Reviewed-by: Luis Chamberlain mcgrof@kernel.org Signed-off-by: David Gow davidgow@google.com --- Documentation/dev-tools/kselftest.rst | 9 +++++++++ tools/testing/selftests/kselftest_module.h | 4 ++++ 2 files changed, 13 insertions(+)
diff --git a/Documentation/dev-tools/kselftest.rst b/Documentation/dev-tools/kselftest.rst index a833ecf12fbc..1096a9833550 100644 --- a/Documentation/dev-tools/kselftest.rst +++ b/Documentation/dev-tools/kselftest.rst @@ -250,6 +250,14 @@ assist writing kernel modules that are for use with kselftest: - ``tools/testing/selftests/kselftest_module.h`` - ``tools/testing/selftests/kselftest/module.sh``
+Note that test modules should taint the kernel with TAINT_TEST. This will +happen automatically for modules which are in the ``tools/testing/`` +directory, or for modules which use the ``kselftest_module.h`` header above. +Otherwise, you'll need to add ``MODULE_INFO(test, "Y")`` to your module +source. selftests which do not load modules typically should not taint the +kernel, but in cases where a non-test module is loaded, TEST_TAINT can be +applied from userspace by writing to ``/proc/sys/kernel/tainted``. + How to use ----------
@@ -308,6 +316,7 @@ A bare bones test module might look like this: KSTM_MODULE_LOADERS(test_foo); MODULE_AUTHOR("John Developer jd@fooman.org"); MODULE_LICENSE("GPL"); + MODULE_INFO(test, "Y");
Example test script ------------------- diff --git a/tools/testing/selftests/kselftest_module.h b/tools/testing/selftests/kselftest_module.h index e2ea41de3f35..63cd7487373f 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_TEST, LOCKDEP_STILL_OK); \ selftest(); \ return kstm_report(total_tests, failed_tests, skipped_tests); \ } \ @@ -51,4 +53,6 @@ static void __exit __module##_exit(void) \ module_init(__module##_init); \ module_exit(__module##_exit)
+MODULE_INFO(test, "Y"); + #endif /* __KSELFTEST_MODULE_H */
On Sat, Jul 2, 2022 at 12:10 PM David Gow davidgow@google.com wrote:
Make any kselftest test module (using the kselftest_module framework) taint the kernel with TAINT_TEST on module load.
Also mark the module as a test module using MODULE_INFO(test, "Y") so that other tools can tell this is a test module. We can't rely solely on this, though, as these test modules are also often built-in.
Finally, update the kselftest documentation to mention that the kernel should be tainted, and how to do so manually (as below).
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.
Reviewed-by: Luis Chamberlain mcgrof@kernel.org Signed-off-by: David Gow davidgow@google.com
Whoops: forgot the changelogs. Only patches 2 and 4 had changes. For this patch:
Changes since v4: https://lore.kernel.org/lkml/20220513083212.3537869-3-davidgow@google.com/ - Actually use the new TAINT_TEST name, instead of TAINT_KUNIT (Thanks, kernel-test-robot) - Document how to use this (or MODULE_INFO()) to taint the kernel. (Thanks, Luis) - Also add MODULE_INFO(test, "Y") to embed the fact that this is a test module into the .ko - Nothing depends on it now, but it should allow us to tell this is a test module without executing it in the future.
No changes since v3: https://lore.kernel.org/lkml/20220513083212.3537869-3-davidgow@google.com/
On Sat, Jul 2, 2022 at 12:10 AM David Gow davidgow@google.com wrote:
Make any kselftest test module (using the kselftest_module framework) taint the kernel with TAINT_TEST on module load.
Also mark the module as a test module using MODULE_INFO(test, "Y") so that other tools can tell this is a test module. We can't rely solely on this, though, as these test modules are also often built-in.
Finally, update the kselftest documentation to mention that the kernel should be tainted, and how to do so manually (as below).
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.
Reviewed-by: Luis Chamberlain mcgrof@kernel.org Signed-off-by: David Gow davidgow@google.com
Acked-by: Brendan Higgins brendanhiggins@google.com
linux-kselftest-mirror@lists.linaro.org
-
Aaron Tomlin -
Brendan Higgins -
David Gow -
Nathan Chancellor