Messages in this thread Patch in this message | | | Date | Fri, 2 Dec 2022 16:55:07 -0800 | From | Alexei Starovoitov <> | Subject | Re: [PATCH] error-injection: Add prompt for function error injection |
| |
On Fri, Dec 02, 2022 at 06:17:24PM -0500, Steven Rostedt wrote: > On Fri, 2 Dec 2022 13:27:11 -0800 > Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > > > --- a/lib/Kconfig.debug > > +++ b/lib/Kconfig.debug > > @@ -1874,8 +1874,12 @@ config NETDEV_NOTIFIER_ERROR_INJECT > > If unsure, say N. > > > > config FUNCTION_ERROR_INJECTION > > Why not just call this "ERROR_INJECTION" having this be FUNCTION and the > one for functions be FUNC is confusing.
That's what I had initially, but it causes plenty of churn to arch/*/Makefile and a bunch of .h-s. Keeping it as FUNCTION_ERROR_INJECTION removes all that noise from the diff.
> > + bool > > + > > +config FUNC_ERROR_INJECTION > > bool "Fault-injections of functions" > > depends on HAVE_FUNCTION_ERROR_INJECTION && KPROBES > > + select FUNCTION_ERROR_INJECTION > > help > > Add fault injections into various functions that are annotated with > > ALLOW_ERROR_INJECTION() in the kernel. BPF may also modify the return > > @@ -1883,6 +1887,17 @@ config FUNCTION_ERROR_INJECTION > > > > If unsure, say N > > > > +config SYSCALL_ERROR_INJECTION > > + bool "Error injections in syscalls" > > + depends on HAVE_FUNCTION_ERROR_INJECTION && KPROBES > > + select FUNCTION_ERROR_INJECTION > > + default y > > IIUC, Linus prefers everything to be "default n" unless there's a really > good reason for it. Like only making other options available, but not doing > anything to the kernel. I do have DYNAMIC_FTRACE as "default y" but that's > only because it depends on CONFIG_FUNCTION_TRACER and nobody that enables > that should have DYNAMIC_FTRACE off (except for academia).
The FUNCTION_ERROR_INJECTION used to be "def_bool y" for ~5 years. BTW the macro was called BPF_ALLOW_ERROR_INJECTION() when Josef initially implemented it. Massami later renamed it ALLOW_ERROR_INJECTION() and allowed kprobes to use it. Today there is a user expectation that this feature is available in the kernel. We can do "default n" here, let distros decide and potentially upset users. I don't feel strongly about that.
> > > + help > > + Allows error injection framework to return errors from syscalls. > > + BPF may modify return values of syscalls as well. > > And here's the thing. If BPF returns anything *but* an error, then this is > a misnomer and incorrect. Name it something else like "HIJACK_SYSCALLS".
The bpf prog must return errno. No doubt about that. Today the verifier validates return values whenever is necessary. When original bpf_override_return was added the verifier wasn't that smart. Since then we added return value checks pretty much everywhere. Looks like the check is still missing bpf_override_return. We will fix it asap.
> > + > > + If unsure, say Y > > And I'm curious, why Y if unsure?
Copy-paste. I can remove that line.
| |