On 12/18/2017 10:51 AM, Masami Hiramatsu wrote: > On Fri, 15 Dec 2017 14:12:54 -0500 > Josef Bacik <jo...@toxicpanda.com> wrote: >> From: Josef Bacik <jba...@fb.com> >> >> Error injection is sloppy and very ad-hoc. BPF could fill this niche >> perfectly with it's kprobe functionality. We could make sure errors are >> only triggered in specific call chains that we care about with very >> specific situations. Accomplish this with the bpf_override_funciton >> helper. This will modify the probe'd callers return value to the >> specified value and set the PC to an override function that simply >> returns, bypassing the originally probed function. This gives us a nice >> clean way to implement systematic error injection for all of our code >> paths. > > OK, got it. I think the error_injectable function list should be defined > in kernel/trace/bpf_trace.c because only bpf calls it and needs to care > the "safeness". > > [...] >> diff --git a/arch/x86/kernel/kprobes/ftrace.c >> b/arch/x86/kernel/kprobes/ftrace.c >> index 8dc0161cec8f..1ea748d682fd 100644 >> --- a/arch/x86/kernel/kprobes/ftrace.c >> +++ b/arch/x86/kernel/kprobes/ftrace.c >> @@ -97,3 +97,17 @@ int arch_prepare_kprobe_ftrace(struct kprobe *p) >> p->ainsn.boostable = false; >> return 0; >> } >> + >> +asmlinkage void override_func(void); >> +asm( >> + ".type override_func, @function\n" >> + "override_func:\n" >> + " ret\n" >> + ".size override_func, .-override_func\n" >> +); >> + >> +void arch_ftrace_kprobe_override_function(struct pt_regs *regs) >> +{ >> + regs->ip = (unsigned long)&override_func; >> +} >> +NOKPROBE_SYMBOL(arch_ftrace_kprobe_override_function); > > Calling this as "override_function" is meaningless. This is a function > which just return. So I think combination of just_return_func() and > arch_bpf_override_func_just_return() will be better. > > Moreover, this arch/x86/kernel/kprobes/ftrace.c is an archtecture > dependent implementation of kprobes, not bpf.
Josef, please work out any necessary cleanups that would still need to be addressed based on Masami's feedback and send them as follow-up patches, thanks. > Hmm, arch/x86/net/bpf_jit_comp.c will be better place? (No, it's JIT only and I'd really prefer to keep it that way, mixing this would result in a huge mess.)