diff mbox series

selftests: Add a taint selftest

Message ID 20220823211958.2519055-1-adelg@google.com (mailing list archive)
State New
Headers show
Series selftests: Add a taint selftest | expand

Commit Message

Andrew Delgadillo Aug. 23, 2022, 9:19 p.m. UTC
From: Andrew Delgadillo <adelg@google.com>

When testing a kernel, one of the earliest signals one can get is if a
kernel has become tainted. For example, an organization might be
interested in mass testing commits on their hardware. An obvious first
step would be to make sure every commit boots, and a next step would be
to make sure there are no warnings/crashes/lockups, hence the utility of
a taint test.

Signed-off-by: Andrew Delgadillo <adelg@google.com>
---
 tools/testing/selftests/core/Makefile |  1 +
 tools/testing/selftests/core/taint.sh | 28 +++++++++++++++++++++++++++
 2 files changed, 29 insertions(+)
 create mode 100755 tools/testing/selftests/core/taint.sh

Comments

Greg Kroah-Hartman Aug. 24, 2022, 5:06 a.m. UTC | #1
On Tue, Aug 23, 2022 at 09:19:58PM +0000, Andrew Delgadilo wrote:
> From: Andrew Delgadillo <adelg@google.com>
> 
> When testing a kernel, one of the earliest signals one can get is if a
> kernel has become tainted. For example, an organization might be
> interested in mass testing commits on their hardware. An obvious first
> step would be to make sure every commit boots, and a next step would be
> to make sure there are no warnings/crashes/lockups, hence the utility of
> a taint test.

What's wrong with the tools/debugging/kernel-chktaint script?

Why do we need another "get what the taint status is" program?

thanks,

greg k-h
Andrew Delgadillo Aug. 24, 2022, 5:50 a.m. UTC | #2
On Tue, Aug 23, 2022 at 10:06 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Tue, Aug 23, 2022 at 09:19:58PM +0000, Andrew Delgadilo wrote:
> > From: Andrew Delgadillo <adelg@google.com>
> >
> > When testing a kernel, one of the earliest signals one can get is if a
> > kernel has become tainted. For example, an organization might be
> > interested in mass testing commits on their hardware. An obvious first
> > step would be to make sure every commit boots, and a next step would be
> > to make sure there are no warnings/crashes/lockups, hence the utility of
> > a taint test.
>
> What's wrong with the tools/debugging/kernel-chktaint script?
>
> Why do we need another "get what the taint status is" program?

The main functionality that this selftests has that kernel-chktaint
does not is that it exits with a non-zero status code if the kernel is
tainted. kernel-chktaint outputs information to stdout based on the
taint status, but will always exit 0.

The issue with this is that it cannot be plugged into a test runner
that checks the exit code of a test script. In other words, if I
wanted to plug it into git bisect, I would have to wrap
kernel-chktaint in a command that transformed the output to an exit
code. Sure that is doable, but it is not as simple as it could be.

More concretely, I am setting kselftest runs against kernel commits
(with a harness that logs kselftest runs into some other
infrastructure), and such a test that is missing is a kselftest that
checks the kernel's taint status. One could argue that one should just
create a kselftest target that calls into kernel-chktaint and parse
the output there to determine what the exit status is, but that seems
fragile as a change in the underlying script could break it. For
example, if I want to test for taint #18, and I am grepping for the
string " * an in-kernel test has been run (#18)", I will actually get
a false positive because the underlying script does not check for
taint #18. Contrived example yes, but I think it shows that textual
grepping for errors is error prone (as an aside, I'll send a patch to
update the script to check for the new taint bit).

> thanks,
>
> greg k-h
Greg Kroah-Hartman Aug. 24, 2022, 5:51 a.m. UTC | #3
On Wed, Aug 24, 2022 at 07:06:27AM +0200, Greg KH wrote:
> On Tue, Aug 23, 2022 at 09:19:58PM +0000, Andrew Delgadilo wrote:
> > From: Andrew Delgadillo <adelg@google.com>
> > 
> > When testing a kernel, one of the earliest signals one can get is if a
> > kernel has become tainted. For example, an organization might be
> > interested in mass testing commits on their hardware. An obvious first
> > step would be to make sure every commit boots, and a next step would be
> > to make sure there are no warnings/crashes/lockups, hence the utility of
> > a taint test.
> 
> What's wrong with the tools/debugging/kernel-chktaint script?
> 
> Why do we need another "get what the taint status is" program?

Ah, looks like that script should probably be changed to return an error
code if a taint is found, but that should be a simpler patch rather than
create a whole new script.

thanks,

greg k-h
Greg Kroah-Hartman Aug. 24, 2022, 6:22 a.m. UTC | #4
On Tue, Aug 23, 2022 at 10:50:33PM -0700, Andrew Delgadillo wrote:
> On Tue, Aug 23, 2022 at 10:06 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Tue, Aug 23, 2022 at 09:19:58PM +0000, Andrew Delgadilo wrote:
> > > From: Andrew Delgadillo <adelg@google.com>
> > >
> > > When testing a kernel, one of the earliest signals one can get is if a
> > > kernel has become tainted. For example, an organization might be
> > > interested in mass testing commits on their hardware. An obvious first
> > > step would be to make sure every commit boots, and a next step would be
> > > to make sure there are no warnings/crashes/lockups, hence the utility of
> > > a taint test.
> >
> > What's wrong with the tools/debugging/kernel-chktaint script?
> >
> > Why do we need another "get what the taint status is" program?
> 
> The main functionality that this selftests has that kernel-chktaint
> does not is that it exits with a non-zero status code if the kernel is
> tainted. kernel-chktaint outputs information to stdout based on the
> taint status, but will always exit 0.

Great, then change that, don't create a whole new script :)

> The issue with this is that it cannot be plugged into a test runner
> that checks the exit code of a test script. In other words, if I
> wanted to plug it into git bisect, I would have to wrap
> kernel-chktaint in a command that transformed the output to an exit
> code. Sure that is doable, but it is not as simple as it could be.
> 
> More concretely, I am setting kselftest runs against kernel commits
> (with a harness that logs kselftest runs into some other
> infrastructure), and such a test that is missing is a kselftest that
> checks the kernel's taint status. One could argue that one should just
> create a kselftest target that calls into kernel-chktaint and parse
> the output there to determine what the exit status is, but that seems
> fragile as a change in the underlying script could break it. For
> example, if I want to test for taint #18, and I am grepping for the
> string " * an in-kernel test has been run (#18)", I will actually get
> a false positive because the underlying script does not check for
> taint #18. Contrived example yes, but I think it shows that textual
> grepping for errors is error prone (as an aside, I'll send a patch to
> update the script to check for the new taint bit).

Then modify the existing script to handle your use case, let's not have
duplicate ones in the tree.

thanks,

greg k-h
diff mbox series

Patch

diff --git a/tools/testing/selftests/core/Makefile b/tools/testing/selftests/core/Makefile
index f6f2d6f473c6a..695bdbfb02f90 100644
--- a/tools/testing/selftests/core/Makefile
+++ b/tools/testing/selftests/core/Makefile
@@ -2,6 +2,7 @@ 
 CFLAGS += -g -I../../../../usr/include/
 
 TEST_GEN_PROGS := close_range_test
+TEST_PROGS := taint.sh
 
 include ../lib.mk
 
diff --git a/tools/testing/selftests/core/taint.sh b/tools/testing/selftests/core/taint.sh
new file mode 100755
index 0000000000000..661c2cb8cd9bf
--- /dev/null
+++ b/tools/testing/selftests/core/taint.sh
@@ -0,0 +1,28 @@ 
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+set -oue pipefail
+
+# By default, we only want to check if our system has:
+# - seen an oops or bug
+# - a warning occurred
+# - a lockup occurred
+# The bit values for these, and more, can be found at
+# Documentation/admin-guide/tainted-kernels.html
+# This value can be overridden by passing a mask as the
+# first positional argument.
+taint_bitmask=$(( 128 + 512 + 16384 ))
+
+# If we have a positional argument, then override our
+# default bitmask.
+if [[ -n "${1-}" ]]; then
+	taint_bitmask=$1
+fi
+
+taint_bits=$(cat /proc/sys/kernel/tainted)
+
+result=$(( taint_bitmask & taint_bits ))
+if [[ "$result" -ne 0 ]]; then
+	exit 1
+fi
+
+exit 0