mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git
synced 2024-09-12 13:55:32 +00:00
kretprobe: Prevent triggering kretprobe from within kprobe_flush_task
[ Upstream commit9b38cc704e
] Ziqian reported lockup when adding retprobe on _raw_spin_lock_irqsave. My test was also able to trigger lockdep output: ============================================ WARNING: possible recursive locking detected 5.6.0-rc6+ #6 Not tainted -------------------------------------------- sched-messaging/2767 is trying to acquire lock: ffffffff9a492798 (&(kretprobe_table_locks[i].lock)){-.-.}, at: kretprobe_hash_lock+0x52/0xa0 but task is already holding lock: ffffffff9a491a18 (&(kretprobe_table_locks[i].lock)){-.-.}, at: kretprobe_trampoline+0x0/0x50 other info that might help us debug this: Possible unsafe locking scenario: CPU0 ---- lock(&(kretprobe_table_locks[i].lock)); lock(&(kretprobe_table_locks[i].lock)); *** DEADLOCK *** May be due to missing lock nesting notation 1 lock held by sched-messaging/2767: #0: ffffffff9a491a18 (&(kretprobe_table_locks[i].lock)){-.-.}, at: kretprobe_trampoline+0x0/0x50 stack backtrace: CPU: 3 PID: 2767 Comm: sched-messaging Not tainted 5.6.0-rc6+ #6 Call Trace: dump_stack+0x96/0xe0 __lock_acquire.cold.57+0x173/0x2b7 ? native_queued_spin_lock_slowpath+0x42b/0x9e0 ? lockdep_hardirqs_on+0x590/0x590 ? __lock_acquire+0xf63/0x4030 lock_acquire+0x15a/0x3d0 ? kretprobe_hash_lock+0x52/0xa0 _raw_spin_lock_irqsave+0x36/0x70 ? kretprobe_hash_lock+0x52/0xa0 kretprobe_hash_lock+0x52/0xa0 trampoline_handler+0xf8/0x940 ? kprobe_fault_handler+0x380/0x380 ? find_held_lock+0x3a/0x1c0 kretprobe_trampoline+0x25/0x50 ? lock_acquired+0x392/0xbc0 ? _raw_spin_lock_irqsave+0x50/0x70 ? __get_valid_kprobe+0x1f0/0x1f0 ? _raw_spin_unlock_irqrestore+0x3b/0x40 ? finish_task_switch+0x4b9/0x6d0 ? __switch_to_asm+0x34/0x70 ? __switch_to_asm+0x40/0x70 The code within the kretprobe handler checks for probe reentrancy, so we won't trigger any _raw_spin_lock_irqsave probe in there. The problem is in outside kprobe_flush_task, where we call: kprobe_flush_task kretprobe_table_lock raw_spin_lock_irqsave _raw_spin_lock_irqsave where _raw_spin_lock_irqsave triggers the kretprobe and installs kretprobe_trampoline handler on _raw_spin_lock_irqsave return. The kretprobe_trampoline handler is then executed with already locked kretprobe_table_locks, and first thing it does is to lock kretprobe_table_locks ;-) the whole lockup path like: kprobe_flush_task kretprobe_table_lock raw_spin_lock_irqsave _raw_spin_lock_irqsave ---> probe triggered, kretprobe_trampoline installed ---> kretprobe_table_locks locked kretprobe_trampoline trampoline_handler kretprobe_hash_lock(current, &head, &flags); <--- deadlock Adding kprobe_busy_begin/end helpers that mark code with fake probe installed to prevent triggering of another kprobe within this code. Using these helpers in kprobe_flush_task, so the probe recursion protection check is hit and the probe is never set to prevent above lockup. Link: http://lkml.kernel.org/r/158927059835.27680.7011202830041561604.stgit@devnote2 Fixes:ef53d9c5e4
("kprobes: improve kretprobe scalability with hashed locking") Cc: Ingo Molnar <mingo@kernel.org> Cc: "Gustavo A . R . Silva" <gustavoars@kernel.org> Cc: Anders Roxell <anders.roxell@linaro.org> Cc: "Naveen N . Rao" <naveen.n.rao@linux.ibm.com> Cc: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com> Cc: David Miller <davem@davemloft.net> Cc: Ingo Molnar <mingo@elte.hu> Cc: Peter Zijlstra <peterz@infradead.org> Cc: stable@vger.kernel.org Reported-by: "Ziqian SUN (Zamir)" <zsun@redhat.com> Acked-by: Masami Hiramatsu <mhiramat@kernel.org> Signed-off-by: Jiri Olsa <jolsa@kernel.org> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org> Signed-off-by: Sasha Levin <sashal@kernel.org>
This commit is contained in:
parent
8a7737dd00
commit
62ce8f40b3
3 changed files with 31 additions and 13 deletions
|
@ -744,16 +744,11 @@ 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;
|
||||
|
@ -763,16 +758,12 @@ __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;
|
||||
kprobe_busy_begin();
|
||||
|
||||
INIT_HLIST_HEAD(&empty_rp);
|
||||
kretprobe_hash_lock(current, &head, &flags);
|
||||
|
@ -851,7 +842,7 @@ __visible __used void *trampoline_handler(struct pt_regs *regs)
|
|||
__this_cpu_write(current_kprobe, &ri->rp->kp);
|
||||
ri->ret_addr = correct_ret_addr;
|
||||
ri->rp->handler(ri, regs);
|
||||
__this_cpu_write(current_kprobe, &kretprobe_kprobe);
|
||||
__this_cpu_write(current_kprobe, &kprobe_busy);
|
||||
}
|
||||
|
||||
recycle_rp_inst(ri, &empty_rp);
|
||||
|
@ -867,8 +858,7 @@ __visible __used void *trampoline_handler(struct pt_regs *regs)
|
|||
|
||||
kretprobe_hash_unlock(current, &flags);
|
||||
|
||||
__this_cpu_write(current_kprobe, NULL);
|
||||
preempt_enable();
|
||||
kprobe_busy_end();
|
||||
|
||||
hlist_for_each_entry_safe(ri, tmp, &empty_rp, hlist) {
|
||||
hlist_del(&ri->hlist);
|
||||
|
|
|
@ -385,6 +385,10 @@ static inline struct kprobe_ctlblk *get_kprobe_ctlblk(void)
|
|||
return this_cpu_ptr(&kprobe_ctlblk);
|
||||
}
|
||||
|
||||
extern struct kprobe kprobe_busy;
|
||||
void kprobe_busy_begin(void);
|
||||
void kprobe_busy_end(void);
|
||||
|
||||
kprobe_opcode_t *kprobe_lookup_name(const char *name, unsigned int offset);
|
||||
int register_kprobe(struct kprobe *p);
|
||||
void unregister_kprobe(struct kprobe *p);
|
||||
|
|
|
@ -1218,6 +1218,26 @@ __releases(hlist_lock)
|
|||
}
|
||||
NOKPROBE_SYMBOL(kretprobe_table_unlock);
|
||||
|
||||
struct kprobe kprobe_busy = {
|
||||
.addr = (void *) get_kprobe,
|
||||
};
|
||||
|
||||
void kprobe_busy_begin(void)
|
||||
{
|
||||
struct kprobe_ctlblk *kcb;
|
||||
|
||||
preempt_disable();
|
||||
__this_cpu_write(current_kprobe, &kprobe_busy);
|
||||
kcb = get_kprobe_ctlblk();
|
||||
kcb->kprobe_status = KPROBE_HIT_ACTIVE;
|
||||
}
|
||||
|
||||
void kprobe_busy_end(void)
|
||||
{
|
||||
__this_cpu_write(current_kprobe, NULL);
|
||||
preempt_enable();
|
||||
}
|
||||
|
||||
/*
|
||||
* This function is called from finish_task_switch when task tk becomes dead,
|
||||
* so that we can recycle any function-return probe instances associated
|
||||
|
@ -1235,6 +1255,8 @@ void kprobe_flush_task(struct task_struct *tk)
|
|||
/* Early boot. kretprobe_table_locks not yet initialized. */
|
||||
return;
|
||||
|
||||
kprobe_busy_begin();
|
||||
|
||||
INIT_HLIST_HEAD(&empty_rp);
|
||||
hash = hash_ptr(tk, KPROBE_HASH_BITS);
|
||||
head = &kretprobe_inst_table[hash];
|
||||
|
@ -1248,6 +1270,8 @@ void kprobe_flush_task(struct task_struct *tk)
|
|||
hlist_del(&ri->hlist);
|
||||
kfree(ri);
|
||||
}
|
||||
|
||||
kprobe_busy_end();
|
||||
}
|
||||
NOKPROBE_SYMBOL(kprobe_flush_task);
|
||||
|
||||
|
|
Loading…
Reference in a new issue