Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
MIME-Version: 1.0
References: <20220429043913.626647-1-davidgow@google.com> <20220513083212.3537869-2-davidgow@google.com>
 <CAGS_qxr54nYThsj6UhqX54JO5WnyJXVQURnNF1eCzGB+4GCKLA@mail.gmail.com>
In-Reply-To: <CAGS_qxr54nYThsj6UhqX54JO5WnyJXVQURnNF1eCzGB+4GCKLA@mail.gmail.com>
From:   David Gow <davidgow@google.com>
Date:   Sat, 14 May 2022 11:04:27 +0800
Message-ID: <CABVgOS=gTznLFBTZbmNH7AFDnr7O70mWR9v4q6sDA7q04fKT=Q@mail.gmail.com>
Subject: Re: [PATCH v3 2/3] kunit: Taint the kernel when KUnit tests are run
To:     Daniel Latypov <dlatypov@google.com>
Cc:     Brendan Higgins <brendanhiggins@google.com>,
        Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
        Jonathan Corbet <corbet@lwn.net>,
        Andrew Morton <akpm@linux-foundation.org>,
        Kees Cook <keescook@chromium.org>,
        Shuah Khan <skhan@linuxfoundation.org>,
        Greg KH <gregkh@linuxfoundation.org>,
        Luis Chamberlain <mcgrof@kernel.org>,
        "Guilherme G . Piccoli" <gpiccoli@igalia.com>,
        Sebastian Reichel <sre@kernel.org>,
        John Ogness <john.ogness@linutronix.de>,
        Joe Fradley <joefradley@google.com>,
        KUnit Development <kunit-dev@googlegroups.com>,
        "open list:KERNEL SELFTEST FRAMEWORK" 
        <linux-kselftest@vger.kernel.org>,
        "open list:DOCUMENTATION" <linux-doc@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Jani Nikula <jani.nikula@linux.intel.com>,
        Lucas De Marchi <lucas.demarchi@intel.com>,
        Aaron Tomlin <atomlin@redhat.com>,
        linux-fsdevel@vger.kernel.org, linux-block@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

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