| Message ID | 20220513083212.3537869-1-davidgow@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v3,1/3] panic: Taint kernel if tests are run | expand |
On Fri, May 13, 2022 at 04:32:11PM +0800, David Gow wrote: > 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.) > > Signed-off-by: David Gow <davidgow@google.com> > --- > > Updated this to handle the most common case of selftest modules, in > addition to KUnit tests. There's room for other tests or test frameworks > to use this as well, either with a call to add_taint() from within the > kernel, or by writing to /proc/sys/kernel/tainted. > > 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 v2: > https://lore.kernel.org/linux-kselftest/20220430030019.803481-1-davidgow@google.com/ > - 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@google.com/ > - 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..546f3071940d 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 (such as a KUnit test) has been run I think mentioning just kunit fuzzes its interpretation here. Best to keep that out. Other than that: Acked-by: Luis Chamberlain <mcgrof@kernel.org> Luis
On Fri, May 13, 2022 at 4:32 AM David Gow <davidgow@google.com> wrote: > > 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.) > > Signed-off-by: David Gow <davidgow@google.com> Aside from Luis' comment (which I agree with), this looks good. I am not an expert on the taint mechanism, but this seems pretty straightforward. Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
diff --git a/Documentation/admin-guide/tainted-kernels.rst b/Documentation/admin-guide/tainted-kernels.rst index ceeed7b0798d..546f3071940d 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 (such as a KUnit 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 f5844908a089..2f5f2a9ecaf7 100644 --- a/include/linux/panic.h +++ b/include/linux/panic.h @@ -74,7 +74,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 eb4dfb932c85..1cf707e3bacd 100644 --- a/kernel/panic.c +++ b/kernel/panic.c @@ -404,6 +404,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 }, }; /**
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.) Signed-off-by: David Gow <davidgow@google.com> --- Updated this to handle the most common case of selftest modules, in addition to KUnit tests. There's room for other tests or test frameworks to use this as well, either with a call to add_taint() from within the kernel, or by writing to /proc/sys/kernel/tainted. 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 v2: https://lore.kernel.org/linux-kselftest/20220430030019.803481-1-davidgow@google.com/ - 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@google.com/ - 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(-)