Messages in this thread | | | Date | Thu, 31 May 2018 16:25:38 +0530 | From | "Naveen N. Rao" <> | Subject | Re: [PATCH -tip v4 24/27] bpf: error-inject: kprobes: Clear current_kprobe and enable preempt in kprobe |
| |
Masami Hiramatsu wrote: > Clear current_kprobe and enable preemption in kprobe > even if pre_handler returns !0. > > This simplifies function override using kprobes. > > Jprobe used to require to keep the preemption disabled and > keep current_kprobe until it returned to original function > entry. For this reason kprobe_int3_handler() and similar > arch dependent kprobe handers checks pre_handler result > and exit without enabling preemption if the result is !0. > > After removing the jprobe, Kprobes does not need to > keep preempt disabled even if user handler returns !0 > anymore.
I think the reason jprobes did it that way is to address architecture specific requirements when changing a function. So, without that infrastructure, I am not sure if we will be able to claim support for over-riding functions with kprobes. I am not sure if we want to claim that, but this is something we need to be clear on.
For powerpc, the current function override in error-inject works fine since the new function does nothing. But, if anyone wants to do more work in the replacement function, it won't work with the current approach.
> > But since the function override handler in error-inject > and bpf is also returns !0 if it overrides a function, > to balancing the preempt count, it enables preemption > and reset current kprobe by itself. > > That is a bad design that is very buggy. This fixes > such unbalanced preempt-count and current_kprobes setting > in kprobes, bpf and error-inject. > > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> > --- > arch/arc/kernel/kprobes.c | 5 +++-- > arch/arm/probes/kprobes/core.c | 10 +++++----- > arch/arm64/kernel/probes/kprobes.c | 10 +++++----- > arch/ia64/kernel/kprobes.c | 13 ++++--------- > arch/mips/kernel/kprobes.c | 4 ++-- > arch/powerpc/kernel/kprobes.c | 7 +++++--
I think you should also update arch/powerpc/kernel/kprobes-ftrace.c
- Naveen
| |