* [PATCH bpf] bpf: trace: send signals asynchronously if !preemptible
@ 2025-01-15 10:36 Puranjay Mohan
2025-01-15 16:55 ` Yonghong Song
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Puranjay Mohan @ 2025-01-15 10:36 UTC (permalink / raw)
To: Song Liu, Jiri Olsa, KP Singh, Matt Bobrowski, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau,
Eduard Zingerman, Yonghong Song, John Fastabend,
Stanislav Fomichev, Hao Luo, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, bpf, linux-kernel, linux-trace-kernel,
puranjay12
BPF programs can execute in all kinds of contexts and when a program
running in a non-preemptible context uses the bpf_send_signal() kfunc,
it will cause issues because this kfunc can sleep.
So change `irqs_disabled()` to `!preemptible()` that covers all edge
cases: preempt_count() == 0 and irqs_disabled()
Reported-by: syzbot+97da3d7e0112d59971de@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/all/67486b09.050a0220.253251.0084.GAE@google.com/
Fixes: 1bc7896e9ef4 ("bpf: Fix deadlock with rq_lock in bpf_send_signal()")
Signed-off-by: Puranjay Mohan <puranjay@kernel.org>
---
kernel/trace/bpf_trace.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 1b8db5aee9d3..d162c87e09a8 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -853,7 +853,7 @@ static int bpf_send_signal_common(u32 sig, enum pid_type type, struct task_struc
if (unlikely(is_global_init(task)))
return -EPERM;
- if (irqs_disabled()) {
+ if (!preemptible()) {
/* Do an early check on signal validity. Otherwise,
* the error is lost in deferred irq_work.
*/
--
2.40.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH bpf] bpf: trace: send signals asynchronously if !preemptible
2025-01-15 10:36 [PATCH bpf] bpf: trace: send signals asynchronously if !preemptible Puranjay Mohan
@ 2025-01-15 16:55 ` Yonghong Song
2025-01-15 21:50 ` patchwork-bot+netdevbpf
2025-01-21 1:26 ` Hou Tao
2 siblings, 0 replies; 4+ messages in thread
From: Yonghong Song @ 2025-01-15 16:55 UTC (permalink / raw)
To: Puranjay Mohan, Song Liu, Jiri Olsa, KP Singh, Matt Bobrowski,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Eduard Zingerman, John Fastabend,
Stanislav Fomichev, Hao Luo, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, bpf, linux-kernel, linux-trace-kernel,
puranjay12
On 1/15/25 2:36 AM, Puranjay Mohan wrote:
> BPF programs can execute in all kinds of contexts and when a program
> running in a non-preemptible context uses the bpf_send_signal() kfunc,
> it will cause issues because this kfunc can sleep.
>
> So change `irqs_disabled()` to `!preemptible()` that covers all edge
> cases: preempt_count() == 0 and irqs_disabled()
>
> Reported-by: syzbot+97da3d7e0112d59971de@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/all/67486b09.050a0220.253251.0084.GAE@google.com/
> Fixes: 1bc7896e9ef4 ("bpf: Fix deadlock with rq_lock in bpf_send_signal()")
> Signed-off-by: Puranjay Mohan <puranjay@kernel.org>
Acked-by: Yonghong Song <yonghong.song@linux.dev>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH bpf] bpf: trace: send signals asynchronously if !preemptible
2025-01-15 10:36 [PATCH bpf] bpf: trace: send signals asynchronously if !preemptible Puranjay Mohan
2025-01-15 16:55 ` Yonghong Song
@ 2025-01-15 21:50 ` patchwork-bot+netdevbpf
2025-01-21 1:26 ` Hou Tao
2 siblings, 0 replies; 4+ messages in thread
From: patchwork-bot+netdevbpf @ 2025-01-15 21:50 UTC (permalink / raw)
To: Puranjay Mohan
Cc: song, jolsa, kpsingh, mattbobrowski, ast, daniel, andrii,
martin.lau, eddyz87, yonghong.song, john.fastabend, sdf, haoluo,
rostedt, mhiramat, mathieu.desnoyers, bpf, linux-kernel,
linux-trace-kernel, puranjay12
Hello:
This patch was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <ast@kernel.org>:
On Wed, 15 Jan 2025 10:36:47 +0000 you wrote:
> BPF programs can execute in all kinds of contexts and when a program
> running in a non-preemptible context uses the bpf_send_signal() kfunc,
> it will cause issues because this kfunc can sleep.
>
> So change `irqs_disabled()` to `!preemptible()` that covers all edge
> cases: preempt_count() == 0 and irqs_disabled()
>
> [...]
Here is the summary with links:
- [bpf] bpf: trace: send signals asynchronously if !preemptible
https://git.kernel.org/bpf/bpf-next/c/87c544108b61
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH bpf] bpf: trace: send signals asynchronously if !preemptible
2025-01-15 10:36 [PATCH bpf] bpf: trace: send signals asynchronously if !preemptible Puranjay Mohan
2025-01-15 16:55 ` Yonghong Song
2025-01-15 21:50 ` patchwork-bot+netdevbpf
@ 2025-01-21 1:26 ` Hou Tao
2 siblings, 0 replies; 4+ messages in thread
From: Hou Tao @ 2025-01-21 1:26 UTC (permalink / raw)
To: Puranjay Mohan, Alexei Starovoitov, Yonghong Song, bpf
Cc: Song Liu, Jiri Olsa, KP Singh, Matt Bobrowski, Daniel Borkmann,
Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman,
John Fastabend, Stanislav Fomichev, Hao Luo, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, linux-kernel,
linux-trace-kernel, puranjay12
Hi,
On 1/15/2025 6:36 PM, Puranjay Mohan wrote:
> BPF programs can execute in all kinds of contexts and when a program
> running in a non-preemptible context uses the bpf_send_signal() kfunc,
> it will cause issues because this kfunc can sleep.
>
> So change `irqs_disabled()` to `!preemptible()` that covers all edge
> cases: preempt_count() == 0 and irqs_disabled()
>
> Reported-by: syzbot+97da3d7e0112d59971de@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/all/67486b09.050a0220.253251.0084.GAE@google.com/
> Fixes: 1bc7896e9ef4 ("bpf: Fix deadlock with rq_lock in bpf_send_signal()")
> Signed-off-by: Puranjay Mohan <puranjay@kernel.org>
> ---
> kernel/trace/bpf_trace.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index 1b8db5aee9d3..d162c87e09a8 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -853,7 +853,7 @@ static int bpf_send_signal_common(u32 sig, enum pid_type type, struct task_struc
> if (unlikely(is_global_init(task)))
> return -EPERM;
>
> - if (irqs_disabled()) {
> + if (!preemptible()) {
Should we unfold preemptible() to "preempt_count() == 0 &&
!irqs_disabled()" instead, because when preemption is disabled,
preemptible() will evaluate as 0 and the irq_disabled() case will be
skipped ?
> /* Do an early check on signal validity. Otherwise,
> * the error is lost in deferred irq_work.
> */
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-01-21 1:26 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-15 10:36 [PATCH bpf] bpf: trace: send signals asynchronously if !preemptible Puranjay Mohan
2025-01-15 16:55 ` Yonghong Song
2025-01-15 21:50 ` patchwork-bot+netdevbpf
2025-01-21 1:26 ` Hou Tao
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).