Linux Trace Kernel
 help / color / mirror / Atom feed
* [PATCH bpf-next] bpf: Fix suspicious RCU usage for sleepable bpf_get_stack()
@ 2026-10-28  9:35 Feng Yang
  2026-10-28  9:59 ` sashiko-bot
  2026-10-28 12:51 ` Songsen Yang
  0 siblings, 2 replies; 3+ messages in thread
From: Feng Yang @ 2026-10-28  9:35 UTC (permalink / raw)
  To: song, jolsa, ihor.solodrai, kpsingh, matt, ast, daniel, andrii,
	eddyz87, memxor, martin.lau, yonghong.song, emil, rostedt,
	mhiramat, mathieu.desnoyers
  Cc: bpf, linux-trace-kernel

From: Feng Yang <yangfeng@kylinos.cn>

Sleepable BPF programs run under rcu_read_lock_trace(), not classic RCU.
bpf_get_stack() relies on the caller holding a classic RCU read lock:
get_perf_callchain() -> get_callchain_entry() dereferences
callchain_cpus_entries, and the returned entry is read until
callchain_store().  __bpf_get_stack() takes that lock only for the
sleepable variant, i.e. when may_fault is true.

The sleepable variant was added by commit d4dd9775ec24 ("bpf: wire up
sleepable bpf_get_stack() and bpf_get_task_stack() helpers") but only
wired up for kprobe/uprobe.  All other program types that can be
sleepable still get the non-sleepable variant:

 - tracing_prog_func_proto() serves BPF_TRACE_ITER, fentry/fexit/fmod_ret
   and BPF_TRACE_RAW_TP programs, and falls back to
   raw_tp_prog_func_proto() for BPF_FUNC_get_stack, which always returns
   bpf_get_stack_proto_raw_tp.  Sleepable iterators have been allowed
   since commit cf83b2d2e2b6 ("bpf: Permit cond_resched for some
   iterators"), e.g. for the task_file target.

 - tp_prog_func_proto() serves BPF_PROG_TYPE_TRACEPOINT programs and
   always returns bpf_get_stack_proto_tp.  Sleepable programs of that
   type are allowed on syscall tracepoints since commit 8cfb77d30920
   ("bpf: Verifier support for sleepable tracepoint programs"); they run
   through trace_call_bpf_faultable() and bpf_prog_run_array_sleepable(),
   which hold rcu_tasks_trace but no classic RCU read lock.

So a sleepable program calling bpf_get_stack() triggers:

  WARNING: suspicious RCU usage
  suspicious rcu_dereference_check() usage!

Route these paths to bpf_get_stack_sleepable() and mark the new protos
might_sleep, since build_id_parse() may fault in file data for
BPF_F_USER_STACK | BPF_F_USER_BUILD_ID.  Sleepable programs that call
bpf_get_stack() from inside a bpf_rcu_read_lock() region are now rejected
by the verifier, which is the correct behavior for a helper that can
sleep.

Reported-by: Kaiyan Mei <M202472210@hust.edu.cn>
Reported-by: Yinhao Hu <dddddd@hust.edu.cn>
Closes: https://lore.kernel.org/all/41ce77bf.17656.1a17b3fcb04.Coremail.kaiyanm@hust.edu.cn/
Signed-off-by: Feng Yang <yangfeng@kylinos.cn>
---
 kernel/trace/bpf_trace.c | 54 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 52 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index dfac5e1b8f40..8cfc9a6542f2 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -84,6 +84,7 @@ static struct bpf_raw_event_map *bpf_get_raw_tracepoint_module(const char *name)
 
 u64 bpf_get_stackid(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5);
 u64 bpf_get_stack(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5);
+u64 bpf_get_stack_sleepable(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5);
 
 static int bpf_btf_printf_prepare(struct btf_ptr *ptr, u32 btf_ptr_size,
 				  u64 flags, const struct btf **btf,
@@ -1469,6 +1470,26 @@ static const struct bpf_func_proto bpf_get_stack_proto_tp = {
 	.arg4_type	= ARG_ANYTHING,
 };
 
+BPF_CALL_4(bpf_get_stack_tp_sleepable, void *, tp_buff, void *, buf, u32, size,
+	   u64, flags)
+{
+	struct pt_regs *regs = *(struct pt_regs **)tp_buff;
+
+	return bpf_get_stack_sleepable((unsigned long) regs, (unsigned long) buf,
+				       (unsigned long) size, flags, 0);
+}
+
+static const struct bpf_func_proto bpf_get_stack_proto_tp_sleepable = {
+	.func		= bpf_get_stack_tp_sleepable,
+	.gpl_only	= true,
+	.might_sleep	= true,
+	.ret_type	= RET_INTEGER,
+	.arg1_type	= ARG_PTR_TO_CTX,
+	.arg2_type	= ARG_PTR_TO_UNINIT_MEM,
+	.arg3_type	= ARG_MEM_SIZE_OR_ZERO,
+	.arg4_type	= ARG_ANYTHING,
+};
+
 static const struct bpf_func_proto *
 tp_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 {
@@ -1478,7 +1499,8 @@ tp_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 	case BPF_FUNC_get_stackid:
 		return &bpf_get_stackid_proto_tp;
 	case BPF_FUNC_get_stack:
-		return &bpf_get_stack_proto_tp;
+		return prog->sleepable ? &bpf_get_stack_proto_tp_sleepable
+				       : &bpf_get_stack_proto_tp;
 	case BPF_FUNC_get_attach_cookie:
 		return &bpf_get_attach_cookie_proto_trace;
 	default:
@@ -1708,6 +1730,33 @@ static const struct bpf_func_proto bpf_get_stack_proto_raw_tp = {
 	.arg4_type	= ARG_ANYTHING,
 };
 
+BPF_CALL_4(bpf_get_stack_raw_tp_sleepable, struct bpf_raw_tracepoint_args *, args,
+	   void *, buf, u32, size, u64, flags)
+{
+	struct pt_regs *regs = get_bpf_raw_tp_regs();
+	int ret;
+
+	if (IS_ERR(regs))
+		return PTR_ERR(regs);
+
+	perf_fetch_caller_regs(regs);
+	ret = bpf_get_stack_sleepable((unsigned long) regs, (unsigned long) buf,
+				      (unsigned long) size, flags, 0);
+	put_bpf_raw_tp_regs();
+	return ret;
+}
+
+static const struct bpf_func_proto bpf_get_stack_proto_raw_tp_sleepable = {
+	.func		= bpf_get_stack_raw_tp_sleepable,
+	.gpl_only	= true,
+	.might_sleep	= true,
+	.ret_type	= RET_INTEGER,
+	.arg1_type	= ARG_PTR_TO_CTX,
+	.arg2_type	= ARG_PTR_TO_UNINIT_MEM,
+	.arg3_type	= ARG_MEM_SIZE_OR_ZERO,
+	.arg4_type	= ARG_ANYTHING,
+};
+
 static const struct bpf_func_proto *
 raw_tp_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 {
@@ -1717,7 +1766,8 @@ raw_tp_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 	case BPF_FUNC_get_stackid:
 		return &bpf_get_stackid_proto_raw_tp;
 	case BPF_FUNC_get_stack:
-		return &bpf_get_stack_proto_raw_tp;
+		return prog->sleepable ? &bpf_get_stack_proto_raw_tp_sleepable
+				       : &bpf_get_stack_proto_raw_tp;
 	case BPF_FUNC_get_attach_cookie:
 		return &bpf_get_attach_cookie_proto_tracing;
 	default:
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH bpf-next] bpf: Fix suspicious RCU usage for sleepable bpf_get_stack()
  2026-10-28  9:35 [PATCH bpf-next] bpf: Fix suspicious RCU usage for sleepable bpf_get_stack() Feng Yang
@ 2026-10-28  9:59 ` sashiko-bot
  2026-10-28 12:51 ` Songsen Yang
  1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-10-28  9:59 UTC (permalink / raw)
  To: Feng Yang; +Cc: bpf, linux-trace-kernel

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] bpf: per-CPU bpf_raw_tp_regs corruption in bpf_get_stack_raw_tp_sleepable()
--

commit c66538ad7075b69839a1969f72a7ea7eff53e112
Author: Feng Yang <yangfeng@kylinos.cn>

bpf: Fix suspicious RCU usage for sleepable bpf_get_stack()

This patch routes sleepable tracepoint and raw tracepoint/tracing BPF
programs calling bpf_get_stack() to bpf_get_stack_sleepable() so that
classic RCU read lock is held when accessing callchain entries and the
helper is marked might_sleep.

> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index 14adfb85fdb2b..7887d1c00be83 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
[ ... ]
> @@ -1705,6 +1727,33 @@ static const struct bpf_func_proto bpf_get_stack_proto_raw_tp = {
>  	.arg4_type	= ARG_ANYTHING,
>  };
>  
> +BPF_CALL_4(bpf_get_stack_raw_tp_sleepable, struct bpf_raw_tracepoint_args *, args,
> +	   void *, buf, u32, size, u64, flags)
> +{
> +	struct pt_regs *regs = get_bpf_raw_tp_regs();
> +	int ret;
> +
> +	if (IS_ERR(regs))
> +		return PTR_ERR(regs);
> +
> +	perf_fetch_caller_regs(regs);
> +	ret = bpf_get_stack_sleepable((unsigned long) regs, (unsigned long) buf,
> +				      (unsigned long) size, flags, 0);
> +	put_bpf_raw_tp_regs();
> +	return ret;
> +}

[Severity: High]
Can holding the per-CPU bpf_raw_tp_regs slot across bpf_get_stack_sleepable()
corrupt active register state or exhaust the per-CPU slots on the local CPU?

In kernel/trace/bpf_trace.c, get_bpf_raw_tp_regs() and put_bpf_raw_tp_regs()
manage a 3-slot per-CPU array indexed as a LIFO stack via
bpf_raw_tp_nest_level:

    static struct pt_regs *get_bpf_raw_tp_regs(void)
    {
        struct bpf_raw_tp_regs *tp_regs = this_cpu_ptr(&bpf_raw_tp_regs);
        int nest_level = this_cpu_inc_return(bpf_raw_tp_nest_level);

        if (nest_level > ARRAY_SIZE(tp_regs->regs)) {
            this_cpu_dec(bpf_raw_tp_nest_level);
            return ERR_PTR(-EBUSY);
        }

        return &tp_regs->regs[nest_level - 1];
    }

    static void put_bpf_raw_tp_regs(void)
    {
        this_cpu_dec(bpf_raw_tp_nest_level);
    }

Sleepable BPF programs using raw_tp_prog_func_proto() or
tracing_prog_func_proto() run under rcu_read_lock_trace() and
migrate_disable() with preemption enabled. When such a program calls
bpf_get_stack_raw_tp_sleepable() with BPF_F_USER_STACK | BPF_F_USER_BUILD_ID,
bpf_get_stack_sleepable() -> __bpf_get_stack() only disables preemption around
get_perf_callchain() and callchain_store(), and then re-enables preemption
before calling callchain_finalize():

    /* trace should not be dereferenced after this point */
    if (may_fault)
        rcu_read_unlock();
    preempt_enable();

    return callchain_finalize(buf, size, trace_nr, elem_size, flags, may_fault);

Inside callchain_finalize() -> stack_map_get_build_id_offset_sleepable() ->
build_id_parse_file(), the task can sleep on page faults or filesystem reads
while still holding the per-CPU bpf_raw_tp_regs slot.

If task A on a CPU acquires regs[0] (nest_level becomes 1) and sleeps or is
preempted inside callchain_finalize(), task B on the same CPU can call
get_bpf_raw_tp_regs(), acquire regs[1] (nest_level becomes 2), populate
regs[1] via perf_fetch_caller_regs(), and get preempted before
__bpf_get_stack() disables preemption. If task A then wakes up on the same
CPU and calls put_bpf_raw_tp_regs(), bpf_raw_tp_nest_level is decremented
from 2 to 1 while task B is still about to use regs[1]. Any subsequent caller
on that CPU (another task, or an IRQ/NMI raw tracepoint program calling
bpf_get_stack_raw_tp(), bpf_get_stackid_raw_tp(), or
bpf_perf_event_output_raw_tp()) will increment bpf_raw_tp_nest_level back to
2, receive the same &tp_regs->regs[1] pointer, and overwrite task B's saved
registers before task B runs get_perf_callchain().

In addition, if three sleepable tasks on the same CPU block concurrently in
build_id_parse_file(), bpf_raw_tp_nest_level stays at 3 for the duration of
the I/O, causing all get_bpf_raw_tp_regs() calls on that CPU across task,
IRQ, and NMI contexts to fail with -EBUSY.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261028093548.790977-1-yangfeng59949@163.com?part=1

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH bpf-next] bpf: Fix suspicious RCU usage for sleepable bpf_get_stack()
  2026-10-28  9:35 [PATCH bpf-next] bpf: Fix suspicious RCU usage for sleepable bpf_get_stack() Feng Yang
  2026-10-28  9:59 ` sashiko-bot
@ 2026-10-28 12:51 ` Songsen Yang
  1 sibling, 0 replies; 3+ messages in thread
From: Songsen Yang @ 2026-10-28 12:51 UTC (permalink / raw)
  To: Feng Yang
  Cc: song, jolsa, ihor.solodrai, kpsingh, matt, ast, daniel, andrii,
	eddyz87, memxor, martin.lau, yonghong.song, emil, rostedt,
	mhiramat, mathieu.desnoyers, bpf, linux-trace-kernel

On Wed, Oct 28, 2026 at 05:35:48PM +0800, Feng Yang wrote:
> Sleepable BPF programs run under rcu_read_lock_trace(), not classic RCU.
> bpf_get_stack() relies on the caller holding a classic RCU read lock:
> get_perf_callchain() -> get_callchain_entry() dereferences
> callchain_cpus_entries, and the returned entry is read until
> callchain_store().  __bpf_get_stack() takes that lock only for the
> sleepable variant, i.e. when may_fault is true.

Hi Feng,

I hit the same root cause independently via syzbot and sent a fix a
few hours after yours, without having seen this patch -- sorry for the
overlap:

  https://lore.kernel.org/bpf/20261028121859.994-1-yangsongsen@insta360.com/

It also covers the syzbot report for BPF_PROG_TYPE_SYSCALL programs
(always sleepable, run via bpf_prog_test_run_syscall() under
rcu_read_lock_trace() only), which reach bpf_get_stack_raw_tp() through
tracing_prog_func_proto():

  https://syzkaller.appspot.com/bug?extid=72a43cdb78469f7fbad1

The approach is different: instead of routing sleepable programs to new
sleepable protos, it simply takes rcu_read_lock() unconditionally in
__bpf_get_stack() around get_perf_callchain()/callchain_store(), as
Andrii suggested earlier that an extra nested rcu_read_lock() is
harmless:

  https://lore.kernel.org/all/CAEf4BzYN33yQhO-tsF8pUWDMZoiLAXkV+Z1oP6fJkByocYcjpw@mail.gmail.com/

That is a 3+/6- change in kernel/bpf/stackmap.c, it covers every caller
of __bpf_get_stack() (including the tracepoint, raw_tp, iter and syscall
cases listed above), and since no helper gains might_sleep it does not
hold the per-CPU bpf_raw_tp_regs slot across a possible sleep in
callchain_finalize(), which is the issue the Sashiko review raised here.

It does not do what your patch additionally does, i.e. allow
BPF_F_USER_BUILD_ID to fault in build IDs from these program types.

I'm happy to go with whichever the maintainers prefer, or to drop mine.

Thanks,
Songsen

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-10-28 12:52 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-28  9:35 [PATCH bpf-next] bpf: Fix suspicious RCU usage for sleepable bpf_get_stack() Feng Yang
2026-10-28  9:59 ` sashiko-bot
2026-10-28 12:51 ` Songsen Yang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox