| Message ID | 20220429043913.626647-1-davidgow@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | kunit: Taint kernel if any tests run | expand |
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@google.com> > --- > > 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
On Fri, 29 Apr 2022, Greg KH <gregkh@linuxfoundation.org> wrote: > 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@google.com> >> --- >> >> 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. Would you consider a patch adding more self-explanatory taint flag strings to the output? BR, Jani.
On Fri, Apr 29, 2022 at 02:21:26PM +0300, Jani Nikula wrote: > On Fri, 29 Apr 2022, Greg KH <gregkh@linuxfoundation.org> wrote: > > 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@google.com> > >> --- > >> > >> 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. > > Would you consider a patch adding more self-explanatory taint flag > strings to the output? Where would those strings go? In the oops report? Or somewhere else? thanks, greg k-h
On Fri, 29 Apr 2022, Greg KH <gregkh@linuxfoundation.org> wrote: > On Fri, Apr 29, 2022 at 02:21:26PM +0300, Jani Nikula wrote: >> On Fri, 29 Apr 2022, Greg KH <gregkh@linuxfoundation.org> wrote: >> > 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@google.com> >> >> --- >> >> >> >> 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. >> >> Would you consider a patch adding more self-explanatory taint flag >> strings to the output? > > Where would those strings go? In the oops report? Or somewhere else? I was thinking the oops report. Basically most times I look at an oops with taint, I need to double check what the flags mean. There are soon 19 of them, you need to look at a lot of oops to remember them all. Currently we also print ' ' (or 'G' in case of non-properietary module) for every unset taint flag. If we stopped doing that we wouldn't even need that much more horizontal space for the strings, unless several flags were set. (I assume people who do remember all the flags by heart would still want to keep them too.) BR, Jani. > > thanks, > > greg k-h
On Fri, Apr 29, 2022 at 02:54:25PM +0300, Jani Nikula wrote: > On Fri, 29 Apr 2022, Greg KH <gregkh@linuxfoundation.org> wrote: > > On Fri, Apr 29, 2022 at 02:21:26PM +0300, Jani Nikula wrote: > >> On Fri, 29 Apr 2022, Greg KH <gregkh@linuxfoundation.org> wrote: > >> > 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@google.com> > >> >> --- > >> >> > >> >> 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. > >> > >> Would you consider a patch adding more self-explanatory taint flag > >> strings to the output? > > > > Where would those strings go? In the oops report? Or somewhere else? > > I was thinking the oops report. Basically most times I look at an oops > with taint, I need to double check what the flags mean. There are soon > 19 of them, you need to look at a lot of oops to remember them all. I agree, it isn't easy to remember. > Currently we also print ' ' (or 'G' in case of non-properietary module) > for every unset taint flag. If we stopped doing that we wouldn't even > need that much more horizontal space for the strings, unless several > flags were set. (I assume people who do remember all the flags by heart > would still want to keep them too.) I recommend keeping the current layout, but maybe adding a new line that gives the "key" for what the current taint flags mean? For example, the oops report here: https://lore.kernel.org/r/20220413033425.GM16799@magnolia Has the lines: kernel BUG at mm/filemap.c:1653! invalid opcode: 0000 [#1] PREEMPT SMP CPU: 0 PID: 1349866 Comm: 0:116 Tainted: G W 5.18.0-rc2-djwx #rc2 19cc48221d47ada6c8e5859639b6a0946c9a3777 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS ?-20171121_152543-x86-ol7-builder-01.us.oracle.com-4.el7.1 04/01/2014 Workqueue: xfs-conv/sda4 xfs_end_io [xfs] RIP: 0010:folio_end_writeback+0x79/0x80 Perhaps we add another line right before or after "Hardware name:" that lists the flags that are set at the moment and what they mean: Taint flags: [G]=PROPRIETARY_MODULE, [W]=WARN Or something like that (format was a first guess only). Anyway, might be helpful? thanks, greg k-h
On Fri, Apr 29, 2022 at 3:09 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > 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@google.com> < snip > > > + [ TAINT_KUNIT ] = { 'N', ' ', false }, > > As kunit tests can be in modules, shouldn't this be "true" here? Ah, good catch. While I tend to use either built-in tests (or modules which are immediately unloaded), there are definitely some cases where the tests are part of long-lasting modules. I'll send out v2 with that changed. > 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. Yeah, 'T' would've been nice, but I doubt it'd be worth trying to change it now. At least we haven't had to resort to emoji... Adding an actual name as Jani suggested would be a good idea, IMHO, though obviously best done in a separate patch. Cheers, -- David
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 }, }; /** diff --git a/lib/kunit/test.c b/lib/kunit/test.c index 0f66c13d126e..ea8e9162445d 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> @@ -498,6 +499,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_KUNIT, LOCKDEP_STILL_OK); + kunit_print_subtest_start(suite); kunit_suite_for_each_test_case(suite, test_case) {
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@google.com> --- 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(-)