[v1] panic: lib: Add TAINT for FAULT_INJECTION

[RFC][PATCH] panic: lib: Add TAINT for FAULT_INJECTION

Posted by Steven Rostedt 8 months, 1 week ago

From: Steven Rostedt <rostedt@goodmis.org>

There's been several times where syzbot reports a bug that was caused by a
fault injection, but it doesn't report this fact in its email reports.
The bug report could happen in code that wasn't involved with the fault
injection due to the code that faulted not cleaning up things properly,
leading to an unstable kernel, which in turn can trigger issues elsewhere
that doesn't have a bug (much like a proprietary module could do).

It would be very useful if the syzbot report notified the developer that
the test had injected faults into the kernel.

Introduce a new taint flag 'V' that gets set when a fault injection takes
place. (Note, there's not many taint flags left, so 'V' looked as good as
any other value).

This will let the syzbot see that the kernel had a fault injection during
the test and can report that it happened when sending out its emails.

It may also be useful for other bug reports.

Link: https://lore.kernel.org/all/67f67726.050a0220.25d1c8.0004.GAE@google.com/
Link: https://github.com/google/syzkaller/issues/5621

Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---

[
 Currently this keeps lockdep enabled. Should lockdep be turned off too?

 Also, I'm surprised that fault injection didn't already taint the kernel.
]

 include/linux/panic.h | 3 ++-
 kernel/panic.c        | 1 +
 lib/fault-inject.c    | 3 +++
 3 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/include/linux/panic.h b/include/linux/panic.h
index 2494d51707ef..2d9048af6f89 100644
--- a/include/linux/panic.h
+++ b/include/linux/panic.h
@@ -75,7 +75,8 @@ static inline void set_arch_panic_timeout(int timeout, int arch_default_timeout)
 #define TAINT_RANDSTRUCT		17
 #define TAINT_TEST			18
 #define TAINT_FWCTL			19
-#define TAINT_FLAGS_COUNT		20
+#define TAINT_FAULT_INJECTION		20
+#define TAINT_FLAGS_COUNT		21
 #define TAINT_FLAGS_MAX			((1UL << TAINT_FLAGS_COUNT) - 1)
 
 struct taint_flag {
diff --git a/kernel/panic.c b/kernel/panic.c
index a3889f38153d..fec561f07ca7 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -512,6 +512,7 @@ const struct taint_flag taint_flags[TAINT_FLAGS_COUNT] = {
 	TAINT_FLAG(RANDSTRUCT,			'T', ' ', true),
 	TAINT_FLAG(TEST,			'N', ' ', true),
 	TAINT_FLAG(FWCTL,			'J', ' ', true),
+	TAINT_FLAG(FAULT_INJECTION,		'V', ' ', false),
 };
 
 #undef TAINT_FLAG
diff --git a/lib/fault-inject.c b/lib/fault-inject.c
index 999053fa133e..2dac4c20adc1 100644
--- a/lib/fault-inject.c
+++ b/lib/fault-inject.c
@@ -176,6 +176,9 @@ bool should_fail_ex(struct fault_attr *attr, ssize_t size, int flags)
 	if (atomic_read(&attr->times) != -1)
 		atomic_dec_not_zero(&attr->times);
 
+	pr_notice_once("Tainting kernel with TAINT_FAULT_INJECTION\n");
+	add_taint(TAINT_FAULT_INJECTION, LOCKDEP_STILL_OK);
+
 	return true;
 }
 
-- 
2.47.2

Re: [RFC][PATCH] panic: lib: Add TAINT for FAULT_INJECTION

Posted by Andrew Morton 8 months, 1 week ago

On Thu, 10 Apr 2025 14:43:59 -0400 Steven Rostedt <rostedt@goodmis.org> wrote:

> From: Steven Rostedt <rostedt@goodmis.org>
> 
> There's been several times where syzbot reports a bug that was caused by a
> fault injection, but it doesn't report this fact in its email reports.
> The bug report could happen in code that wasn't involved with the fault
> injection due to the code that faulted not cleaning up things properly,
> leading to an unstable kernel, which in turn can trigger issues elsewhere
> that doesn't have a bug (much like a proprietary module could do).
> 
> It would be very useful if the syzbot report notified the developer that
> the test had injected faults into the kernel.
> 
> Introduce a new taint flag 'V' that gets set when a fault injection takes
> place. (Note, there's not many taint flags left, so 'V' looked as good as
> any other value).
> 
> This will let the syzbot see that the kernel had a fault injection during
> the test and can report that it happened when sending out its emails.
> 
> It may also be useful for other bug reports.

Seems sane.  Does any userspace tooling need an update for this?

> --- a/lib/fault-inject.c
> +++ b/lib/fault-inject.c
> @@ -176,6 +176,9 @@ bool should_fail_ex(struct fault_attr *attr, ssize_t size, int flags)
>  	if (atomic_read(&attr->times) != -1)
>  		atomic_dec_not_zero(&attr->times);
>  
> +	pr_notice_once("Tainting kernel with TAINT_FAULT_INJECTION\n");
> +	add_taint(TAINT_FAULT_INJECTION, LOCKDEP_STILL_OK);

	if (pr_notice_once("Tainting kernel with TAINT_FAULT_INJECTION\n"))
		add_taint(TAINT_FAULT_INJECTION, LOCKDEP_STILL_OK);

perhaps?

>  	return true;
>  }
>  
> -- 
> 2.47.2

Re: [RFC][PATCH] panic: lib: Add TAINT for FAULT_INJECTION

Posted by Steven Rostedt 8 months, 1 week ago

On Thu, 10 Apr 2025 13:51:14 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:

> > It may also be useful for other bug reports.  
> 
> Seems sane.  Does any userspace tooling need an update for this?

Well, the syzbot does show the taints in their reports. It can use that to
explicitly state that the kernel has fault injections, or at least the
developer can see the taint.

> 
> > --- a/lib/fault-inject.c
> > +++ b/lib/fault-inject.c
> > @@ -176,6 +176,9 @@ bool should_fail_ex(struct fault_attr *attr, ssize_t size, int flags)
> >  	if (atomic_read(&attr->times) != -1)
> >  		atomic_dec_not_zero(&attr->times);
> >  
> > +	pr_notice_once("Tainting kernel with TAINT_FAULT_INJECTION\n");
> > +	add_taint(TAINT_FAULT_INJECTION, LOCKDEP_STILL_OK);  
> 
> 	if (pr_notice_once("Tainting kernel with TAINT_FAULT_INJECTION\n"))
> 		add_taint(TAINT_FAULT_INJECTION, LOCKDEP_STILL_OK);
> 
> perhaps?

I'm fine with that too. I can send a v2.

-- Steve

> 
> >  	return true;
> >  }
> >