> bpf: Fix suspicious RCU usage for sleepable bpf_get_stack()

TL;DR: Commit message only: it understates scope. Sleepable LSM and SYSCALL
programs also get the might_sleep bpf_get_stack(), so calls in
preempt/IRQ-disabled or lock regions are rejected too. Code matches existing
policy.

> bpf: Fix suspicious RCU usage for sleepable bpf_get_stack()
>
> 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 <[email protected]>
> Reported-by: Yinhao Hu <[email protected]>
> Closes: 
> https://lore.kernel.org/all/[email protected]/
> Signed-off-by: Feng Yang <[email protected]>

The commit message lists tracing_prog_func_proto() (iterators,
fentry/fexit/fmod_ret, BPF_TRACE_RAW_TP) and BPF_PROG_TYPE_TRACEPOINT as the
affected paths, and says only that calls from inside a bpf_rcu_read_lock()
region are now rejected.  Is that the full scope of the behaviour change?

The raw_tp_prog_func_proto() change reaches more program types than that:

  - BPF_PROG_TYPE_RAW_TRACEPOINT uses it directly, via
    raw_tracepoint_verifier_ops.get_func_proto.

  - BPF_PROG_TYPE_LSM reaches it through the default case of
    bpf_lsm_func_proto() -> tracing_prog_func_proto().

  - BPF_PROG_TYPE_SYSCALL, which is always sleepable, reaches it through the
    default case of syscall_prog_func_proto() -> tracing_prog_func_proto().

For each of these, a sleepable program's BPF_FUNC_get_stack now resolves to
bpf_get_stack_proto_raw_tp_sleepable, which has .might_sleep = true.
check_helper_call() will then reject the call wherever in_sleepable_context()
is false:

  sleepable helper bpf_get_stack#67 in rcu_read_lock region

or the equivalent non-preemptible, IRQ-disabled or lock region variants.
Before the patch these programs loaded and ran, since the old helper never
slept.

The code change itself matches the policy already applied to uprobe programs
and bpf_get_task_stack() by commit 9d02927fdf4e ("bpf: Mark faultable stack
helpers as sleepable"), so only the description of the user-visible scope
looks incomplete.

Could the commit message mention that sleepable LSM and BPF_PROG_TYPE_SYSCALL
programs are affected as well, and that calls from preempt-disabled,
IRQ-disabled and lock-held regions are rejected too, not only calls inside a
bpf_rcu_read_lock() region?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/41409077371

Reply via email to