Re: [PATCH bpf v2] bpf: guard uprobes against private-stack corruption

Andrii Nakryiko <[email protected]>
Newsgroups org.kernel.vger.stable,org.kernel.vger.bpf
Message-ID <CAEf4BzbkKwM-uwc1=o2p2v-bjuYNxM++La8Gr5edv5H6sGGqDA@mail.gmail.com>
On Tue, Aug 18, 2026 at 1:33 PM Christian Simon <[email protected]> wrote:
>
> Eligible BPF programs use one private stack per program and CPU. Both
> bpf_prog_run_array_uprobe() and uprobe_prog_run() use migrate_disable()
> to keep an invocation on one CPU, but another task can still preempt it
> and run the same program on that CPU. The second invocation then reuses
> and can overwrite the first invocation's private stack.
>
> Protect each real program invocation with the existing per-program
> recursion context. When the program is already active on this CPU,
> account for the missed invocation and skip it. Return zero when skipping
> an uprobe-multi invocation so session handling does not suppress its
> return probe.
>
> Skip dummy_bpf_prog in the classic array path before acquiring the
> recursion context because its active pointer is NULL.
>
> Add a regression test that pins two threads to one CPU and overlaps
> classic and multi uprobe invocations while preserving a sentinel in a
> private stack frame. Without the guards, the second invocation executes
> and corrupts the first invocation's sentinel.
>
> Fixes: 7d1cd70d4b16 ("bpf, x86: Support private stack in jit")
> Fixes: 6c17a882d380 ("bpf, arm64: JIT support for private stack")
> Closes: https://github.com/open-telemetry/opentelemetry-ebpf-instrumentation/issues/3056
> Cc: [email protected]
> Signed-off-by: Christian Simon <[email protected]>
> ---
> Changes in v2:
> - Address review comments from sashiko-bot
>   - Guard the uprobe-multi path as well.
>   - Remove const from correct line
> - Add a regression selftest for uprobe-classic/multi paths.
> - Add the arm64 Fixes tag.
>
>  include/linux/bpf.h                           |  33 +++--
>  kernel/trace/bpf_trace.c                      |  11 +-
>  .../bpf/prog_tests/uprobe_private_stack.c     | 118 ++++++++++++++++++
>  .../bpf/progs/uprobe_private_stack.c          |  54 ++++++++
>  4 files changed, 206 insertions(+), 10 deletions(-)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/uprobe_private_stack.c
>  create mode 100644 tools/testing/selftests/bpf/progs/uprobe_private_stack.c
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 7719f6528445..a94fc9898ece 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -2572,6 +2572,14 @@ static inline void bpf_reset_run_ctx(struct bpf_run_ctx *old_ctx)
>
>  typedef u32 (*bpf_prog_run_fn)(const struct bpf_prog *prog, const void *ctx);
>
> +#ifdef CONFIG_BPF_SYSCALL
> +void notrace bpf_prog_inc_misses_counter(struct bpf_prog *prog);
> +#else
> +static inline void bpf_prog_inc_misses_counter(struct bpf_prog *prog)
> +{
> +}
> +#endif
> +
>  static __always_inline u32
>  bpf_prog_run_array(const struct bpf_prog_array *array,
>                    const void *ctx, bpf_prog_run_fn run_prog)
> @@ -2617,7 +2625,7 @@ bpf_prog_run_array_uprobe(const struct bpf_prog_array *array,
>                           const void *ctx, bpf_prog_run_fn run_prog)
>  {
>         const struct bpf_prog_array_item *item;
> -       const struct bpf_prog *prog;
> +       struct bpf_prog *prog;
>         struct bpf_run_ctx *old_run_ctx;
>         struct bpf_trace_run_ctx run_ctx;
>         u32 ret = 1;
> @@ -2635,15 +2643,30 @@ bpf_prog_run_array_uprobe(const struct bpf_prog_array *array,
>         old_run_ctx = bpf_set_run_ctx(&run_ctx.run_ctx);
>         item = &array->items[0];
>         while ((prog = READ_ONCE(item->prog))) {
> +               /* dummy_bpf_prog has no recursion state. */
> +               if (unlikely(!prog->len)) {
> +                       item++;
> +                       continue;
> +               }
> +
> +               if (unlikely(!bpf_prog_get_recursion_context(prog))) {
> +                       bpf_prog_inc_misses_counter(prog);
> +                       bpf_prog_put_recursion_context(prog);
> +                       item++;
> +                       continue;
> +               }
> +

I think it's unacceptable to skip sleepable uprobe execution just
because there is the same BPF program attached to a *different* uprobe
(and all due to a private stack that no one asked for or needs for
uprobes, really).

As a short-term fix, we should probably disable private stack for
sleepable uprobe/kprobe program (and tracepoint/raw_tracepoint), and
think how we can make private stack less per-CPU dependent.

pw-bot: cr


>                 if (!prog->sleepable)
>                         rcu_read_lock();
>
>                 run_ctx.bpf_cookie = item->bpf_cookie;
>                 ret &= run_prog(prog, ctx);
> -               item++;
>
>                 if (!prog->sleepable)
>                         rcu_read_unlock();
> +
> +               bpf_prog_put_recursion_context(prog);
> +               item++;
>         }
>         bpf_reset_run_ctx(old_run_ctx);
>         migrate_enable();
> @@ -3208,8 +3231,6 @@ static inline bool has_current_bpf_ctx(void)
>         return !!current->bpf_ctx;
>  }
>
> -void notrace bpf_prog_inc_misses_counter(struct bpf_prog *prog);
> -
>  void bpf_dynptr_init(struct bpf_dynptr_kern *ptr, void *data,
>                      enum bpf_dynptr_type type, u32 offset, u32 size);
>  void bpf_dynptr_set_null(struct bpf_dynptr_kern *ptr);

[...]
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.