Most in-kernel tests (such as KUnit tests) are not supposed to run on
production systems: they may do deliberately illegal things to trigger
errors, and have security implications (for example, KUnit assertions
will often deliberately leak kernel addresses).
Add a new taint type, TAINT_TEST to signal that a test has been run.
This will be printed as 'N' (originally for kuNit, as every other
sensible letter was taken.)
This should discourage people from running these tests on production
systems, and to make it easier to tell if tests have been run
accidentally (by loading the wrong configuration, etc.)
Acked-by: Luis Chamberlain <mcgrof(a)kernel.org>
Reviewed-by: Brendan Higgins <brendanhiggins(a)google.com>
Signed-off-by: David Gow <davidgow(a)google.com>
---
This is v6 of the "make tests taint the kernel" patchset. The only
changes since v5 (which is the version in linux-next at time of writing)
are some rather critical fixes to patch 2/4, where the cruicial check
was inverted. (Oops!)
The 'N' character for the taint is even less useful now that it's no
longer short for kuNit, but all the letters in TEST are taken. :-(
No changes since v5:
https://lore.kernel.org/linux-kselftest/20220702040959.3232874-1-davidgow@g…
No changes since v4:
https://lore.kernel.org/linux-kselftest/20220701084744.3002019-1-davidgow@g…
Changes since v3:
https://lore.kernel.org/lkml/20220513083212.3537869-1-davidgow@google.com/
- Remove the mention of KUnit from the documentation.
- Add Luis and Brendan's Acked/Reviewed-by tags.
Changes since v2:
https://lore.kernel.org/linux-kselftest/20220430030019.803481-1-davidgow@go…
- Rename TAINT_KUNIT -> TAINT_TEST.
- Split into separate patches for adding the taint, and triggering it.
- Taint on a kselftest_module being loaded (patch 3/3)
Changes since v1:
https://lore.kernel.org/linux-kselftest/20220429043913.626647-1-davidgow@go…
- Make the taint per-module, to handle the case when tests are in
(longer lasting) modules. (Thanks Greg KH).
Note that this still has checkpatch.pl warnings around bracket
placement, which are intentional as part of matching the surrounding
code.
---
Documentation/admin-guide/tainted-kernels.rst | 1 +
include/linux/panic.h | 3 ++-
kernel/panic.c | 1 +
3 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/Documentation/admin-guide/tainted-kernels.rst b/Documentation/admin-guide/tainted-kernels.rst
index ceeed7b0798d..7d80e8c307d1 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 an in-kernel 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 e71161da69c4..c7759b3f2045 100644
--- a/include/linux/panic.h
+++ b/include/linux/panic.h
@@ -68,7 +68,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_TEST 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 a3c758dba15a..6b3369e21026 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -428,6 +428,7 @@ const struct taint_flag taint_flags[TAINT_FLAGS_COUNT] = {
[ TAINT_LIVEPATCH ] = { 'K', ' ', true },
[ TAINT_AUX ] = { 'X', ' ', true },
[ TAINT_RANDSTRUCT ] = { 'T', ' ', true },
+ [ TAINT_TEST ] = { 'N', ' ', true },
};
/**
--
2.37.0.rc0.161.g10f37bed90-goog
Accidentally hit direct reply, adding Shuah Khan <shuah(a)kernel.org>,
linux-kernel(a)vger.kernel.org, linux-fsdevel(a)vger.kernel.org,
linux-kselftest(a)vger.kernel.org, Shuah Khan
<skhan(a)linuxfoundation.org>
On Mon, Jul 11, 2022 at 4:04 PM Dylan Hatch <dylanbhatch(a)google.com> wrote:
>
> On Wed, Jun 22, 2022 at 10:15 AM Shuah Khan <skhan(a)linuxfoundation.org> wrote:
> >
> > On 6/21/22 6:18 PM, Dylan Hatch wrote:
> > > On Fri, Jun 17, 2022 at 3:27 PM Shuah Khan <skhan(a)linuxfoundation.org> wrote:
> > >>
> > >> On 6/17/22 4:05 PM, Dylan Hatch wrote:
> > >>> On Fri, Jun 17, 2022 at 12:38 PM Shuah Khan <skhan(a)linuxfoundation.org> wrote:
> > >>>>
> > >>>> On 6/17/22 12:45 PM, Dylan Hatch wrote:
> > >>>>> On Thu, Jun 16, 2022 at 4:01 PM Shuah Khan <skhan(a)linuxfoundation.org> wrote:
> > >>>>>>
> > >>>
> > >>>>
> > >>>> It depends on the goal of the test. Is the test looking to see if the
> > >>>> probe fails with insufficient permissions, then you are changing the
> > >>>> test to not check for that condition.
> > >>>
> > >>> The goal of the test is to validate the output of /proc/$PID/maps, and
> > >>> the memory probe is only needed as setup to determine what the
> > >>> expected output should be. This used to be sufficient, but now it can
> > >>> no longer fully disambiguate it with the introduction of
> > >>> vsyscall=xonly. The solution proposed here is to disambiguate it by
> > >>> also checking the length read from /proc/$PID/maps.
> > >>>
> > >>>>
> > >>
> > >> Makes sense. However the question is does this test need to be enhanced
> > >> with the addition of vsyscall=xonly?
> > >>
> > >>>> I would say in this case, the right approach would be to leave the test
> > >>>> as is and report expected fail and add other cases.
> > >>>>
> > >>>> The goal being adding more coverage and not necessarily opt for a simple
> > >>>> solution.
> > >>>
> > >>> What does it mean to report a test as expected fail? Is this a
> > >>> mechanism unique to kselftest? I agree adding another test case would
> > >>> work, but I'm unsure how to do it within the framework of kselftest.
> > >>> Ideally, there would be separate test cases for vsyscall=none,
> > >>> vsyscall=emulate, and vsyscall=xonly, but these options can be toggled
> > >>> both in the kernel config and on the kernel command line, meaning (to
> > >>> the best of my knowledge) these test cases would have to be built
> > >>> conditionally against the conflig options and also parse the command
> > >>> line for the 'vsyscall' option.
> > >>>
> > >>
> > >> Expected fail isn't unique kselftest. It is a testing criteria where
> > >> a test is expected to fail. For example if a file can only be opened
> > >> with privileged user a test that runs and looks for failure is an
> > >> expected to fail case - we are looking for a failure.
> > >>
> > >> A complete battery of tests for vsyscall=none, vsyscall=emulate,
> > >> vsyscall=xonly would test for conditions that are expected to pass
> > >> and fail based on the config.
> > >>
> > >> tools/testing/selftests/proc/config doesn't have any config options
> > >> that are relevant to VSYSCALL
> > >>
> > >> Can you please send me the how you are running the test and what the
> > >> failure output looks like?
> > >
> > > I'm building a kernel with the following relevant configurations:
> > >
> > > $ cat .config | grep VSYSCALL
> > > CONFIG_GENERIC_TIME_VSYSCALL=y
> > > CONFIG_X86_VSYSCALL_EMULATION=y
> > > CONFIG_LEGACY_VSYSCALL_XONLY=y
> > > # CONFIG_LEGACY_VSYSCALL_NONE is not set
> > >
> > > Running the test without this change both in virtme and on real
> > > hardware gives the following error:
> > >
> > > # ./tools/testing/selftests/proc/proc-pid-vm
> > > proc-pid-vm: proc-pid-vm.c:328: int main(void): Assertion `rv == len' failed.
> > > Aborted
> > >
> > > This is because when CONFIG_LEGACY_VSYSCALL_XONLY=y a probe of the
> > > vsyscall page results in a segfault. This test was originally written
> > > before this option existed so it incorrectly assumes the vsyscall page
> > > isn't mapped at all, and the expected buffer length doesn't match the
> > > result.
> > >
> > > An alternate method of fixing this test could involve setting the
> > > expected result based on the config with #ifdef blocks, but I wasn't
> > > sure if that could be done for kernel config options in kselftest
> > > code. There's also the matter of checking the kernel command line for
> > > a `vsyscall=` arg, is parsing /proc/cmdline the best way to do this?
> > >
> >
> > We have a few tests do ifdef to be able to test the code as well as deal
> > with config specific tests. Not an issue.
> >
> > Parsing /proc/cmdline line is flexible for sure, if you want to use that
> > route.
> >
> > Thank you for finding the problem and identifying missing coverage. Look
> > forward to any patches fixing the problem.
> >
> > thanks,
> > -- Shuah
>
I've done some experimenting with ifdefs on config options, but it
seems that these options do not propagate properly into the tests. Is
there a specific method I should be using to propagate the config
values, or would you be able to point me to an example where this is
done properly?
Thanks and sorry for the slow reply on this,
Dylan
From: David Gow <davidgow(a)google.com>
There are several tests which depend on PCI, and hence need a bunch of
extra options to run under UML. This makes it awkward to give
configuration instructions (whether in documentation, or as part of a
.kunitconfig file), as two separate, incompatible sets of config options
are required for UML and "most other architectures".
For non-UML architectures, it's possible to add default kconfig options
via the qemu_config python files, but there's no equivalent for UML. Add
a new tools/testing/kunit/configs/arch_uml.config file containing extra
kconfig options to use on UML.
Tested-by: José Expósito <jose.exposito89(a)gmail.com>
Reviewed-by: Daniel Latypov <dlatypov(a)google.com>
Signed-off-by: David Gow <davidgow(a)google.com>
Reviewed-by: Brendan Higgins <brendanhiggins(a)google.com>
Signed-off-by: Daniel Latypov <dlatypov(a)google.com>
---
NOTE: This depends on v4 of the repeatable --kunitconfig patch here:
https://patchwork.kernel.org/project/linux-kselftest/patch/20220708013632.1…
Please apply it first first.
Changes since v2: (dlatypov(a)google.com)
- Rebase on top of the -kselftest kunit branch + v4 of the --kunitconfig
patch. It rebased cleanly, but it evidently would not apply cleanly
due to all the conflicts v4 --kunitconfig had with --qemu_args
Changes since v1:
https://lore.kernel.org/linux-kselftest/20220624084400.1454579-1-davidgow@g…
- (Hopefully) fix a pytype warning re: architecture being None in the
tests. (Thanks, Daniel)
- Rebase on top of the new combined v3 of the kconfig/kunitconfig
patchset.
- Add José's Tested-by and Daniel's Reviewed-by.
Changes since RFC:
https://lore.kernel.org/linux-kselftest/20220622035326.759935-1-davidgow@go…
- Rebase on top of the previous kconfig patches.
- Fix a missing make_arch_qemuconfig->make_arch_config rename (Thanks
Brendan)
- Fix the tests to use the base LinuxSourceTreeOperations class, which
has no default kconfig options (and so won't conflict with those set
in the tests). Only test_build_reconfig_existing_config actually
failed, but I updated a few more in case the defaults changed.
---
tools/testing/kunit/configs/arch_uml.config | 5 +++++
tools/testing/kunit/kunit_kernel.py | 14 ++++++++++----
tools/testing/kunit/kunit_tool_test.py | 12 ++++++++++++
3 files changed, 27 insertions(+), 4 deletions(-)
create mode 100644 tools/testing/kunit/configs/arch_uml.config
diff --git a/tools/testing/kunit/configs/arch_uml.config b/tools/testing/kunit/configs/arch_uml.config
new file mode 100644
index 000000000000..e824ce43b05a
--- /dev/null
+++ b/tools/testing/kunit/configs/arch_uml.config
@@ -0,0 +1,5 @@
+# Config options which are added to UML builds by default
+
+# Enable virtio/pci, as a lot of tests require it.
+CONFIG_VIRTIO_UML=y
+CONFIG_UML_PCI_OVER_VIRTIO=y
diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py
index 56492090e28e..f5c26ea89714 100644
--- a/tools/testing/kunit/kunit_kernel.py
+++ b/tools/testing/kunit/kunit_kernel.py
@@ -26,6 +26,7 @@ KUNITCONFIG_PATH = '.kunitconfig'
OLD_KUNITCONFIG_PATH = 'last_used_kunitconfig'
DEFAULT_KUNITCONFIG_PATH = 'tools/testing/kunit/configs/default.config'
BROKEN_ALLCONFIG_PATH = 'tools/testing/kunit/configs/broken_on_uml.config'
+UML_KCONFIG_PATH = 'tools/testing/kunit/configs/arch_uml.config'
OUTFILE_PATH = 'test.log'
ABS_TOOL_PATH = os.path.abspath(os.path.dirname(__file__))
QEMU_CONFIGS_DIR = os.path.join(ABS_TOOL_PATH, 'qemu_configs')
@@ -53,7 +54,7 @@ class LinuxSourceTreeOperations:
except subprocess.CalledProcessError as e:
raise ConfigError(e.output.decode())
- def make_arch_qemuconfig(self, base_kunitconfig: kunit_config.Kconfig) -> kunit_config.Kconfig:
+ def make_arch_config(self, base_kunitconfig: kunit_config.Kconfig) -> kunit_config.Kconfig:
return base_kunitconfig
def make_allyesconfig(self, build_dir: str, make_options) -> None:
@@ -109,7 +110,7 @@ class LinuxSourceTreeOperationsQemu(LinuxSourceTreeOperations):
self._kernel_command_line = qemu_arch_params.kernel_command_line + ' kunit_shutdown=reboot'
self._extra_qemu_params = qemu_arch_params.extra_qemu_params
- def make_arch_qemuconfig(self, base_kunitconfig: kunit_config.Kconfig) -> kunit_config.Kconfig:
+ def make_arch_config(self, base_kunitconfig: kunit_config.Kconfig) -> kunit_config.Kconfig:
kconfig = kunit_config.parse_from_string(self._kconfig)
kconfig.merge_in_entries(base_kunitconfig)
return kconfig
@@ -138,6 +139,11 @@ class LinuxSourceTreeOperationsUml(LinuxSourceTreeOperations):
def __init__(self, cross_compile=None):
super().__init__(linux_arch='um', cross_compile=cross_compile)
+ def make_arch_config(self, base_kunitconfig: kunit_config.Kconfig) -> kunit_config.Kconfig:
+ kconfig = kunit_config.parse_file(UML_KCONFIG_PATH)
+ kconfig.merge_in_entries(base_kunitconfig)
+ return kconfig
+
def make_allyesconfig(self, build_dir: str, make_options) -> None:
stdout.print_with_timestamp(
'Enabling all CONFIGs for UML...')
@@ -298,7 +304,7 @@ class LinuxSourceTree:
if build_dir and not os.path.exists(build_dir):
os.mkdir(build_dir)
try:
- self._kconfig = self._ops.make_arch_qemuconfig(self._kconfig)
+ self._kconfig = self._ops.make_arch_config(self._kconfig)
self._kconfig.write_to_file(kconfig_path)
self._ops.make_olddefconfig(build_dir, make_options)
except ConfigError as e:
@@ -329,7 +335,7 @@ class LinuxSourceTree:
return self.build_config(build_dir, make_options)
existing_kconfig = kunit_config.parse_file(kconfig_path)
- self._kconfig = self._ops.make_arch_qemuconfig(self._kconfig)
+ self._kconfig = self._ops.make_arch_config(self._kconfig)
if self._kconfig.is_subset_of(existing_kconfig) and not self._kunitconfig_changed(build_dir):
return True
diff --git a/tools/testing/kunit/kunit_tool_test.py b/tools/testing/kunit/kunit_tool_test.py
index ad63d0d34f3f..446ac432d9a4 100755
--- a/tools/testing/kunit/kunit_tool_test.py
+++ b/tools/testing/kunit/kunit_tool_test.py
@@ -430,6 +430,10 @@ class LinuxSourceTreeTest(unittest.TestCase):
f.write('CONFIG_KUNIT=y')
tree = kunit_kernel.LinuxSourceTree(build_dir)
+ # Stub out the source tree operations, so we don't have
+ # the defaults for any given architecture get in the
+ # way.
+ tree._ops = kunit_kernel.LinuxSourceTreeOperations('none', None)
mock_build_config = mock.patch.object(tree, 'build_config').start()
# Should generate the .config
@@ -447,6 +451,10 @@ class LinuxSourceTreeTest(unittest.TestCase):
f.write('CONFIG_KUNIT=y\nCONFIG_KUNIT_TEST=y')
tree = kunit_kernel.LinuxSourceTree(build_dir)
+ # Stub out the source tree operations, so we don't have
+ # the defaults for any given architecture get in the
+ # way.
+ tree._ops = kunit_kernel.LinuxSourceTreeOperations('none', None)
mock_build_config = mock.patch.object(tree, 'build_config').start()
self.assertTrue(tree.build_reconfig(build_dir, make_options=[]))
@@ -463,6 +471,10 @@ class LinuxSourceTreeTest(unittest.TestCase):
f.write('CONFIG_KUNIT=y\nCONFIG_KUNIT_TEST=y')
tree = kunit_kernel.LinuxSourceTree(build_dir)
+ # Stub out the source tree operations, so we don't have
+ # the defaults for any given architecture get in the
+ # way.
+ tree._ops = kunit_kernel.LinuxSourceTreeOperations('none', None)
mock_build_config = mock.patch.object(tree, 'build_config').start()
# ... so we should trigger a call to build_config()
base-commit: cbb6bc7059151df198b45e883ed731d8f528b65b
--
2.37.0.rc0.161.g10f37bed90-goog
For some distributions (e.g. OpenWrt) we don't want to rely on rsync
to copy the tests to the target as some extra dependencies need to be
installed. The Makefile in tools/testing/selftests/net/forwarding
already installs most of the tests.
This series adds the two missing tests to the list of installed tests.
That way a downstream distribution can build a package using this
Makefile (and add dependencies there as needed).
Martin Blumenstingl (2):
selftests: forwarding: Install local_termination.sh
selftests: forwarding: Install no_forwarding.sh
tools/testing/selftests/net/forwarding/Makefile | 2 ++
1 file changed, 2 insertions(+)
--
2.37.0
This patch series makes two changes to how KUnit test suites are stored
and executed:
- The .kunit_test_suites section is now used for tests in modules (in
lieu of a module_init funciton), as well as for built-in tests. The
module loader will now trigger test execution. This frees up the
module_init function for other uses.
- Instead of storing an array of arrays of suites, have the
kunit_test_suite() and kunit_test_suites() macros append to one global
(or per-module) list of test suites. This removes a needless layer of
indirection, and removes the need to NULL-terminate suite_sets.
The upshot of this is that it should now be possible to use the
kunit_test_suite() and kunit_test_suites() macros to register test
suites even from within modules which otherwise had module_init
functions. This was proving to be quite a common issue, resulting in
several modules calling into KUnit's private suite execution functions
to run their tests (often introducing incompatibilities with the KUnit
tooling).
This series also fixes the thunderbolt, nitro_enclaves, and
sdhci-of-aspeed tests to use kunit_test_suite() now that it works. This
is required, as otherwise the first two patches may break these tests
entirely.
Huge thanks to Jeremy Kerr, who designed and implemented the module
loader changes, and to Daniel Latypov for pushing the simplification of
the nested arrays in .kunit_test_suites.
I've tested this series both with builtin tests on a number of
architectures, and with modules on x86_64, and it seems good-to-go to
me. More testing (particularly of modules) with more interesting setups
never hurts, though!
Cheers,
-- David
Changes since v2:
https://lore.kernel.org/linux-kselftest/20220621085345.603820-1-davidgow@go…
- Add various Reviewed-by and Acked-by tags.
- Fix the Kconfig for thunderbolt to not allow USB4=y and KUNIT=m with
tests enabled.
- Clean up the sdhci-of-aspeed init a bit more (Thanks Daniel)
Changes since v1:
https://lore.kernel.org/linux-kselftest/20220618090310.1174932-1-davidgow@g…
- Fix a compile issue when CONFIG_KUNIT=m (Thanks Christophe)
- No longer NULL-terminate suite_sets.
- Move the thunderbird Kconfig to the correct patch (Thanks Andra)
- Add all the Tested-by and Acked-by tags.
---
Daniel Latypov (1):
Daniel Latypov (1):
kunit: flatten kunit_suite*** to kunit_suite** in .kunit_test_suites
David Gow (3):
thunderbolt: test: Use kunit_test_suite() macro
nitro_enclaves: test: Use kunit_test_suite() macro
mmc: sdhci-of-aspeed: test: Use kunit_test_suite() macro
Jeremy Kerr (1):
kunit: unify module and builtin suite definitions
drivers/mmc/host/Kconfig | 5 +-
drivers/mmc/host/sdhci-of-aspeed-test.c | 8 +-
drivers/mmc/host/sdhci-of-aspeed.c | 34 +----
drivers/thunderbolt/Kconfig | 6 +-
drivers/thunderbolt/domain.c | 3 -
drivers/thunderbolt/tb.h | 8 -
drivers/thunderbolt/test.c | 12 +-
drivers/virt/nitro_enclaves/Kconfig | 5 +-
drivers/virt/nitro_enclaves/ne_misc_dev.c | 27 ----
.../virt/nitro_enclaves/ne_misc_dev_test.c | 5 +-
include/kunit/test.h | 60 ++------
include/linux/module.h | 5 +
kernel/module/main.c | 6 +
lib/kunit/executor.c | 115 ++++----------
lib/kunit/executor_test.c | 144 +++++-------------
lib/kunit/test.c | 54 ++++++-
16 files changed, 154 insertions(+), 343 deletions(-)
--
2.37.0.rc0.161.g10f37bed90-goog