Re: [PATCH v3 2/3] kunit: Taint the kernel when KUnit tests are run

[David Gow <davidgow@google.com>]

On Sat, May 14, 2022 at 3:09 AM Daniel Latypov <dlatypov@google.com> wrote:
>
> On Fri, May 13, 2022 at 1:32 AM David Gow <davidgow@google.com> 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.
> >
> > Signed-off-by: David Gow <davidgow@google.com>
>
> Tested-by: Daniel Latypov <dlatypov@google.com>
>
> Looks good to me.
>
> There's an edge case where we might have 0 suites or 0 tests and we
> still taint the kernel, but I don't think we need to deal with that.
> At the start of kunit_run_tests() is the cleanest place to do this.

Hmm... thinking about it, I think it might be worth not tainting if 0
suites run, but tainting if 0 tests run.

If we taint even if there are no suites present, that'll make things
awkward for the "build KUnit in, but not any tests" case: the kernel
would be tainted regardless. Given Android might be having the KUnit
execution stuff built-in (but using modules for tests), it's probably
worth not tainting there. (Though I think they have a separate way of
disabling KUnit as well, so it's probably not a complete
deal-breaker).

The case of having suites but no tests should still taint the kernel,
as suite_init functions could still run.

Assuming that seems sensible, I'll send out a v4 with that changed.

> I wasn't quite sure where this applied, but I manually applied the changes here.
> Without this patch, this command exits fine:
> $ ./tools/testing/kunit/kunit.py run --kernel_args=panic_on_taint=0x40000
>
> With it, I get
> [12:03:31] Kernel panic - not syncing: panic_on_taint set ...
> [12:03:31] CPU: 0 PID: 1 Comm: swapper Tainted: G                 N

This is showing both 'G' and 'N' ('G' being the character for GPL --
i.e. the kernel is not tainted by proprietary modules: 'P').

Jani did suggest a better way of printing these in the v1 discussion
(printing the actual names of taints present), which I might do in a
follow-up.

> 5.17.0-00001-gea9ee5e7aed8-dirty #60
>
> I'm a bit surprised that it prints 'G' and not 'N', but this does seem
> to be the right mask
> $ python3 -c 'print(hex(1<<18))'
> 0x40000
> and it only takes effect when this patch is applied.
> I'll chalk that up to my ignorance of how taint works.

-- David