Re: [PATCH -tip v4 24/27] bpf: error-inject: kprobes: Clear current_kprobe and enable preempt in kprobe

From: Naveen N. Rao
Date: Thu May 31 2018 - 06:55:54 EST


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@xxxxxxxxxx>
---
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