On Fri, Apr 29, 2022 at 12:39:14PM +0800, David Gow wrote:
> KUnit tests are not supposed to run on production systems: they may do
> deliberately illegal things to trigger errors, and have security
> implications (assertions will often deliberately leak kernel addresses).
>
> Add a new taint type, TAINT_KUNIT to signal that a KUnit test has been
> run. This will be printed as 'N' (for kuNit, as K, U and T were already
> taken).
>
> This should discourage people from running KUnit 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@xxxxxxxxxx>
> ---
>
> This is something I'd been thinking about for a while, and it came up
> again, so I'm finally giving it a go.
>
> Two notes:
> - I decided to add a new type of taint, as none of the existing ones
> really seemed to fit. We could live with considering KUnit tests as
> TAINT_WARN or TAINT_CRAP or something otherwise, but neither are quite
> right.
> - The taint_flags table gives a couple of checkpatch.pl errors around
> bracket placement. I've kept the new entry consistent with what's
> there rather than reformatting the whole table, but be prepared for
> complaints about spaces.
>
> Thoughts?
> -- David
>
> ---
> Documentation/admin-guide/tainted-kernels.rst | 1 +
> include/linux/panic.h | 3 ++-
> kernel/panic.c | 1 +
> lib/kunit/test.c | 4 ++++
> 4 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/admin-guide/tainted-kernels.rst b/Documentation/admin-guide/tainted-kernels.rst
> index ceeed7b0798d..8f18fc4659d4 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 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..1d316c26bf27 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_KUNIT 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..b24ca63ed738 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_KUNIT ] = { 'N', ' ', false },
As kunit tests can be in modules, shouldn't this be "true" here?
Overall, I like it, makes sense to me. The "N" will take some getting
used to, and I have no idea why "T" was for "struct randomization", that
would have allowed you to use "T" instead. Oh well.
thanks,
greg k-h
