Skip to content

Commit

Permalink
x86/kprobes: Avoid kretprobe recursion bug
Browse files Browse the repository at this point in the history
[ Upstream commit b191fa9 ]

Avoid kretprobe recursion loop bg by setting a dummy
kprobes to current_kprobe per-CPU variable.

This bug has been introduced with the asm-coded trampoline
code, since previously it used another kprobe for hooking
the function return placeholder (which only has a nop) and
trampoline handler was called from that kprobe.

This revives the old lost kprobe again.

With this fix, we don't see deadlock anymore.

And you can see that all inner-called kretprobe are skipped.

  event_1                                  235               0
  event_2                                19375           19612

The 1st column is recorded count and the 2nd is missed count.
Above shows (event_1 rec) + (event_2 rec) ~= (event_2 missed)
(some difference are here because the counter is racy)

Reported-by: Andrea Righi <[email protected]>
Tested-by: Andrea Righi <[email protected]>
Signed-off-by: Masami Hiramatsu <[email protected]>
Acked-by: Steven Rostedt <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Mathieu Desnoyers <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: [email protected]
Fixes: c9becf5 ("[PATCH] kretprobe: kretprobe-booster")
Link: http://lkml.kernel.org/r/155094064889.6137.972160690963039.stgit@devbox
Signed-off-by: Ingo Molnar <[email protected]>
Signed-off-by: Sasha Levin <[email protected]>
  • Loading branch information
mhiramat authored and Sasha Levin committed Jun 30, 2020
1 parent a07f99a commit 45ac65d
Showing 1 changed file with 20 additions and 2 deletions.
22 changes: 20 additions & 2 deletions arch/x86/kernel/kprobes/core.c
Original file line number Diff line number Diff line change
Expand Up @@ -740,11 +740,16 @@ asm(
NOKPROBE_SYMBOL(kretprobe_trampoline);
STACK_FRAME_NON_STANDARD(kretprobe_trampoline);

static struct kprobe kretprobe_kprobe = {
.addr = (void *)kretprobe_trampoline,
};

/*
* Called from kretprobe_trampoline
*/
__visible __used void *trampoline_handler(struct pt_regs *regs)
{
struct kprobe_ctlblk *kcb;
struct kretprobe_instance *ri = NULL;
struct hlist_head *head, empty_rp;
struct hlist_node *tmp;
Expand All @@ -754,6 +759,17 @@ __visible __used void *trampoline_handler(struct pt_regs *regs)
void *frame_pointer;
bool skipped = false;

preempt_disable();

/*
* Set a dummy kprobe for avoiding kretprobe recursion.
* Since kretprobe never run in kprobe handler, kprobe must not
* be running at this point.
*/
kcb = get_kprobe_ctlblk();
__this_cpu_write(current_kprobe, &kretprobe_kprobe);
kcb->kprobe_status = KPROBE_HIT_ACTIVE;

INIT_HLIST_HEAD(&empty_rp);
kretprobe_hash_lock(current, &head, &flags);
/* fixup registers */
Expand Down Expand Up @@ -829,10 +845,9 @@ __visible __used void *trampoline_handler(struct pt_regs *regs)
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);
__this_cpu_write(current_kprobe, &kretprobe_kprobe);
}

recycle_rp_inst(ri, &empty_rp);
Expand All @@ -848,6 +863,9 @@ __visible __used void *trampoline_handler(struct pt_regs *regs)

kretprobe_hash_unlock(current, &flags);

__this_cpu_write(current_kprobe, NULL);
preempt_enable();

hlist_for_each_entry_safe(ri, tmp, &empty_rp, hlist) {
hlist_del(&ri->hlist);
kfree(ri);
Expand Down

0 comments on commit 45ac65d

Please sign in to comment.