[PATCH v2] perf: Fix null pointer access in is_include_guest_event()

[PATCH v2] perf: Fix null pointer access in is_include_guest_event()

[Vinay Belgaumkar]

A kernel panic is observed when an application has a perf connection open
and the driver managing the pmu event unbinds. The issue occurs when
perf_pmu_unregister() is called while there is an active perf session. The
null pointer error is seen when the event is freed through __free_event()
and mediated_pmu_unaccount_event is called. There is a missing null check
inside is_include_guest_event(). Also call mediated_pmu_unaccount_event()
inside __pmu_detach_event() similar to __free_event().

v2: Add another check to prevent null access (sashiko)

Fixes: eff95e170275 ("perf: Add APIs to create/release mediated guest vPMUs")
Cc: Alexander Kanevskiy <alexander.kanevskiy@intel.com>
Assisted-by: Claude:Claude-Sonnet-5
Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
---
 kernel/events/core.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 94ca277e0819..88e0d5e476a8 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -4917,6 +4917,11 @@ int perf_event_read_local(struct perf_event *event, u64 *value,
 		goto out;
 	}
 
+	if (event->state <= PERF_EVENT_STATE_REVOKED) {
+		ret = -EINVAL;
+		goto out;
+	}
+
 	/*
 	 * Get the event CPU numbers, and adjust them to local if the event is
 	 * a per-package event that can be read locally
@@ -6350,6 +6355,9 @@ static DEFINE_MUTEX(perf_mediated_pmu_mutex);
 /* !exclude_guest event of PMU with PERF_PMU_CAP_MEDIATED_VPMU */
 static inline bool is_include_guest_event(struct perf_event *event)
 {
+	if (!event->pmu)
+		return false;
+
 	if ((event->pmu->capabilities & PERF_PMU_CAP_MEDIATED_VPMU) &&
 	    !event->attr.exclude_guest)
 		return true;
@@ -12977,6 +12985,7 @@ static void __pmu_detach_event(struct pmu *pmu, struct perf_event *event,
 	exclusive_event_destroy(event);
 	module_put(pmu->module);
 
+	mediated_pmu_unaccount_event(event);
 	event->pmu = NULL; /* force fault instead of UAF */
 }
 
-- 
2.38.1

Re: [PATCH v2] perf: Fix null pointer access in is_include_guest_event()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] The patch silently alters the behavior of perf_event_read_local() by explicitly returning -EINVAL for revoked or dead events.

Pre-existing issues:
- [High] TOCTOU race in `perf_event_read_local()` allows NULL pointer dereference or Use-After-Free of `event->pmu` despite the newly added state check.
--

commit 3c330c9904a35189d22ecaa756a6d8b5458cfb9c
Author: Vinay Belgaumkar <vinay.belgaumkar@intel.com>

perf: Fix null pointer access in is_include_guest_event()

A kernel panic is observed when an application has a perf connection open
and the driver managing the pmu event unbinds. The issue occurs when
perf_pmu_unregister() is called while there is an active perf session. The
null pointer error is seen when the event is freed through __free_event()
and mediated_pmu_unaccount_event is called. There is a missing null check
inside is_include_guest_event(). Also call mediated_pmu_unaccount_event()
inside __pmu_detach_event() similar to __free_event().

> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index a6c8e38a31104..9809fc10c4f51 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -4917,6 +4917,11 @@ int perf_event_read_local(struct perf_event *event, u64 *value,
>  		goto out;
>  	}
>  
> +	if (event->state <= PERF_EVENT_STATE_REVOKED) {
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +

[Severity: Medium]
Does this explicitly change the return value for revoked or dead events to
-EINVAL? The commit message describes fixing a null pointer access in
is_include_guest_event(), but does not mention altering the behavior of
perf_event_read_local() for revoked events. Could this be documented in the
commit message?

[Severity: High]
This is a pre-existing issue, but does this lockless state check leave a
time-of-check to time-of-use race window that can still lead to a null
pointer dereference of event->pmu?

If CPU A calls perf_event_read_local(), disables interrupts, and locklessly
checks event->state, it can pass if the event state is
PERF_EVENT_STATE_INACTIVE.

Concurrently, if CPU B calls perf_pmu_unregister() which leads to
__pmu_detach_event(), it will not send an IPI to synchronize with CPU A
since the event is inactive. CPU B then sets event->state to
PERF_EVENT_STATE_REVOKED and clears event->pmu to NULL.

When CPU A continues and calls __perf_event_read_cpu(event, event->cpu),
if the event has PERF_EV_CAP_READ_SCOPE, it will dereference
event->pmu->scope, which was just set to NULL by CPU B, causing a panic.

Can this sequence be synchronized safely?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260828212803.3963976-1-vinay.belgaumkar@intel.com?part=1