From: Feng Yang <[email protected]>
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]>
---
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
