Message-Id: <20250528215559.1983214-8-sashal@kernel.org>
Date: Wed, 28 May 2025 17:55:58 -0400
From: Sasha Levin <sashal@...nel.org>
To: patches@...ts.linux.dev,
	stable@...r.kernel.org
Cc: Peter Zijlstra <peterz@...radead.org>,
	Ravi Bangoria <ravi.bangoria@....com>,
	Sasha Levin <sashal@...nel.org>,
	mingo@...hat.com,
	acme@...nel.org,
	ast@...nel.org,
	daniel@...earbox.net,
	linux-kernel@...r.kernel.org,
	netdev@...r.kernel.org,
	bpf@...r.kernel.org
Subject: [PATCH AUTOSEL 6.15 8/9] perf: Ensure bpf_perf_link path is properly serialized

From: Peter Zijlstra <peterz@...radead.org>

[ Upstream commit 7ed9138a72829d2035ecbd8dbd35b1bc3c137c40 ]

Ravi reported that the bpf_perf_link_attach() usage of
perf_event_set_bpf_prog() is not serialized by ctx->mutex, unlike the
PERF_EVENT_IOC_SET_BPF case.

Reported-by: Ravi Bangoria <ravi.bangoria@....com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.org>
Reviewed-by: Ravi Bangoria <ravi.bangoria@....com>
Link: https://lkml.kernel.org/r/20250307193305.486326750@infradead.org
Signed-off-by: Sasha Levin <sashal@...nel.org>
---

Based on my analysis of the commit and the kernel tree context, here is
my evaluation: **YES** This commit should be backported to stable kernel
trees. Here's my detailed analysis: ## Bug Analysis The commit fixes a
**race condition and security vulnerability** in the BPF perf event
attachment path. Specifically: 1. **Race Condition**: The
`bpf_perf_link_attach()` function calls `perf_event_set_bpf_prog()`
without holding the `ctx->mutex`, while the equivalent ioctl path
(`PERF_EVENT_IOC_SET_BPF`) properly acquires this mutex before calling
the same function. 2. **Inconsistent Locking**: The fix shows two
different code paths accessing the same critical section with different
locking semantics: - **ioctl path** (line 2309): Acquires `ctx->mutex`
via `_perf_ioctl()` → `__perf_event_set_bpf_prog()` -
**bpf_perf_link_attach path**: Called `perf_event_set_bpf_prog()`
directly without mutex protection ## Code Changes Analysis The fix
introduces proper serialization by: 1. **Creating
`__perf_event_set_bpf_prog()`**: An internal version that doesn't
acquire locks 2. **Modifying `perf_event_set_bpf_prog()`**: Now acquires
`ctx->mutex` before calling the internal version 3. **Updating ioctl
path**: Uses the internal version since it already holds the mutex ##
Why This Should Be Backported 1. **Security Impact**: Race conditions in
BPF attachment can lead to use-after-free or other memory corruption
issues that could be exploited 2. **Bug Fix Nature**: This is clearly a
bug fix that addresses inconsistent locking semantics rather than adding
new features 3. **Minimal Risk**: The change is small, contained, and
follows existing patterns - it simply ensures consistent locking across
both code paths 4. **Critical Subsystem**: This affects the BPF
subsystem and perf events, both critical kernel components where race
conditions can have serious security implications 5. **Similar
Historical Precedent**: Looking at the reference commits, commit #5 with
"Backport Status: YES" was backported for fixing a similar type
validation issue in BPF perf events, showing that BPF perf-related fixes
are appropriate for stable trees The commit addresses the exact type of
concurrency bug that stable trees are designed to fix - it's a clear
bugfix with minimal regression risk that addresses a potential security
vulnerability in a critical kernel subsystem.

 kernel/events/core.c | 34 ++++++++++++++++++++++++++++++----
 1 file changed, 30 insertions(+), 4 deletions(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 95e703891b24f..eaa9588eb968d 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -6239,6 +6239,9 @@ static int perf_event_set_output(struct perf_event *event,
 static int perf_event_set_filter(struct perf_event *event, void __user *arg);
 static int perf_copy_attr(struct perf_event_attr __user *uattr,
 			  struct perf_event_attr *attr);
+static int __perf_event_set_bpf_prog(struct perf_event *event,
+				     struct bpf_prog *prog,
+				     u64 bpf_cookie);
 
 static long _perf_ioctl(struct perf_event *event, unsigned int cmd, unsigned long arg)
 {
@@ -6301,7 +6304,7 @@ static long _perf_ioctl(struct perf_event *event, unsigned int cmd, unsigned lon
 		if (IS_ERR(prog))
 			return PTR_ERR(prog);
 
-		err = perf_event_set_bpf_prog(event, prog, 0);
+		err = __perf_event_set_bpf_prog(event, prog, 0);
 		if (err) {
 			bpf_prog_put(prog);
 			return err;
@@ -11069,8 +11072,9 @@ static inline bool perf_event_is_tracing(struct perf_event *event)
 	return false;
 }
 
-int perf_event_set_bpf_prog(struct perf_event *event, struct bpf_prog *prog,
-			    u64 bpf_cookie)
+static int __perf_event_set_bpf_prog(struct perf_event *event,
+				     struct bpf_prog *prog,
+				     u64 bpf_cookie)
 {
 	bool is_kprobe, is_uprobe, is_tracepoint, is_syscall_tp;
 
@@ -11108,6 +11112,20 @@ int perf_event_set_bpf_prog(struct perf_event *event, struct bpf_prog *prog,
 	return perf_event_attach_bpf_prog(event, prog, bpf_cookie);
 }
 
+int perf_event_set_bpf_prog(struct perf_event *event,
+			    struct bpf_prog *prog,
+			    u64 bpf_cookie)
+{
+	struct perf_event_context *ctx;
+	int ret;
+
+	ctx = perf_event_ctx_lock(event);
+	ret = __perf_event_set_bpf_prog(event, prog, bpf_cookie);
+	perf_event_ctx_unlock(event, ctx);
+
+	return ret;
+}
+
 void perf_event_free_bpf_prog(struct perf_event *event)
 {
 	if (!event->prog)
@@ -11130,7 +11148,15 @@ static void perf_event_free_filter(struct perf_event *event)
 {
 }
 
-int perf_event_set_bpf_prog(struct perf_event *event, struct bpf_prog *prog,
+static int __perf_event_set_bpf_prog(struct perf_event *event,
+				     struct bpf_prog *prog,
+				     u64 bpf_cookie)
+{
+	return -ENOENT;
+}
+
+int perf_event_set_bpf_prog(struct perf_event *event,
+			    struct bpf_prog *prog,
 			    u64 bpf_cookie)
 {
 	return -ENOENT;
-- 
2.39.5

Powered by blists - more mailing lists