From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 637B523BD02; Wed, 28 May 2025 21:56:47 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1748469407; cv=none; b=Qx8mNbjKPRD7BjX6F1IC7F3QU4DWiL/vfdAvv10Hgfd8Uau98eFGHmvYp+4uULU70o5dJZS0+D1jQRomwmgbgv29nlbvZIagu7IdJCuydrQZcUDnwlmuglXga4I0YF59oiy8A8Cu1MRrXM58D249FbxB1IxWC3dD0657uIk0H0g= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1748469407; c=relaxed/simple; bh=7ipO+CcTyqLC2tdZaG7Aofb5TPn4W7/PmutHuo/ElSU=; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version:Content-Type; b=M0y0FcmZhlcC9T1dfOrYIZ9JEXqTWLEflgB7DK59FDszlyIFGUq2buUUKAQaIBh8ZCiilyMYfKRZdq5RfwOHdfnj3R1mM9Ld/Hjm2yDNpE3eKZL1aq4Zcn2V9Sc8CMaKdIoHnqwf8Y7SL+TOUPk4HEOVYH672S0h9lShDPsbXpU= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=c5akxl9c; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="c5akxl9c" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 1E105C4CEEE; Wed, 28 May 2025 21:56:46 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1748469407; bh=7ipO+CcTyqLC2tdZaG7Aofb5TPn4W7/PmutHuo/ElSU=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=c5akxl9c7nw6WrBuwEPa5nD9K8gLW/54N1PyUo6f/kf5vzDXAg/CCGOKiZm/T2XJp K3wqbvJydZzUs/ehM+V5w2r/tYmnecaclwT4xE5vHWjR1YR5q2X3xOFOZIUCiHUpEA RKeutYKA+sDg3Rwxhyf3fW8RVHsWP8rz7J4YzOKnqVuvE/wgO8RRM1WKXe3R+Bw9DN weTlBi6QufrdyIhPBF3n9409eJSdmyC5Hb+vJM0lVhSjtAFkbRUmsty5lSheNq9Dv7 eeo2rWMg4DG71z5IiqTdMJVTlIVEOkm7Pr3J59UXTajdiGLnXWIHD4uIMlVoI1OSAb wsqD3PP9insyw== From: Sasha Levin To: patches@lists.linux.dev, stable@vger.kernel.org Cc: Peter Zijlstra , Ravi Bangoria , Sasha Levin , mingo@redhat.com, acme@kernel.org, ast@kernel.org, daniel@iogearbox.net, linux-kernel@vger.kernel.org, netdev@vger.kernel.org, bpf@vger.kernel.org Subject: [PATCH AUTOSEL 5.15 3/3] perf: Ensure bpf_perf_link path is properly serialized Date: Wed, 28 May 2025 17:56:42 -0400 Message-Id: <20250528215642.1983928-3-sashal@kernel.org> X-Mailer: git-send-email 2.39.5 In-Reply-To: <20250528215642.1983928-1-sashal@kernel.org> References: <20250528215642.1983928-1-sashal@kernel.org> Precedence: bulk X-Mailing-List: stable@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-stable: review X-Patchwork-Hint: Ignore X-stable-base: Linux 5.15.184 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From: Peter Zijlstra [ 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 Signed-off-by: Peter Zijlstra (Intel) Reviewed-by: Ravi Bangoria Link: https://lkml.kernel.org/r/20250307193305.486326750@infradead.org Signed-off-by: Sasha Levin --- 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 520a890a2a6f7..9519661390344 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -5822,6 +5822,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) { @@ -5890,7 +5893,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; @@ -10360,8 +10363,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_tracepoint, is_syscall_tp; @@ -10395,6 +10399,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 (!perf_event_is_tracing(event)) { @@ -10414,7 +10432,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