uprobes: prevent mutex_lock() under rcu_read_lock()
Andrii Nakryiko
maintainerPATCH 1/1 uprobes: prevent mutex_lock() under rcu_read_lock()
Recent changes made uprobe_cpu_buffer preparation lazy, and moved it deeper into __uprobe_trace_func(). This is problematic because __uprobe_trace_func() is called inside rcu_read_lock()/rcu_read_unlock() block, which then calls prepare_uprobe_buffer() -> uprobe_buffer_get() -> mutex_lock(&ucb->mutex), leading to a splat about using mutex under non-sleepable RCU:
BUG: sleeping function called from invalid context at kernel/locking/mutex.c:585
in_atomic(): 0, irqs_disabled(): 0, non_block: 0, pid: 98231, name: stress-ng-sigq
preempt_count: 0, expected: 0
RCU nest depth: 1, expected: 0
...
Call Trace:
<TASK>
dump_stack_lvl+0x3d/0xe0
__might_resched+0x24c/0x270
? prepare_uprobe_buffer+0xd5/0x1d0
__mutex_lock+0x41/0x820
? ___perf_sw_event+0x206/0x290
? __perf_event_task_sched_in+0x54/0x660
? __perf_event_task_sched_in+0x54/0x660
prepare_uprobe_buffer+0xd5/0x1d0
__uprobe_trace_func+0x4a/0x140
uprobe_dispatcher+0x135/0x280
? uprobe_dispatcher+0x94/0x280
uprobe_notify_resume+0x650/0xec0
? atomic_notifier_call_chain+0x21/0x110
? atomic_notifier_call_chain+0xf8/0x110
irqentry_exit_to_user_mode+0xe2/0x1e0
asm_exc_int3+0x35/0x40
RIP: 0033:0x7f7e1d4da390
Code: 33 04 00 0f 1f 80 00 00 00 00 f3 0f 1e fa b9 01 00 00 00 e9 b2 fc ff ff 66 90 f3 0f 1e fa 31 c9 e9 a5 fc ff ff 0f 1f 44 00 00 <cc> 0f 1e fa b8 27 00 00 00 0f 05 c3 0f 1f 40 00 f3 0f 1e fa b8 6e
RSP: 002b:00007ffd2abc3608 EFLAGS: 00000246
RAX: 0000000000000000 RBX: 0000000076d325f1 RCX: 0000000000000000
RDX: 0000000076d325f1 RSI: 000000000000000a RDI: 00007ffd2abc3690
RBP: 000000000000000a R08: 00017fb700000000 R09: 00017fb700000000
R10: 00017fb700000000 R11: 0000000000000246 R12: 0000000000017ff2
R13: 00007ffd2abc3610 R14: 0000000000000000 R15: 00007ffd2abc3780
</TASK>Luckily, it's easy to fix by moving prepare_uprobe_buffer() to be called slightly earlier: into uprobe_trace_func() and uretprobe_trace_func(), outside of RCU locked section. This still keeps this buffer preparation lazy and helps avoid the overhead when it's not needed. E.g., if there is only BPF uprobe handler installed on a given uprobe, buffer won't be initialized.
Note, the other user of prepare_uprobe_buffer(), __uprobe_perf_func(), is not affected, as it doesn't prepare buffer under RCU read lock.
--- kernel/trace/trace_uprobe.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)
kernel/trace/trace_uprobe.c +9 -5
@@ -970,19 +970,17 @@ static struct uprobe_cpu_buffer *prepare_uprobe_buffer(struct trace_uprobe *tu,static void __uprobe_trace_func (struct trace_uprobe *tu,unsigned long func,struct pt_regs *regs, -struct uprobe_cpu_buffer **ucbp, +struct uprobe_cpu_buffer *ucb,struct trace_event_file *trace_file) {struct uprobe_trace_entry_head *entry;struct trace_event_buffer fbuffer; -struct uprobe_cpu_buffer *ucb;void *data;int size, esize;struct trace_event_call *call =trace_probe_event_call (&tu->tp);WARN_ON (call != trace_file->event_call); - ucb =prepare_uprobe_buffer (tu, regs, ucbp);if (WARN_ON_ONCE (ucb->dsize >PAGE_SIZE ))return ;
@@ -1014,13 +1012,16 @@ static int uprobe_trace_func(struct trace_uprobe *tu, struct pt_regs *regs, struct uprobe_cpu_buffer **ucbp) {struct event_file_link *link; +struct uprobe_cpu_buffer *ucb;if (is_ret_probe (tu))return 0 ; + ucb =prepare_uprobe_buffer (tu, regs, ucbp); +rcu_read_lock ();trace_probe_for_each_link_rcu (link, &tu->tp) -__uprobe_trace_func (tu,0 , regs, ucbp, link->file); +__uprobe_trace_func (tu,0 , regs, ucb, link->file);rcu_read_unlock ();return 0 ;
@@ -1031,10 +1032,13 @@ static void uretprobe_trace_func(struct trace_uprobe *tu, unsigned long func, struct uprobe_cpu_buffer **ucbp) {struct event_file_link *link; +struct uprobe_cpu_buffer *ucb; + + ucb =prepare_uprobe_buffer (tu, regs, ucbp);rcu_read_lock ();trace_probe_for_each_link_rcu (link, &tu->tp) -__uprobe_trace_func (tu, func, regs, ucbp, link->file); +__uprobe_trace_func (tu, func, regs, ucb, link->file);rcu_read_unlock (); }
Signature · 1 line
2.43.0
Quoted 7 lines · Andrii Nakryiko
Fixes 1b8f85defbc8 ("uprobes: prepare uprobe args buffer lazily")Reported-by Breno Leitao <[email protected]>Signed-off-by Andrii Nakryiko <[email protected]>--- kernel/trace/trace_uprobe.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)
Quoted 46 lines · Andrii Nakryiko
Recent changes made
uprobe_cpu_bufferpreparation lazy, and moved it deeper into__uprobe_trace_func(). This is problematic because__uprobe_trace_func()is called inside rcu_read_lock()/rcu_read_unlock() block, which then callsprepare_uprobe_buffer()->uprobe_buffer_get()->mutex_lock(&ucb->mutex), leading to a splat about using mutex under non-sleepable RCU:BUG: sleeping function called from invalid context at kernel/locking/mutex.c:585 in_atomic(): 0, irqs_disabled(): 0, non_block: 0, pid: 98231, name: stress-ng-sigq preempt_count: 0, expected: 0 RCU nest depth: 1, expected: 0 ... Call Trace: <TASK> dump_stack_lvl+0x3d/0xe0 __might_resched+0x24c/0x270 ? prepare_uprobe_buffer+0xd5/0x1d0 __mutex_lock+0x41/0x820 ? ___perf_sw_event+0x206/0x290 ? __perf_event_task_sched_in+0x54/0x660 ? __perf_event_task_sched_in+0x54/0x660 prepare_uprobe_buffer+0xd5/0x1d0 __uprobe_trace_func+0x4a/0x140 uprobe_dispatcher+0x135/0x280 ? uprobe_dispatcher+0x94/0x280 uprobe_notify_resume+0x650/0xec0 ? atomic_notifier_call_chain+0x21/0x110 ? atomic_notifier_call_chain+0xf8/0x110 irqentry_exit_to_user_mode+0xe2/0x1e0 asm_exc_int3+0x35/0x40 RIP: 0033:0x7f7e1d4da390 Code: 33 04 00 0f 1f 80 00 00 00 00 f3 0f 1e fa b9 01 00 00 00 e9 b2 fc ff ff 66 90 f3 0f 1e fa 31 c9 e9 a5 fc ff ff 0f 1f 44 00 00 <cc> 0f 1e fa b8 27 00 00 00 0f 05 c3 0f 1f 40 00 f3 0f 1e fa b8 6e RSP: 002b:00007ffd2abc3608 EFLAGS: 00000246 RAX: 0000000000000000 RBX: 0000000076d325f1 RCX: 0000000000000000 RDX: 0000000076d325f1 RSI: 000000000000000a RDI: 00007ffd2abc3690 RBP: 000000000000000a R08: 00017fb700000000 R09: 00017fb700000000 R10: 00017fb700000000 R11: 0000000000000246 R12: 0000000000017ff2 R13: 00007ffd2abc3610 R14: 0000000000000000 R15: 00007ffd2abc3780 </TASK>Luckily, it's easy to fix by moving
prepare_uprobe_buffer()to be called slightly earlier: intouprobe_trace_func()anduretprobe_trace_func(), outside of RCU locked section. This still keeps this buffer preparation lazy and helps avoid the overhead when it's not needed. E.g., if there is only BPF uprobe handler installed on a given uprobe, buffer won't be initialized.
Quoted 6 lines
Note, the other user of
prepare_uprobe_buffer(),__uprobe_perf_func(), is not affected, as it doesn't prepare buffer under RCU read lock.Fixes 1b8f85defbc8 ("uprobes: prepare uprobe args buffer lazily")Reported-by Breno Leitao <[email protected]>Signed-off-by Andrii Nakryiko <[email protected]>
Masami Hiramatsu (Google)
maintainerQuoted 40 lines · On Mon, 20 May 2024 22:30:17 -0700 Andrii Nakryiko <[email protected]> wrote:
Recent changes made
uprobe_cpu_bufferpreparation lazy, and moved it deeper into__uprobe_trace_func(). This is problematic because__uprobe_trace_func()is called inside rcu_read_lock()/rcu_read_unlock() block, which then callsprepare_uprobe_buffer()->uprobe_buffer_get()->mutex_lock(&ucb->mutex), leading to a splat about using mutex under non-sleepable RCU:BUG: sleeping function called from invalid context at kernel/locking/mutex.c:585 in_atomic(): 0, irqs_disabled(): 0, non_block: 0, pid: 98231, name: stress-ng-sigq preempt_count: 0, expected: 0 RCU nest depth: 1, expected: 0 ... Call Trace: <TASK> dump_stack_lvl+0x3d/0xe0 __might_resched+0x24c/0x270 ? prepare_uprobe_buffer+0xd5/0x1d0 __mutex_lock+0x41/0x820 ? ___perf_sw_event+0x206/0x290 ? __perf_event_task_sched_in+0x54/0x660 ? __perf_event_task_sched_in+0x54/0x660 prepare_uprobe_buffer+0xd5/0x1d0 __uprobe_trace_func+0x4a/0x140 uprobe_dispatcher+0x135/0x280 ? uprobe_dispatcher+0x94/0x280 uprobe_notify_resume+0x650/0xec0 ? atomic_notifier_call_chain+0x21/0x110 ? atomic_notifier_call_chain+0xf8/0x110 irqentry_exit_to_user_mode+0xe2/0x1e0 asm_exc_int3+0x35/0x40 RIP: 0033:0x7f7e1d4da390 Code: 33 04 00 0f 1f 80 00 00 00 00 f3 0f 1e fa b9 01 00 00 00 e9 b2 fc ff ff 66 90 f3 0f 1e fa 31 c9 e9 a5 fc ff ff 0f 1f 44 00 00 <cc> 0f 1e fa b8 27 00 00 00 0f 05 c3 0f 1f 40 00 f3 0f 1e fa b8 6e RSP: 002b:00007ffd2abc3608 EFLAGS: 00000246 RAX: 0000000000000000 RBX: 0000000076d325f1 RCX: 0000000000000000 RDX: 0000000076d325f1 RSI: 000000000000000a RDI: 00007ffd2abc3690 RBP: 000000000000000a R08: 00017fb700000000 R09: 00017fb700000000 R10: 00017fb700000000 R11: 0000000000000246 R12: 0000000000017ff2 R13: 00007ffd2abc3610 R14: 0000000000000000 R15: 00007ffd2abc3780 </TASK>
Quoted 9 lines
Luckily, it's easy to fix by moving
prepare_uprobe_buffer()to be called slightly earlier: intouprobe_trace_func()anduretprobe_trace_func(), outside of RCU locked section. This still keeps this buffer preparation lazy and helps avoid the overhead when it's not needed. E.g., if there is only BPF uprobe handler installed on a given uprobe, buffer won't be initialized.Note, the other user of
prepare_uprobe_buffer(),__uprobe_perf_func(), is not affected, as it doesn't prepare buffer under RCU read lock.
Oops, good catch! This looks good to me. Let me pick it. Let me add a simple uprobe test in ftracetest so that this error can detect in selftests. (I could reproduced it.)
Thank you,
Quoted 68 lines
Fixes 1b8f85defbc8 ("uprobes: prepare uprobe args buffer lazily")Reported-by Breno Leitao <[email protected]>Signed-off-by Andrii Nakryiko <[email protected]>--- kernel/trace/trace_uprobe.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)kernel/trace/trace_uprobe.c +9
-5@@ -970,19 +970,17 @@ static struct uprobe_cpu_buffer *prepare_uprobe_buffer(struct trace_uprobe *tu,static void __uprobe_trace_func (struct trace_uprobe *tu,unsigned long func,struct pt_regs *regs, -struct uprobe_cpu_buffer **ucbp, +struct uprobe_cpu_buffer *ucb,struct trace_event_file *trace_file) {struct uprobe_trace_entry_head *entry;struct trace_event_buffer fbuffer; -struct uprobe_cpu_buffer *ucb;void *data;int size, esize;struct trace_event_call *call =trace_probe_event_call (&tu->tp);WARN_ON (call != trace_file->event_call); - ucb =prepare_uprobe_buffer (tu, regs, ucbp);if (WARN_ON_ONCE (ucb->dsize >PAGE_SIZE ))return ;@@ -1014,13 +1012,16 @@ static int uprobe_trace_func(struct trace_uprobe *tu, struct pt_regs *regs, struct uprobe_cpu_buffer **ucbp) {struct event_file_link *link; +struct uprobe_cpu_buffer *ucb;if (is_ret_probe (tu))return 0 ; + ucb =prepare_uprobe_buffer (tu, regs, ucbp); +rcu_read_lock ();trace_probe_for_each_link_rcu (link, &tu->tp) -__uprobe_trace_func (tu,0 , regs, ucbp, link->file); +__uprobe_trace_func (tu,0 , regs, ucb, link->file);rcu_read_unlock ();return 0 ;@@ -1031,10 +1032,13 @@ static void uretprobe_trace_func(struct trace_uprobe *tu, unsigned long func, struct uprobe_cpu_buffer **ucbp) {struct event_file_link *link; +struct uprobe_cpu_buffer *ucb; + + ucb =prepare_uprobe_buffer (tu, regs, ucbp);rcu_read_lock ();trace_probe_for_each_link_rcu (link, &tu->tp) -__uprobe_trace_func (tu, func, regs, ucbp, link->file); +__uprobe_trace_func (tu, func, regs, ucb, link->file);rcu_read_unlock (); }Signature · 1 line
2.43.0
Signature · 1 line
Masami Hiramatsu (Google) <[email protected]>