BPF List
 help / color / mirror / Atom feed
From: Jiri Olsa <olsajiri@gmail.com>
To: HyeongJun An <sammiee5311@gmail.com>
Cc: olsajiri@gmail.com, bpf@vger.kernel.org, simon@swine.de,
	ast@kernel.org, andrii@kernel.org, daniel@iogearbox.net,
	martin.lau@kernel.org, tj@kernel.org, yonghong.song@linux.dev,
	stable@vger.kernel.org, andrii.nakryiko@gmail.com,
	alexei.starovoitov@gmail.com
Subject: Re: [PATCH bpf v3 1/2] bpf: disable private stack for sleepable programs
Date: Sat, 31 Oct 2026 00:01:02 +0100	[thread overview]
Message-ID: <auUhrnOOUWY3Mb9y@krava> (raw)
In-Reply-To: <20261030012258.3005240-1-sammiee5311@gmail.com>

On Fri, Oct 30, 2026 at 10:22:58AM +0900, HyeongJun An wrote:
> On Fri, Aug 28, 2026 at 12:32:27AM +0200, Jiri Olsa wrote:
> > > my understanding is that preemptable kernel doesn't mean that
> > > bpf prog can be preempted by user space.
> > > only by kernel.
> [...]
> > hum.. non-sleepable uprobe prog is run by bpf_prog_run_array_uprobe
> > and it disables only task migration, preemption is not disabled and
> > holds rcu_read_lock (which seems ok for preemption)
> > so I'm not sure why it wouldn't be preemptible by another task
> 
> It is preemptible, and the private stack gets corrupted when it happens.
> We hit this in a uprobe/uretprobe based tracer and reduced it to a small
> reproducer.
> 
> The second writer is not user space. The scheduler preempts the task
> while it is inside the program. migrate_disable() keeps that task on its
> CPU, but the CPU can still run another task. That task hits the same
> uprobe, runs the same program and writes its frame into the same per-CPU
> private stack. When the first task resumes, its frame holds the second
> task's values. Neither uprobe run path disables preemption:
> 
>   bpf_prog_run_array_uprobe()  migrate_disable(), plus rcu_read_lock()
>                                for non-sleepable programs
>   uprobe_prog_run() (multi)    migrate_disable(), plus rcu_read_lock()
>                                or rcu_read_lock_trace()
> 
> With PREEMPT_RCU, rcu_read_lock() does not disable preemption.
> 
> bpf_enable_priv_stack() gives a private stack to every
> BPF_PROG_TYPE_KPROBE program with at least BPF_PRIV_STACK_MIN_SIZE of
> stack and no tail call. So classic uprobes and uprobe.multi are
> affected as well, not only sleepable programs.
> 
> The test: four processes pinned to one CPU call getppid() in a loop for
> 10 s. The program fills 16 stack slots (a 216-byte frame) with
> bpf_get_current_pid_tgid(), spins with 200 bpf_ktime_get_ns() calls,
> and counts runs where a slot no longer matches a fresh
> bpf_get_current_pid_tgid(). The JIT output confirms that this program
> uses the private stack. The controls do not: one adds a tail call into
> an empty PROG_ARRAY, the other uses a single slot.
> 
> Clobbered entry runs / total runs, one 10 s run per cell (7.0: one of
> two runs, the other was within a few percent), x86_64, 4-vCPU KVM
> guests:
> 
>   kernel         preempt    uprobe      uprobe.multi  uprobe.s
>   6.12.112       voluntary  0/288000    0/296000      0/288000  *
>   6.18.56        none       0/272000    0/288000      0/288000
>   6.18.56        voluntary  0/272000    0/288000      0/288000
>   6.18.56        full       497/272000  481/280000    501/280000
>   6.18.56        lazy       0/280000    0/280000      0/288000
>   7.0 (Ubuntu)   full       1221/856004 1202/864004   1158/856004
>   7.0 (Ubuntu)   lazy       0/840004    0/848004      1/856004
>   7.2.10         full       649/296000  654/288000    642/288000
>   7.2.10         lazy       642/288000  622/280000    655/288000
>   bpf-next       full       700/288000  595/280000    619/280000
>   bpf-next       lazy       634/288000  670/288000    647/288000
> 
> * 6.12 has no private stack. Its JIT output uses the task stack.
> 
> The return probes gave numbers of the same size. Both controls stayed at
> 0 in every row. On 7.0 under lazy, each variant gave 0 to 2 clobbered
> runs per 10 s run across repeats. bpf-next was 7.3.0-rc5-g763187dbbf83.
> On 6.18, lazy stays at zero while 7.2 and bpf-next corrupt under lazy as
> often as under full. We did not look into why.
> 
> In the tracer this showed up as copies from another task's buffer
> pointer and length (wrong bytes or -EFAULT), and as return probes that
> found no entry state because the tid in the frame was not theirs.
> This was on the Ubuntu 7.0 kernel with its default lazy preemption, at
> one to four bad calls per million under steady load, and more when the
> host was busy. It went to zero once the programs stopped using the
> private stack.
> 
> On the fix: I agree with Andrii that skipping the second run is wrong
> for uprobes. It is a separate, legitimate event, and a tracer that
> counts calls would silently lose it. Disabling the private stack for
> uprobes, sleepable or not, as he suggested, avoids that. At load time a
> classic uprobe program cannot be told apart from a kprobe, so this needs
> either the load-time marker that was discussed or dropping the private
> stack for BPF_PROG_TYPE_KPROBE. For uprobe.multi, expected_attach_type
> already says BPF_TRACE_UPROBE_MULTI.
> 
> Until then, a single tail call into an empty PROG_ARRAY keeps a program
> off the private stack.
> 
> I can turn the reproducer into a selftest, or run any version of the fix
> on these kernels.

sorry.. seems this fell through cracks

could we add a BPF_TRACE_UPROBE expected attach type for classic uprobes?

but I think we can't reject attaching an unmarked BPF_PROG_TYPE_KPROBE
program to a uprobe if it was JITed to use a private stack, that would 
break existing users

thoughts?

thanks,
jirka


---
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 8d7228834305..c0bb356b1f46 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -1200,6 +1200,7 @@ enum bpf_attach_type {
 	BPF_TRACE_FENTRY_MULTI,
 	BPF_TRACE_FEXIT_MULTI,
 	BPF_TRACE_FSESSION_MULTI,
+	BPF_TRACE_UPROBE,
 	__MAX_BPF_ATTACH_TYPE
 };
 
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 02ed5368f056..8dfa98b22be9 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -2800,6 +2800,12 @@ bpf_prog_load_check_attach(enum bpf_prog_type prog_type,
 		return -EINVAL;
 
 	switch (prog_type) {
+	case BPF_PROG_TYPE_KPROBE:
+		if (expected_attach_type == BPF_TRACE_UPROBE ||
+		    expected_attach_type == BPF_TRACE_UPROBE_MULTI ||
+		    expected_attach_type == BPF_TRACE_UPROBE_SESSION)
+			return 0;
+		return expected_attach_type ? -EINVAL : 0;
 	case BPF_PROG_TYPE_CGROUP_SOCK:
 		switch (expected_attach_type) {
 		case BPF_CGROUP_INET_SOCK_CREATE:
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 26a5ee0708a3..d9f390b0d289 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -5699,6 +5699,10 @@ static enum priv_stack_mode bpf_enable_priv_stack(struct bpf_prog *prog)
 {
 	if (!bpf_jit_supports_private_stack())
 		return NO_PRIV_STACK;
+	if (prog->expected_attach_type == BPF_TRACE_UPROBE ||
+	    prog->expected_attach_type == BPF_TRACE_UPROBE_MULTI ||
+	    prog->expected_attach_type == BPF_TRACE_UPROBE_SESSION)
+		return NO_PRIV_STACK;
 
 	/* bpf_prog_check_recur() checks all prog types that use bpf trampoline
 	 * while kprobe/tp/perf_event/raw_tp don't use trampoline hence checked
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 457d75ea7142..5fb8b40f52e0 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -11835,6 +11835,19 @@ static int __perf_event_set_bpf_prog(struct perf_event *event,
 	    (is_syscall_tp && prog->type != BPF_PROG_TYPE_TRACEPOINT))
 		return -EINVAL;
 
+	/* XXX leftover.. Multi and session uprobes must use their BPF link attachment path. */
+	if (prog->expected_attach_type == BPF_TRACE_UPROBE_MULTI ||
+	    prog->expected_attach_type == BPF_TRACE_UPROBE_SESSION)
+		return -EINVAL;
+
+	if (prog->expected_attach_type == BPF_TRACE_UPROBE && !is_uprobe)
+		return -EINVAL;
+
+	/* XXX can we do this? */
+	/* A preempted uprobe can be re-entered and overwrite its private stack. */
+	if (is_uprobe && !prog->expected_attach_type && prog->aux->jits_use_priv_stack)
+		return -EINVAL;
+
 	if (prog->type == BPF_PROG_TYPE_KPROBE && prog->sleepable && !is_uprobe)
 		/* only uprobe programs are allowed to be sleepable */
 		return -EINVAL;
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 8d7228834305..c0bb356b1f46 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -1200,6 +1200,7 @@ enum bpf_attach_type {
 	BPF_TRACE_FENTRY_MULTI,
 	BPF_TRACE_FEXIT_MULTI,
 	BPF_TRACE_FSESSION_MULTI,
+	BPF_TRACE_UPROBE,
 	__MAX_BPF_ATTACH_TYPE
 };
 
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 02e75b183ed0..3b7aa4903758 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -10842,11 +10842,11 @@ static const struct bpf_sec_def section_defs[] = {
 	SEC_DEF("sk_reuseport/migrate",	SK_REUSEPORT, BPF_SK_REUSEPORT_SELECT_OR_MIGRATE, SEC_ATTACHABLE),
 	SEC_DEF("sk_reuseport",		SK_REUSEPORT, BPF_SK_REUSEPORT_SELECT, SEC_ATTACHABLE),
 	SEC_DEF("kprobe+",		KPROBE,	0, SEC_NONE, attach_kprobe),
-	SEC_DEF("uprobe+",		KPROBE,	0, SEC_NONE, attach_uprobe),
-	SEC_DEF("uprobe.s+",		KPROBE,	0, SEC_SLEEPABLE, attach_uprobe),
+	SEC_DEF("uprobe+",		KPROBE,	BPF_TRACE_UPROBE, SEC_NONE, attach_uprobe),
+	SEC_DEF("uprobe.s+",		KPROBE,	BPF_TRACE_UPROBE, SEC_SLEEPABLE, attach_uprobe),
 	SEC_DEF("kretprobe+",		KPROBE, 0, SEC_NONE, attach_kprobe),
-	SEC_DEF("uretprobe+",		KPROBE, 0, SEC_NONE, attach_uprobe),
-	SEC_DEF("uretprobe.s+",		KPROBE, 0, SEC_SLEEPABLE, attach_uprobe),
+	SEC_DEF("uretprobe+",		KPROBE, BPF_TRACE_UPROBE, SEC_NONE, attach_uprobe),
+	SEC_DEF("uretprobe.s+",		KPROBE, BPF_TRACE_UPROBE, SEC_SLEEPABLE, attach_uprobe),
 	SEC_DEF("kprobe.multi+",	KPROBE,	BPF_TRACE_KPROBE_MULTI, SEC_NONE, attach_kprobe_multi),
 	SEC_DEF("kretprobe.multi+",	KPROBE,	BPF_TRACE_KPROBE_MULTI, SEC_NONE, attach_kprobe_multi),
 	SEC_DEF("kprobe.session+",	KPROBE,	BPF_TRACE_KPROBE_SESSION, SEC_NONE, attach_kprobe_session),

  reply	other threads:[~2026-10-30 23:01 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-22 22:54 [PATCH bpf v3 0/2] disable private stack for sleepable programs Christian Simon
2026-08-22 22:54 ` [PATCH bpf v3 1/2] bpf: " Christian Simon
2026-08-22 23:10   ` sashiko-bot
2026-08-22 23:46   ` bot+bpf-ci
2026-08-26  1:20   ` Alexei Starovoitov
2026-08-26 13:11     ` Jiri Olsa
2026-08-27 14:56     ` Andrii Nakryiko
2026-08-27 16:35       ` Alexei Starovoitov
2026-08-27 16:40         ` Andrii Nakryiko
2026-08-27 16:55           ` Alexei Starovoitov
2026-08-27 22:32             ` Jiri Olsa
2026-10-30  1:22               ` HyeongJun An
2026-10-30 23:01                 ` Jiri Olsa [this message]
2026-10-31  1:19                   ` HyeongJun An
2026-10-31  5:04                     ` Alexei Starovoitov
2026-08-22 22:54 ` [PATCH bpf v3 2/2] selftests/bpf: verify preemptible uprobes avoid private stack Christian Simon
2026-08-22 23:05   ` sashiko-bot
2026-08-22 23:58   ` bot+bpf-ci

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=auUhrnOOUWY3Mb9y@krava \
    --to=olsajiri@gmail.com \
    --cc=alexei.starovoitov@gmail.com \
    --cc=andrii.nakryiko@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=martin.lau@kernel.org \
    --cc=sammiee5311@gmail.com \
    --cc=simon@swine.de \
    --cc=stable@vger.kernel.org \
    --cc=tj@kernel.org \
    --cc=yonghong.song@linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox