linux-kernel - Re: [PATCH v2] kunit: Taint kernel if any tests run

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Ynl8/MqmDZzpbh5y@bombadil.infradead.org>
Date:   Mon, 9 May 2022 13:43:40 -0700
From:   Luis Chamberlain <mcgrof@...nel.org>
To:     David Gow <davidgow@...gle.com>
Cc:     Daniel Latypov <dlatypov@...gle.com>,
        Shuah Khan <skhan@...uxfoundation.org>,
        Lucas De Marchi <lucas.demarchi@...el.com>,
        Aaron Tomlin <atomlin@...hat.com>,
        linux-fsdevel@...r.kernel.org, linux-block@...r.kernel.org,
        Brendan Higgins <brendanhiggins@...gle.com>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Jonathan Corbet <corbet@....net>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Kees Cook <keescook@...omium.org>,
        Greg KH <gregkh@...uxfoundation.org>,
        "Guilherme G . Piccoli" <gpiccoli@...lia.com>,
        Sebastian Reichel <sre@...nel.org>,
        John Ogness <john.ogness@...utronix.de>,
        Joe Fradley <joefradley@...gle.com>,
        KUnit Development <kunit-dev@...glegroups.com>,
        "open list:KERNEL SELFTEST FRAMEWORK" 
        <linux-kselftest@...r.kernel.org>,
        "open list:DOCUMENTATION" <linux-doc@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Jani Nikula <jani.nikula@...ux.intel.com>
Subject: Re: [PATCH v2] kunit: Taint kernel if any tests run

On Fri, May 06, 2022 at 03:01:34PM +0800, David Gow wrote:
> On Thu, May 5, 2022 at 1:57 PM Luis Chamberlain <mcgrof@...nel.org> wrote:
> >
> > On Wed, May 04, 2022 at 02:12:30PM -0700, Luis Chamberlain wrote:
> > > On Wed, May 04, 2022 at 02:19:59PM -0500, Daniel Latypov wrote:
> > > > On Wed, May 4, 2022 at 1:46 PM Luis Chamberlain <mcgrof@...nel.org> wrote:
> > > > > OK so, we can just skip tainting considerations for selftests which
> > > > > don't use modules for now. There may be selftests which do wonky
> > > > > things in userspace but indeed I agree the userspace taint would
> > > > > be better for those but I don't think it may be worth bother
> > > > > worrying about those at this point in time.
> > > > >
> > > > > But my point in that sharing a taint between kunit / selftests modules
> > > > > does make sense and is easily possible. The unfortunate aspect is just
> > > >
> > > > Yes, I 100% agree that we should share a taint for kernelspace testing
> > > > from both kunit/kselftest.
> > > > Someone running the system won't care what framework was used.
> > >
> > > OK do you mind doing the nasty work of manually adding the new
> > > MODULE_TAINT() to the selftests as part of your effort?
> > >
> > > *Alternatively*, if we *moved* all sefltests modules to a new
> > > lib/debug/selftests/ directory or something like that then t would
> > > seem modpost *could* add the taint flag automagically for us without
> > > having to edit or require it on new drivers. We have similar type of
> > > taint for staging, see add_staging_flag().
> > >
> > > I would *highly* prefer this approach, event though it is more work,
> > > because I think this is a step we should take anyway.
> > >
> > > However, I just checked modules on lib/ and well, some of them are
> > > already in their own directory, like lib/math/test_div64.c. So not
> > > sure, maybe just move a few modules which are just in lib/*.c for now
> > > and then just sprinkle the MODULE_TAINT() to the others?
> >
> > I *think* we could just pull this off with a much easier approach,
> > simply looking for the substrings in the module name in modpost.c:
> >
> >   * "_test." || "-test."
> >   * ^"test_" || ^"test-"
> >
> > An issue with this of course is a vendor $FOO with an out of tree
> > test driver may end up with the taint. Perhaps we don't care.
> >
> > That means moving selftests to its own directory is not needed at this
> > point in time.
> 
> I can't say I'm thrilled with the idea of just doing name comparisons,
> particularly since not all of them match this pattern, for example:
> bpf_testmod.ko. (Though, frankly, more of them do than I'd've
> guessed.)
> 
> Maybe adding a taint call to the selftest helper module framework in
> kselftest_module.h, though again, there are several tests which don't
> use it.

Right, I can't think of a generic way to peg this. I think long term
we do stand to gain to move all selftests under a lib/debug/selftests/
or something like that, but for now what I suggested is the only thing
I can come up with.

> I _suspect_ we'd be able to hit most of them by tainting in frameworks
> like the above, and patch the remaining modules manually.

Works with me.

> There's also
> definitely a grey area with things like netdevsim, which are used a
> lot as helper modules by selftests, but may have other uses as well.

They can peg the module if they want the taint.

> (The advantage of the KUnit tainting is that, due to KUnit's
> centralised executor, we can be sure all KUnit tests will correctly
> trigger the taint. But maybe it doesn't matter as much if one or two
> selftests miss out.)

That is what I was thinking.

I'm convinced we *should* move selftests to a one directory. The
amount of stuff in lib/ is getting out of hand.

  Luis