All of lore.kernel.org
 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: 19+ 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-10-31  9:42                       ` HyeongJun An
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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.