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

[David Gow]

On Thu, May 5, 2022 at 1:57 PM Luis Chamberlain <mcgrof@xxxxxxxxxx> 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@xxxxxxxxxx> 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.

I _suspect_ we'd be able to hit most of them by tainting in frameworks
like the above, and patch the remaining modules manually. 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.

(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.)

-- David

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature