include/linux/panic.h | 3 ++- kernel/fail_function.c | 2 ++ kernel/panic.c | 1 + kernel/trace/bpf_trace.c | 2 ++ 4 files changed, 7 insertions(+), 1 deletion(-)
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Since the function error injection framework in the fault injection
subsystem can change the function code flow forcibly, it may cause
unexpected behavior (but that is the purpose of this feature).
To identify this in the kernel oops message, add a new taint flag
for this, and set it if it is (and similar things in BPF) used.
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
include/linux/panic.h | 3 ++-
kernel/fail_function.c | 2 ++
kernel/panic.c | 1 +
kernel/trace/bpf_trace.c | 2 ++
4 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/include/linux/panic.h b/include/linux/panic.h
index c7759b3f2045..2b03a02d86be 100644
--- a/include/linux/panic.h
+++ b/include/linux/panic.h
@@ -69,7 +69,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_FAULT_INJECTED 19
+#define TAINT_FLAGS_COUNT 20
#define TAINT_FLAGS_MAX ((1UL << TAINT_FLAGS_COUNT) - 1)
struct taint_flag {
diff --git a/kernel/fail_function.c b/kernel/fail_function.c
index a7ccd2930c5f..80a743f14a2c 100644
--- a/kernel/fail_function.c
+++ b/kernel/fail_function.c
@@ -9,6 +9,7 @@
#include <linux/kprobes.h>
#include <linux/module.h>
#include <linux/mutex.h>
+#include <linux/panic.h>
#include <linux/slab.h>
#include <linux/uaccess.h>
@@ -298,6 +299,7 @@ static ssize_t fei_write(struct file *file, const char __user *buffer,
fei_attr_free(attr);
goto out;
}
+ add_taint(TAINT_FAULT_INJECTED, LOCKDEP_NOW_UNRELIABLE);
fei_debugfs_add_attr(attr);
list_add_tail(&attr->list, &fei_attr_list);
ret = count;
diff --git a/kernel/panic.c b/kernel/panic.c
index da323209f583..e396a5fd9bb6 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -426,6 +426,7 @@ const struct taint_flag taint_flags[TAINT_FLAGS_COUNT] = {
[ TAINT_AUX ] = { 'X', ' ', true },
[ TAINT_RANDSTRUCT ] = { 'T', ' ', true },
[ TAINT_TEST ] = { 'N', ' ', true },
+ [ TAINT_FAULT_INJECTED ] = { 'J', ' ', false },
};
/**
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 1ed08967fb97..de0614d9796c 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -2137,6 +2137,8 @@ int perf_event_attach_bpf_prog(struct perf_event *event,
goto unlock;
/* set the new array to event->tp_event and set event->prog */
+ if (prog->kprobe_override)
+ add_taint(TAINT_FAULT_INJECTED, LOCKDEP_NOW_UNRELIABLE);
event->prog = prog;
event->bpf_cookie = bpf_cookie;
rcu_assign_pointer(event->tp_event->prog_array, new_array);On Fri, 2 Dec 2022 01:37:13 +0900 "Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote: > From: Masami Hiramatsu (Google) <mhiramat@kernel.org> > > Since the function error injection framework in the fault injection > subsystem can change the function code flow forcibly, it may cause > unexpected behavior (but that is the purpose of this feature). > To identify this in the kernel oops message, add a new taint flag > for this, and set it if it is (and similar things in BPF) used. > > Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> > Acked-by: Steven Rostedt (Google) <rostedt@goodmis.org> -- Steve
On Fri, Dec 02, 2022 at 01:37:13AM +0900, Masami Hiramatsu (Google) wrote: > From: Masami Hiramatsu (Google) <mhiramat@kernel.org> > > Since the function error injection framework in the fault injection > subsystem can change the function code flow forcibly, it may cause > unexpected behavior (but that is the purpose of this feature). > To identify this in the kernel oops message, add a new taint flag > for this, and set it if it is (and similar things in BPF) used. Why is hooking through BPF considered to be "fault injection" here? -- Kees Cook
On Thu, 1 Dec 2022 08:39:28 -0800 Kees Cook <keescook@chromium.org> wrote: > On Fri, Dec 02, 2022 at 01:37:13AM +0900, Masami Hiramatsu (Google) wrote: > > From: Masami Hiramatsu (Google) <mhiramat@kernel.org> > > > > Since the function error injection framework in the fault injection > > subsystem can change the function code flow forcibly, it may cause > > unexpected behavior (but that is the purpose of this feature). > > To identify this in the kernel oops message, add a new taint flag > > for this, and set it if it is (and similar things in BPF) used. > > Why is hooking through BPF considered to be "fault injection" here? > Have you not been reading this thread? -- Steve
On Thu, Dec 01, 2022 at 11:48:48AM -0500, Steven Rostedt wrote: > On Thu, 1 Dec 2022 08:39:28 -0800 > Kees Cook <keescook@chromium.org> wrote: > > > On Fri, Dec 02, 2022 at 01:37:13AM +0900, Masami Hiramatsu (Google) wrote: > > > From: Masami Hiramatsu (Google) <mhiramat@kernel.org> > > > > > > Since the function error injection framework in the fault injection > > > subsystem can change the function code flow forcibly, it may cause > > > unexpected behavior (but that is the purpose of this feature). > > > To identify this in the kernel oops message, add a new taint flag > > > for this, and set it if it is (and similar things in BPF) used. > > > > Why is hooking through BPF considered to be "fault injection" here? > > > > Have you not been reading this thread? I skimmed it -- trying to catch up from turkey week. If this was already covered, then please ignore me. It just wasn't obvious from the commit log why it was included. -- Kees Cook
On Thu, 1 Dec 2022 08:53:02 -0800 Kees Cook <keescook@chromium.org> wrote: > > Have you not been reading this thread? > > I skimmed it -- trying to catch up from turkey week. If this was already > covered, then please ignore me. It just wasn't obvious from the commit > log why it was included. That's a better request :-) That is, please add why this is needed for BPF (and also include a Link: tag to this thread). -- Steve
On Thu, 1 Dec 2022 14:14:26 -0500 Steven Rostedt <rostedt@goodmis.org> wrote: > On Thu, 1 Dec 2022 08:53:02 -0800 > Kees Cook <keescook@chromium.org> wrote: > > > > Have you not been reading this thread? > > > > I skimmed it -- trying to catch up from turkey week. If this was already > > covered, then please ignore me. It just wasn't obvious from the commit > > log why it was included. > > That's a better request :-) > > That is, please add why this is needed for BPF (and also include a Link: > tag to this thread). OK, let me update this :) Thank you! > > -- Steve -- Masami Hiramatsu (Google) <mhiramat@kernel.org>
On 12/1/22 2:14 PM, Steven Rostedt wrote: > On Thu, 1 Dec 2022 08:53:02 -0800 > Kees Cook <keescook@chromium.org> wrote: > >>> Have you not been reading this thread? >> >> I skimmed it -- trying to catch up from turkey week. If this was already >> covered, then please ignore me. It just wasn't obvious from the commit >> log why it was included. > > That's a better request :-) > > That is, please add why this is needed for BPF (and also include a Link: > tag to this thread). Sorry, I'm completely failing to parse. Is this directed at Kees or Benjamin? I'm also not sure what the this is in "why this is needed for BPF"? -chris
On Thu, 1 Dec 2022 16:00:03 -0500 Chris Mason <clm@meta.com> wrote: > On 12/1/22 2:14 PM, Steven Rostedt wrote: > > On Thu, 1 Dec 2022 08:53:02 -0800 > > Kees Cook <keescook@chromium.org> wrote: > > > >>> Have you not been reading this thread? > >> > >> I skimmed it -- trying to catch up from turkey week. If this was already > >> covered, then please ignore me. It just wasn't obvious from the commit > >> log why it was included. > > > > That's a better request :-) > > > > That is, please add why this is needed for BPF (and also include a Link: > > tag to this thread). > > Sorry, I'm completely failing to parse. Is this directed at Kees or > Benjamin? I'm also not sure what the this is in "why this is needed for > BPF"? > It was directed towards Kees. I don't even know who "Benjamin" is. I don't see a "Benjamin" in the Cc list. And "this" is for: --- a/kernel/trace/bpf_trace.c +++ b/kernel/trace/bpf_trace.c @@ -2137,6 +2137,8 @@ int perf_event_attach_bpf_prog(struct perf_event *event, goto unlock; /* set the new array to event->tp_event and set event->prog */ + if (prog->kprobe_override) + add_taint(TAINT_FAULT_INJECTED, LOCKDEP_NOW_UNRELIABLE); event->prog = prog; event->bpf_cookie = bpf_cookie; rcu_assign_pointer(event->tp_event->prog_array, new_array); -- Steve
On Thu, 1 Dec 2022 16:25:26 -0500 Steven Rostedt <rostedt@goodmis.org> wrote: > > Sorry, I'm completely failing to parse. Is this directed at Kees or > > Benjamin? I'm also not sure what the this is in "why this is needed for > > BPF"? > > > > It was directed towards Kees. I don't even know who "Benjamin" is. I don't > see a "Benjamin" in the Cc list. Oh, I see a Benjamin replied to another branch of the email thread. May I suggest getting a better email client ;-) One that has proper threading where it is obvious which email is being replied to. -- Steve
On Thu, Dec 1, 2022 at 1:00 PM Chris Mason <clm@meta.com> wrote:
>
> On 12/1/22 2:14 PM, Steven Rostedt wrote:
> >
> > That is, please add why this is needed for BPF (and also include a Link:
> > tag to this thread).
>
> Sorry, I'm completely failing to parse. Is this directed at Kees or
> Benjamin? I'm also not sure what the this is in "why this is needed for
> BPF"?
It's not at all "needed for bpf".
There are mis-uses of error injection that have nothing to do with
error injection in linux-next, and some people have argued that said
mis-uses are a valid.
They aren't. They need fixing. Thankfully they haven't made it
upstream, and I most definitely do not want random users mis-using
"error injection" to inject random bpf code for non-error cases.
Linus
On Thu, Dec 01, 2022 at 01:18:09PM -0800, Linus Torvalds wrote: > They aren't. They need fixing. Thankfully they haven't made it > upstream, and I most definitely do not want random users mis-using > "error injection" to inject random bpf code for non-error cases. Which seem to be what HID-BPF is all about. And if I see linux-next reports correctly that actually got merged despite all the oustanding objections.
© 2016 - 2025 Red Hat, Inc.