dri-devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4] perf: Fix null pointer access in is_include_guest_event()
@ 2026-08-31 23:50 Vinay Belgaumkar
  2026-09-01  0:09 ` sashiko-bot
  2026-09-01  1:54 ` Mi, Dapeng
  0 siblings, 2 replies; 3+ messages in thread
From: Vinay Belgaumkar @ 2026-08-31 23:50 UTC (permalink / raw)
  To: intel-xe; +Cc: dri-devel, Vinay Belgaumkar, Alexander Kanevskiy, Dapeng Mi

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().

Signature of the failure:

 Oops: general protection fault, kernel NULL pointer dereference
 ...
 RIP: 0010:_free_event+0x3e/0x370
 ...
 Call Trace:
 ...
 perf_event_release_kernel+0x260/0x2d0
 perf_release+0x12/0x20

While here, also add a null check inside perf_event_read_local() which can
cause a similar crash. Sashiko pointed out a possible race between when we
check for revoked state here and then actually access event-pmu inside
__perf_event_read_cpu(). So, add another null check there.

v2: Add another check to prevent null access (sashiko)
v3: Use READ_ONCE for checking pmu state (sashiko)
v4: READ_ONCE for the pmu check as well (sashiko)

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

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 94ca277e0819..4f8c5191ef2c 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -4794,6 +4794,9 @@ static int __perf_event_read_cpu(struct perf_event *event, int event_cpu)
 	if ((unsigned)event_cpu >= nr_cpu_ids)
 		return event_cpu;
 
+	if (!READ_ONCE(event->pmu))
+		return -ENODEV;
+
 	if (event->group_caps & PERF_EV_CAP_READ_SCOPE) {
 		const struct cpumask *cpumask = perf_scope_cpu_topology_cpumask(event->pmu->scope, event_cpu);
 
@@ -4917,6 +4920,11 @@ int perf_event_read_local(struct perf_event *event, u64 *value,
 		goto out;
 	}
 
+	if (READ_ONCE(event->state) <= PERF_EVENT_STATE_REVOKED) {
+		ret = -ENODEV;
+		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 +6358,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 +12988,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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v4] perf: Fix null pointer access in is_include_guest_event()
  2026-08-31 23:50 [PATCH v4] perf: Fix null pointer access in is_include_guest_event() Vinay Belgaumkar
@ 2026-09-01  0:09 ` sashiko-bot
  2026-09-01  1:54 ` Mi, Dapeng
  1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-09-01  0:09 UTC (permalink / raw)
  To: Vinay Belgaumkar; +Cc: intel-xe, dri-devel

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

New issues:
- [High] A TOCTOU race condition in `__perf_event_read_cpu()` can cause a NULL pointer dereference because the result of `READ_ONCE(event->pmu)` is not cached locally.

Pre-existing issues:
- [High] Incomplete fix: TOCTOU races in `perf_event_read_local()` leading to NULL pointer dereference or Use-After-Free of `event->pmu`.
- [High] Blind mutation of `pmu->exclusive_cnt` on PMU unregister for non-exclusive events.
--

commit 5b5202c48251a852d95e17a012ae0db3e210700b
Author: Vinay Belgaumkar <vinay.belgaumkar@intel.com>

perf: Fix null pointer access in is_include_guest_event()

This commit addresses a null pointer dereference in is_include_guest_event()
when perf_pmu_unregister() is called during an active session. It also adds
null and state checks in __perf_event_read_cpu() and perf_event_read_local()
to prevent similar crashes.

> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index a6c8e38a31104..f105cd7ac1524 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -4794,6 +4794,9 @@ static int __perf_event_read_cpu(struct perf_event *event, int event_cpu)
>  	if ((unsigned)event_cpu >= nr_cpu_ids)
>  		return event_cpu;
>  
> +	if (!READ_ONCE(event->pmu))
> +		return -ENODEV;
> +
>  	if (event->group_caps & PERF_EV_CAP_READ_SCOPE) {
>  		const struct cpumask *cpumask = perf_scope_cpu_topology_cpumask(event->pmu->scope, event_cpu);

[Severity: High]
Does this code leave a time-of-check to time-of-use race condition open?

Because the result of READ_ONCE(event->pmu) is not cached locally, a
concurrent detachment could set event->pmu to NULL immediately after the
check.

If that happens, the next line in __perf_event_read_cpu() would dereference a
NULL pointer when accessing event->pmu->scope.

Could we store the PMU pointer in a local variable instead of re-reading
event->pmu?

[ ... ]

> @@ -4917,6 +4920,11 @@ int perf_event_read_local(struct perf_event *event, u64 *value,
>  		goto out;
>  	}
>  
> +	if (READ_ONCE(event->state) <= PERF_EVENT_STATE_REVOKED) {
> +		ret = -ENODEV;
> +		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

[Severity: High]
This is a pre-existing issue, but does this state check fully prevent
concurrent PMU unregistration issues in perf_event_read_local()?

Even if the state check passes, a concurrent detachment could still set
event->pmu to NULL before the function completes.

Later in perf_event_read_local(), the code directly dereferences the PMU
pointer:

kernel/events/core.c:perf_event_read_local() {
	...
	if (event_oncpu == smp_processor_id())
		event->pmu->read(event);
	...
}

Would it be possible for event->pmu to become NULL prior to this final
execution, causing a panic?

[ ... ]

> @@ -13002,6 +13013,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);

[Severity: High]
This is a pre-existing issue, but does calling exclusive_event_destroy()
here cause problems for non-exclusive events?

In __pmu_detach_event(), exclusive_event_destroy() is invoked without
verifying if the event has the PERF_ATTACH_EXCLUSIVE flag set.

Looking at exclusive_event_destroy(), it directly modifies the PMU's atomic
exclusive counter:

kernel/events/core.c:exclusive_event_destroy() {
	...
	if (event->attach_state & PERF_ATTACH_TASK)
		atomic_dec(&pmu->exclusive_cnt);
	else
		atomic_inc(&pmu->exclusive_cnt);
	...
}

Could this blindly corrupt the PMU's reference limits during unregistration
for normal, non-exclusive events?

>  	event->pmu = NULL; /* force fault instead of UAF */
>  }
>

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v4] perf: Fix null pointer access in is_include_guest_event()
  2026-08-31 23:50 [PATCH v4] perf: Fix null pointer access in is_include_guest_event() Vinay Belgaumkar
  2026-09-01  0:09 ` sashiko-bot
@ 2026-09-01  1:54 ` Mi, Dapeng
  1 sibling, 0 replies; 3+ messages in thread
From: Mi, Dapeng @ 2026-09-01  1:54 UTC (permalink / raw)
  To: Vinay Belgaumkar, intel-xe; +Cc: dri-devel, Alexander Kanevskiy


On 9/1/2026 7:50 AM, Vinay Belgaumkar wrote:
> 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().
>
> Signature of the failure:
>
>  Oops: general protection fault, kernel NULL pointer dereference
>  ...
>  RIP: 0010:_free_event+0x3e/0x370
>  ...
>  Call Trace:
>  ...
>  perf_event_release_kernel+0x260/0x2d0
>  perf_release+0x12/0x20
>
> While here, also add a null check inside perf_event_read_local() which can
> cause a similar crash. Sashiko pointed out a possible race between when we
> check for revoked state here and then actually access event-pmu inside
> __perf_event_read_cpu(). So, add another null check there.
>
> v2: Add another check to prevent null access (sashiko)
> v3: Use READ_ONCE for checking pmu state (sashiko)
> v4: READ_ONCE for the pmu check as well (sashiko)

Thanks for finding and reporting this issue.

Please rewrite the change log and explicitly show the call-chain and tell
why NULL pointer is accessed.

e.g, pmu_detach_event() clears the event->pmu to null and then put_event()
calls mediated_pmu_unaccount_event() and then trigger the NULL pointer access.


>
> Fixes: eff95e170275 ("perf: Add APIs to create/release mediated guest vPMUs")
> Cc: Alexander Kanevskiy <alexander.kanevskiy@intel.com>
> Cc: Dapeng Mi <dapeng1.mi@linux.intel.com>
> Assisted-by: Claude:Claude-Sonnet-5
> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> ---
>  kernel/events/core.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 94ca277e0819..4f8c5191ef2c 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -4794,6 +4794,9 @@ static int __perf_event_read_cpu(struct perf_event *event, int event_cpu)
>  	if ((unsigned)event_cpu >= nr_cpu_ids)
>  		return event_cpu;
>  
> +	if (!READ_ONCE(event->pmu))
> +		return -ENODEV;
> +
>  	if (event->group_caps & PERF_EV_CAP_READ_SCOPE) {
>  		const struct cpumask *cpumask = perf_scope_cpu_topology_cpumask(event->pmu->scope, event_cpu);
>  
> @@ -4917,6 +4920,11 @@ int perf_event_read_local(struct perf_event *event, u64 *value,
>  		goto out;
>  	}
>  
> +	if (READ_ONCE(event->state) <= PERF_EVENT_STATE_REVOKED) {
> +		ret = -ENODEV;
> +		goto out;
> +	}
> +

The above two changes has nothing to do with the fix of null pointer
access, better move them into a separate patch.
Thanks.


>  	/*
>  	 * 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 +6358,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 +12988,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 */
>  }
>  

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-09-01  1:54 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-31 23:50 [PATCH v4] perf: Fix null pointer access in is_include_guest_event() Vinay Belgaumkar
2026-09-01  0:09 ` sashiko-bot
2026-09-01  1:54 ` Mi, Dapeng

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox