diff mbox series

[v3,04/10] taint: Add TAINT_FWCTL

Message ID 4-v3-960f17f90f17+516-fwctl_jgg@nvidia.com
State New
Headers show
Series Introduce fwctl subystem | expand

Commit Message

Jason Gunthorpe Aug. 21, 2024, 6:10 p.m. UTC
Requesting a fwctl scope of access that includes mutating device debug
data will cause the kernel to be tainted. Changing the device operation
through things in the debug scope may cause the device to malfunction in
undefined ways. This should be reflected in the TAINT flags to help any
debuggers understand that something has been done.

Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
---
 Documentation/admin-guide/tainted-kernels.rst | 5 +++++
 include/linux/panic.h                         | 3 ++-
 kernel/panic.c                                | 1 +
 tools/debugging/kernel-chktaint               | 8 ++++++++
 4 files changed, 16 insertions(+), 1 deletion(-)

Comments

Greg KH Aug. 21, 2024, 11:35 p.m. UTC | #1
On Wed, Aug 21, 2024 at 03:10:56PM -0300, Jason Gunthorpe wrote:
> Requesting a fwctl scope of access that includes mutating device debug
> data will cause the kernel to be tainted. Changing the device operation
> through things in the debug scope may cause the device to malfunction in
> undefined ways. This should be reflected in the TAINT flags to help any
> debuggers understand that something has been done.

I know naming is hard, but the word "fwctl" is rough, don't you think?
It's become much more than just a random driver in the kernel tree, it's
now a taint flag and is exposed to userspace.  So I think you need to
rename it to something that is at least pronouncable when talking about
it (i.e. something with vowels...)

There's no need to keep it in 8 or less characters, this isn't the
1970's anymore, we have enough room for everyone to spell things out
now. :)

thanks,

greg k-h
Jason Gunthorpe Aug. 22, 2024, 3:34 p.m. UTC | #2
On Thu, Aug 22, 2024 at 07:35:20AM +0800, Greg Kroah-Hartman wrote:
> On Wed, Aug 21, 2024 at 03:10:56PM -0300, Jason Gunthorpe wrote:
> > Requesting a fwctl scope of access that includes mutating device debug
> > data will cause the kernel to be tainted. Changing the device operation
> > through things in the debug scope may cause the device to malfunction in
> > undefined ways. This should be reflected in the TAINT flags to help any
> > debuggers understand that something has been done.
> 
> I know naming is hard, but the word "fwctl" is rough, don't you think?
> It's become much more than just a random driver in the kernel tree, it's
> now a taint flag and is exposed to userspace.  So I think you need to
> rename it to something that is at least pronouncable when talking about
> it (i.e. something with vowels...)

Okay, that makes sense to me. We could also choose a different name
for the taint flag.

Let's see if some people have some ideas, I don't have a ready
alternative..

We've just been calling it "firwmare control" in conversation

Thanks,
Jason
diff mbox series

Patch

diff --git a/Documentation/admin-guide/tainted-kernels.rst b/Documentation/admin-guide/tainted-kernels.rst
index f92551539e8a66..f91a54966a9719 100644
--- a/Documentation/admin-guide/tainted-kernels.rst
+++ b/Documentation/admin-guide/tainted-kernels.rst
@@ -101,6 +101,7 @@  Bit  Log  Number  Reason that got the kernel tainted
  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
+ 19  _/J  524288  userspace used a mutating debug operation in fwctl
 ===  ===  ======  ========================================================
 
 Note: The character ``_`` is representing a blank in this table to make reading
@@ -182,3 +183,7 @@  More detailed explanation for tainting
      produce extremely unusual kernel structure layouts (even performance
      pathological ones), which is important to know when debugging. Set at
      build time.
+
+ 18) ``J`` if userpace opened /dev/fwctl/* and performed a FWTCL_RPC_DEBUG_WRITE
+     to use the devices debugging features. Device debugging features could
+     cause the device to malfunction in undefined ways.
diff --git a/include/linux/panic.h b/include/linux/panic.h
index 3130e0b5116b03..bf4f276be661b7 100644
--- a/include/linux/panic.h
+++ b/include/linux/panic.h
@@ -73,7 +73,8 @@  static inline void set_arch_panic_timeout(int timeout, int arch_default_timeout)
 #define TAINT_AUX			16
 #define TAINT_RANDSTRUCT		17
 #define TAINT_TEST			18
-#define TAINT_FLAGS_COUNT		19
+#define TAINT_FWCTL			19
+#define TAINT_FLAGS_COUNT		20
 #define TAINT_FLAGS_MAX			((1UL << TAINT_FLAGS_COUNT) - 1)
 
 struct taint_flag {
diff --git a/kernel/panic.c b/kernel/panic.c
index f861bedc1925e7..997b18c7455cd4 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -502,6 +502,7 @@  const struct taint_flag taint_flags[TAINT_FLAGS_COUNT] = {
 	TAINT_FLAG(AUX,				'X', ' ', true),
 	TAINT_FLAG(RANDSTRUCT,			'T', ' ', true),
 	TAINT_FLAG(TEST,			'N', ' ', true),
+	TAINT_FLAG(FWCTL,			'J', ' ', true),
 };
 
 #undef TAINT_FLAG
diff --git a/tools/debugging/kernel-chktaint b/tools/debugging/kernel-chktaint
index 279be06332be99..e7da0909d09707 100755
--- a/tools/debugging/kernel-chktaint
+++ b/tools/debugging/kernel-chktaint
@@ -204,6 +204,14 @@  else
 	echo " * an in-kernel test (such as a KUnit test) has been run (#18)"
 fi
 
+T=`expr $T / 2`
+if [ `expr $T % 2` -eq 0 ]; then
+	addout " "
+else
+	addout "J"
+	echo " * fwctl's mutating debug interface was used (#19)"
+fi
+
 echo "For a more detailed explanation of the various taint flags see"
 echo " Documentation/admin-guide/tainted-kernels.rst in the Linux kernel sources"
 echo " or https://kernel.org/doc/html/latest/admin-guide/tainted-kernels.html"