bpf: Add crosstask check to __bpf_get_stack
authorJordan Rome <jordalgo@meta.com>
Wed, 8 Nov 2023 11:23:34 +0000 (03:23 -0800)
committerAndrii Nakryiko <andrii@kernel.org>
Fri, 10 Nov 2023 19:06:10 +0000 (11:06 -0800)
Currently get_perf_callchain only supports user stack walking for
the current task. Passing the correct *crosstask* param will return
0 frames if the task passed to __bpf_get_stack isn't the current
one instead of a single incorrect frame/address. This change
passes the correct *crosstask* param but also does a preemptive
check in __bpf_get_stack if the task is current and returns
-EOPNOTSUPP if it is not.

This issue was found using bpf_get_task_stack inside a BPF
iterator ("iter/task"), which iterates over all tasks.
bpf_get_task_stack works fine for fetching kernel stacks
but because get_perf_callchain relies on the caller to know
if the requested *task* is the current one (via *crosstask*)
it was failing in a confusing way.

It might be possible to get user stacks for all tasks utilizing
something like access_process_vm but that requires the bpf
program calling bpf_get_task_stack to be sleepable and would
therefore be a breaking change.

Fixes: fa28dcb82a38 ("bpf: Introduce helper bpf_get_task_stack()")
Signed-off-by: Jordan Rome <jordalgo@meta.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/bpf/20231108112334.3433136-1-jordalgo@meta.com
include/uapi/linux/bpf.h
kernel/bpf/stackmap.c
tools/include/uapi/linux/bpf.h

index 095ca72..7cf8bcf 100644 (file)
@@ -4517,6 +4517,8 @@ union bpf_attr {
  * long bpf_get_task_stack(struct task_struct *task, void *buf, u32 size, u64 flags)
  *     Description
  *             Return a user or a kernel stack in bpf program provided buffer.
+ *             Note: the user stack will only be populated if the *task* is
+ *             the current task; all other tasks will return -EOPNOTSUPP.
  *             To achieve this, the helper needs *task*, which is a valid
  *             pointer to **struct task_struct**. To store the stacktrace, the
  *             bpf program provides *buf* with a nonnegative *size*.
@@ -4528,6 +4530,7 @@ union bpf_attr {
  *
  *             **BPF_F_USER_STACK**
  *                     Collect a user space stack instead of a kernel stack.
+ *                     The *task* must be the current task.
  *             **BPF_F_USER_BUILD_ID**
  *                     Collect buildid+offset instead of ips for user stack,
  *                     only valid if **BPF_F_USER_STACK** is also specified.
index d6b2774..dff7ba5 100644 (file)
@@ -388,6 +388,7 @@ static long __bpf_get_stack(struct pt_regs *regs, struct task_struct *task,
 {
        u32 trace_nr, copy_len, elem_size, num_elem, max_depth;
        bool user_build_id = flags & BPF_F_USER_BUILD_ID;
+       bool crosstask = task && task != current;
        u32 skip = flags & BPF_F_SKIP_FIELD_MASK;
        bool user = flags & BPF_F_USER_STACK;
        struct perf_callchain_entry *trace;
@@ -410,6 +411,14 @@ static long __bpf_get_stack(struct pt_regs *regs, struct task_struct *task,
        if (task && user && !user_mode(regs))
                goto err_fault;
 
+       /* get_perf_callchain does not support crosstask user stack walking
+        * but returns an empty stack instead of NULL.
+        */
+       if (crosstask && user) {
+               err = -EOPNOTSUPP;
+               goto clear;
+       }
+
        num_elem = size / elem_size;
        max_depth = num_elem + skip;
        if (sysctl_perf_event_max_stack < max_depth)
@@ -421,7 +430,7 @@ static long __bpf_get_stack(struct pt_regs *regs, struct task_struct *task,
                trace = get_callchain_entry_for_task(task, max_depth);
        else
                trace = get_perf_callchain(regs, 0, kernel, user, max_depth,
-                                          false, false);
+                                          crosstask, false);
        if (unlikely(!trace))
                goto err_fault;
 
index 095ca72..7cf8bcf 100644 (file)
@@ -4517,6 +4517,8 @@ union bpf_attr {
  * long bpf_get_task_stack(struct task_struct *task, void *buf, u32 size, u64 flags)
  *     Description
  *             Return a user or a kernel stack in bpf program provided buffer.
+ *             Note: the user stack will only be populated if the *task* is
+ *             the current task; all other tasks will return -EOPNOTSUPP.
  *             To achieve this, the helper needs *task*, which is a valid
  *             pointer to **struct task_struct**. To store the stacktrace, the
  *             bpf program provides *buf* with a nonnegative *size*.
@@ -4528,6 +4530,7 @@ union bpf_attr {
  *
  *             **BPF_F_USER_STACK**
  *                     Collect a user space stack instead of a kernel stack.
+ *                     The *task* must be the current task.
  *             **BPF_F_USER_BUILD_ID**
  *                     Collect buildid+offset instead of ips for user stack,
  *                     only valid if **BPF_F_USER_STACK** is also specified.