On Tue, 2017-02-14 at 00:05 +0900, Masami Hiramatsu wrote:
> This is arm port of commit 737480a0d525 ("kprobes/x86:
> Fix the return address of multiple kretprobes").
>
> Fix the return address of subsequent kretprobes when multiple
> kretprobes are set on the same function.
>
> For example:
>
> # cd /sys/kernel/debug/tracing
> # echo "r:event1 sys_symlink" > kprobe_events
> # echo "r:event2 sys_symlink" >> kprobe_events
> # echo 1 > events/kprobes/enable
> # ln -s /tmp/foo /tmp/bar
>
> (without this patch)
>
> # cat trace | grep -v ^#
> ln-82 [000] dn.2 68.446525: event1:
> (kretprobe_trampoline+0x0/0x18 <- SyS_symlink)
> ln-82 [000] dn.2 68.447831: event2:
> (ret_fast_syscall+0x0/0x1c <- SyS_symlink)
>
> (with this patch)
>
> # cat trace | grep -v ^#
> ln-81 [000] dn.1 39.463469: event1:
> (ret_fast_syscall+0x0/0x1c <- SyS_symlink)
> ln-81 [000] dn.1 39.464701: event2:
> (ret_fast_syscall+0x0/0x1c <- SyS_symlink)
>
> Signed-off-by: Masami Hiramatsu <mhira...@kernel.org>
> Cc: KUMANO Syuhei <kumano.p...@gmail.com>
> ---
I don't fully understand this function, but I've checked that the ARM
version now matches the x86 version (apart from the x86 specific
register fixup and some comments). So, FWIW
Acked-by: Jon Medhurst <t...@linaro.org>
I ran the before and after test case in the commit log on ARM and
verified the result is correct. I also tried running the ARM kprobe
tests with these 3 fixes but the tests fail. However, they also fail
without any of these changes, so I'll investigate that further...
> arch/arm/probes/kprobes/core.c | 24 ++++++++++++++++++++++--
> 1 file changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/probes/kprobes/core.c b/arch/arm/probes/kprobes/core.c
> index 84989ae..023800a 100644
> --- a/arch/arm/probes/kprobes/core.c
> +++ b/arch/arm/probes/kprobes/core.c
> @@ -440,6 +440,7 @@ static __used __kprobes void *trampoline_handler(struct
> pt_regs *regs)
> struct hlist_node *tmp;
> unsigned long flags, orig_ret_address = 0;
> unsigned long trampoline_address = (unsigned long)&kretprobe_trampoline;
> + kprobe_opcode_t *correct_ret_addr = NULL;
>
> INIT_HLIST_HEAD(&empty_rp);
> kretprobe_hash_lock(current, &head, &flags);
> @@ -462,14 +463,34 @@ static __used __kprobes void *trampoline_handler(struct
> pt_regs *regs)
> /* another task is sharing our hash bucket */
> continue;
>
> + orig_ret_address = (unsigned long)ri->ret_addr;
> +
> + if (orig_ret_address != trampoline_address)
> + /*
> + * This is the real return address. Any other
> + * instances associated with this task are for
> + * other calls deeper on the call stack
> + */
> + break;
> + }
> +
> + kretprobe_assert(ri, orig_ret_address, trampoline_address);
> +
> + correct_ret_addr = ri->ret_addr;
> + hlist_for_each_entry_safe(ri, tmp, head, hlist) {
> + if (ri->task != current)
> + /* another task is sharing our hash bucket */
> + continue;
> +
> + orig_ret_address = (unsigned long)ri->ret_addr;
> if (ri->rp && ri->rp->handler) {
> __this_cpu_write(current_kprobe, &ri->rp->kp);
> get_kprobe_ctlblk()->kprobe_status = KPROBE_HIT_ACTIVE;
> + ri->ret_addr = correct_ret_addr;
> ri->rp->handler(ri, regs);
> __this_cpu_write(current_kprobe, NULL);
> }
>
> - orig_ret_address = (unsigned long)ri->ret_addr;
> recycle_rp_inst(ri, &empty_rp);
>
> if (orig_ret_address != trampoline_address)
> @@ -481,7 +502,6 @@ static __used __kprobes void *trampoline_handler(struct
> pt_regs *regs)
> break;
> }
>
> - kretprobe_assert(ri, orig_ret_address, trampoline_address);
> kretprobe_hash_unlock(current, &flags);
>
> hlist_for_each_entry_safe(ri, tmp, &empty_rp, hlist) {
>
