* [PATCH] uprobes: prevent mutex_lock() under rcu_read_lock()
@ 2024-05-21 5:30 Andrii Nakryiko
2024-05-21 14:23 ` Oleg Nesterov
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Andrii Nakryiko @ 2024-05-21 5:30 UTC (permalink / raw)
To: linux-trace-kernel, rostedt, mhiramat
Cc: bpf, oleg, jolsa, Andrii Nakryiko, Breno Leitao
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.
Fixes: 1b8f85defbc8 ("uprobes: prepare uprobe args buffer lazily")
Reported-by: Breno Leitao <leitao@debian.org>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
kernel/trace/trace_uprobe.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index 8541fa1494ae..c98e3b3386ba 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -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();
}
--
2.43.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] uprobes: prevent mutex_lock() under rcu_read_lock()
2024-05-21 5:30 [PATCH] uprobes: prevent mutex_lock() under rcu_read_lock() Andrii Nakryiko
@ 2024-05-21 14:23 ` Oleg Nesterov
2024-05-21 14:31 ` Breno Leitao
2024-05-23 23:45 ` Masami Hiramatsu
2 siblings, 0 replies; 4+ messages in thread
From: Oleg Nesterov @ 2024-05-21 14:23 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: linux-trace-kernel, rostedt, mhiramat, bpf, jolsa, Breno Leitao
On 05/20, Andrii Nakryiko wrote:
>
> Fixes: 1b8f85defbc8 ("uprobes: prepare uprobe args buffer lazily")
> Reported-by: Breno Leitao <leitao@debian.org>
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> ---
> kernel/trace/trace_uprobe.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
Reviewed-by: Oleg Nesterov <oleg@redhat.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] uprobes: prevent mutex_lock() under rcu_read_lock()
2024-05-21 5:30 [PATCH] uprobes: prevent mutex_lock() under rcu_read_lock() Andrii Nakryiko
2024-05-21 14:23 ` Oleg Nesterov
@ 2024-05-21 14:31 ` Breno Leitao
2024-05-23 23:45 ` Masami Hiramatsu
2 siblings, 0 replies; 4+ messages in thread
From: Breno Leitao @ 2024-05-21 14:31 UTC (permalink / raw)
To: Andrii Nakryiko; +Cc: linux-trace-kernel, rostedt, mhiramat, bpf, oleg, jolsa
On Mon, May 20, 2024 at 10:30:17PM -0700, Andrii Nakryiko wrote:
> 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.
>
> Fixes: 1b8f85defbc8 ("uprobes: prepare uprobe args buffer lazily")
> Reported-by: Breno Leitao <leitao@debian.org>
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Tested-by: Breno Leitao <leitao@debian.org>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] uprobes: prevent mutex_lock() under rcu_read_lock()
2024-05-21 5:30 [PATCH] uprobes: prevent mutex_lock() under rcu_read_lock() Andrii Nakryiko
2024-05-21 14:23 ` Oleg Nesterov
2024-05-21 14:31 ` Breno Leitao
@ 2024-05-23 23:45 ` Masami Hiramatsu
2 siblings, 0 replies; 4+ messages in thread
From: Masami Hiramatsu @ 2024-05-23 23:45 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: linux-trace-kernel, rostedt, bpf, oleg, jolsa, Breno Leitao
On Mon, 20 May 2024 22:30:17 -0700
Andrii Nakryiko <andrii@kernel.org> wrote:
> 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.
>
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,
> Fixes: 1b8f85defbc8 ("uprobes: prepare uprobe args buffer lazily")
> Reported-by: Breno Leitao <leitao@debian.org>
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> ---
> kernel/trace/trace_uprobe.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
> index 8541fa1494ae..c98e3b3386ba 100644
> --- a/kernel/trace/trace_uprobe.c
> +++ b/kernel/trace/trace_uprobe.c
> @@ -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();
> }
>
> --
> 2.43.0
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-05-23 23:45 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-21 5:30 [PATCH] uprobes: prevent mutex_lock() under rcu_read_lock() Andrii Nakryiko
2024-05-21 14:23 ` Oleg Nesterov
2024-05-21 14:31 ` Breno Leitao
2024-05-23 23:45 ` Masami Hiramatsu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).