dri-devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 0/2] perf: Add some null ptr checks
@ 2026-09-01 21:19 Vinay Belgaumkar
  2026-09-01 21:19 ` [PATCH v5 1/2] perf: Fix null pointer access in is_include_guest_event() Vinay Belgaumkar
  2026-09-01 21:19 ` [PATCH v5 2/2] perf: Add checks to prevent null ptr access Vinay Belgaumkar
  0 siblings, 2 replies; 6+ messages in thread
From: Vinay Belgaumkar @ 2026-09-01 21:19 UTC (permalink / raw)
  To: intel-xe; +Cc: dri-devel, Vinay Belgaumkar, Dapeng Mi

This series fixes a crash seen when the PMU ptr is freed while
there is an open perf connection.

Cc: Dapeng Mi <dapeng1.mi@linux.intel.com>
Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>

Vinay Belgaumkar (2):
  perf: Fix null pointer access in is_include_guest_event()
  perf: Add checks to prevent null ptr access

 kernel/events/core.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

-- 
2.38.1


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

* [PATCH v5 1/2] perf: Fix null pointer access in is_include_guest_event()
  2026-09-01 21:19 [PATCH v5 0/2] perf: Add some null ptr checks Vinay Belgaumkar
@ 2026-09-01 21:19 ` Vinay Belgaumkar
  2026-09-02  0:54   ` Mi, Dapeng
  2026-09-01 21:19 ` [PATCH v5 2/2] perf: Add checks to prevent null ptr access Vinay Belgaumkar
  1 sibling, 1 reply; 6+ messages in thread
From: Vinay Belgaumkar @ 2026-09-01 21:19 UTC (permalink / raw)
  To: intel-xe; +Cc: dri-devel, Vinay Belgaumkar, Alexander Kanevskiy, Dapeng Mi

A typical module unload occurring event when there is an active perf
connection leads to freeing of the pmu pointer. The call log is something
like:
 ..
 __pmu_detach_event
 pmu_detach_event
 pmu_detach_events
 perf_pmu_unregister
 ..

__pmu_detach_event() sets event->pmu to null. When the perf connection
finally is closed, the following stack trace is observed:

 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

A call to mediated_pmu_unaccount_event() inside _free_event() is the root
cause of this crash. Adding a check to is_include_guest_event() ensures
we don't accidentally access a null ptr.

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)
v5: Improve commit message and split patch into 2 (Dapeng)

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 | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 94ca277e0819..f19d27ac411d 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -6350,6 +6350,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 +12980,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] 6+ messages in thread

* [PATCH v5 2/2] perf: Add checks to prevent null ptr access
  2026-09-01 21:19 [PATCH v5 0/2] perf: Add some null ptr checks Vinay Belgaumkar
  2026-09-01 21:19 ` [PATCH v5 1/2] perf: Fix null pointer access in is_include_guest_event() Vinay Belgaumkar
@ 2026-09-01 21:19 ` Vinay Belgaumkar
  2026-09-01 21:41   ` sashiko-bot
  2026-09-02  0:55   ` Mi, Dapeng
  1 sibling, 2 replies; 6+ messages in thread
From: Vinay Belgaumkar @ 2026-09-01 21:19 UTC (permalink / raw)
  To: intel-xe; +Cc: dri-devel, Vinay Belgaumkar, Dapeng Mi

Sashiko recommended some additional checks to prevent null pointer
access. Check for revoked states inside perf_event_read_local(), as
the pmu event may have already been freed at this point. Add a null
check inside __perf_event_read_cpu() as well before accessing the pmu
ptr.

Cc: Dapeng Mi <dapeng1.mi@linux.intel.com>
Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
---
 kernel/events/core.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index f19d27ac411d..bb90034a2054 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -4788,14 +4788,19 @@ static inline const struct cpumask *perf_scope_cpu_topology_cpumask(unsigned int
 
 static int __perf_event_read_cpu(struct perf_event *event, int event_cpu)
 {
+	struct pmu *pmu = READ_ONCE(event->pmu);
 	int local_cpu = smp_processor_id();
 	u16 local_pkg, event_pkg;
 
 	if ((unsigned)event_cpu >= nr_cpu_ids)
 		return event_cpu;
 
+	if (!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);
+		const struct cpumask *cpumask = perf_scope_cpu_topology_cpumask(pmu->scope,
+										event_cpu);
 
 		if (cpumask && cpumask_test_cpu(local_cpu, cpumask))
 			return local_cpu;
@@ -4917,6 +4922,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
-- 
2.38.1


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

* Re: [PATCH v5 2/2] perf: Add checks to prevent null ptr access
  2026-09-01 21:19 ` [PATCH v5 2/2] perf: Add checks to prevent null ptr access Vinay Belgaumkar
@ 2026-09-01 21:41   ` sashiko-bot
  2026-09-02  0:55   ` Mi, Dapeng
  1 sibling, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-09-01 21:41 UTC (permalink / raw)
  To: Vinay Belgaumkar; +Cc: dri-devel, intel-xe

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

Pre-existing issues:
- [Critical] The added checks for `event->pmu` and `event->state` are insufficient to prevent Time-of-Check to Time-of-Use (TOCTOU) races during concurrent PMU unregistration, leading to potential NULL pointer dereferences or Use-After-Free.
--

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

perf: Add checks to prevent null ptr access

This commit adds checks in __perf_event_read_cpu() and perf_event_read_local()
to verify that the PMU pointer is valid and the event state is not revoked
before proceeding. This aims to prevent null pointer access when reading PMU
events.

> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 7777e82aad5e7..059f82f0cadd4 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -4788,14 +4788,19 @@ static inline const struct cpumask *perf_scope_cpu_topology_cpumask(unsigned int
>  
>  static int __perf_event_read_cpu(struct perf_event *event, int event_cpu)
>  {
> +	struct pmu *pmu = READ_ONCE(event->pmu);
>  	int local_cpu = smp_processor_id();
>  	u16 local_pkg, event_pkg;
>  
>  	if ((unsigned)event_cpu >= nr_cpu_ids)
>  		return event_cpu;
>  
> +	if (!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);
> +		const struct cpumask *cpumask = perf_scope_cpu_topology_cpumask(pmu->scope,
> +										event_cpu);

[Severity: Critical]
This is a pre-existing issue, but does caching the PMU pointer here leave us
vulnerable to a Use-After-Free?

If the PMU driver is concurrently unloaded (e.g., via module unload),
perf_pmu_unregister() executes synchronize_rcu() before detaching events. A
BPF program starting after the RCU sync might still find the event in its map
and race with __pmu_detach_event().

Even though we pass the new !pmu check, if the concurrent unregistration
completes and frees the driver's memory immediately after, accessing
pmu->scope could result in a Use-After-Free.

[ ... ]
> @@ -4917,6 +4922,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;
> +	}
> +

[Severity: Critical]
This is a pre-existing issue, but does this check completely prevent the NULL
pointer dereference further down in the function?

Later in perf_event_read_local(), there is this call:

    if (event_oncpu == smp_processor_id())
        event->pmu->read(event);

If a BPF program invokes perf_event_read_local() concurrently with PMU
unregistration, the READ_ONCE(event->state) check might pass. However,
__pmu_detach_event() forcefully sets event->pmu = NULL without waiting for
perf_event_read_local() to finish. This could cause the subsequent
event->pmu->read(event) to dereference a NULL pointer.

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

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

* Re: [PATCH v5 1/2] perf: Fix null pointer access in is_include_guest_event()
  2026-09-01 21:19 ` [PATCH v5 1/2] perf: Fix null pointer access in is_include_guest_event() Vinay Belgaumkar
@ 2026-09-02  0:54   ` Mi, Dapeng
  0 siblings, 0 replies; 6+ messages in thread
From: Mi, Dapeng @ 2026-09-02  0:54 UTC (permalink / raw)
  To: Vinay Belgaumkar, intel-xe; +Cc: dri-devel, Alexander Kanevskiy


On 9/2/2026 5:19 AM, Vinay Belgaumkar wrote:
> A typical module unload occurring event when there is an active perf
> connection leads to freeing of the pmu pointer. The call log is something
> like:
>  ..
>  __pmu_detach_event
>  pmu_detach_event
>  pmu_detach_events
>  perf_pmu_unregister
>  ..
>
> __pmu_detach_event() sets event->pmu to null. When the perf connection
> finally is closed, the following stack trace is observed:
>
>  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
>
> A call to mediated_pmu_unaccount_event() inside _free_event() is the root
> cause of this crash. Adding a check to is_include_guest_event() ensures
> we don't accidentally access a null ptr.

Better mention adding an extra mediated_pmu_unaccount_event() call before
clearing event->pmu to ensure the nr_include_guest_events count is
correctly decreased.


>
> 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)
> v5: Improve commit message and split patch into 2 (Dapeng)

Please remove this version history in the formal patch.

Others look good to me. Thanks.


>
> 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 | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 94ca277e0819..f19d27ac411d 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -6350,6 +6350,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 +12980,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] 6+ messages in thread

* Re: [PATCH v5 2/2] perf: Add checks to prevent null ptr access
  2026-09-01 21:19 ` [PATCH v5 2/2] perf: Add checks to prevent null ptr access Vinay Belgaumkar
  2026-09-01 21:41   ` sashiko-bot
@ 2026-09-02  0:55   ` Mi, Dapeng
  1 sibling, 0 replies; 6+ messages in thread
From: Mi, Dapeng @ 2026-09-02  0:55 UTC (permalink / raw)
  To: Vinay Belgaumkar, intel-xe; +Cc: dri-devel

LGTM. Thanks.

On 9/2/2026 5:19 AM, Vinay Belgaumkar wrote:
> Sashiko recommended some additional checks to prevent null pointer
> access. Check for revoked states inside perf_event_read_local(), as
> the pmu event may have already been freed at this point. Add a null
> check inside __perf_event_read_cpu() as well before accessing the pmu
> ptr.
>
> Cc: Dapeng Mi <dapeng1.mi@linux.intel.com>
> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> ---
>  kernel/events/core.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index f19d27ac411d..bb90034a2054 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -4788,14 +4788,19 @@ static inline const struct cpumask *perf_scope_cpu_topology_cpumask(unsigned int
>  
>  static int __perf_event_read_cpu(struct perf_event *event, int event_cpu)
>  {
> +	struct pmu *pmu = READ_ONCE(event->pmu);
>  	int local_cpu = smp_processor_id();
>  	u16 local_pkg, event_pkg;
>  
>  	if ((unsigned)event_cpu >= nr_cpu_ids)
>  		return event_cpu;
>  
> +	if (!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);
> +		const struct cpumask *cpumask = perf_scope_cpu_topology_cpumask(pmu->scope,
> +										event_cpu);
>  
>  		if (cpumask && cpumask_test_cpu(local_cpu, cpumask))
>  			return local_cpu;
> @@ -4917,6 +4922,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

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

end of thread, other threads:[~2026-09-02  0:55 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-01 21:19 [PATCH v5 0/2] perf: Add some null ptr checks Vinay Belgaumkar
2026-09-01 21:19 ` [PATCH v5 1/2] perf: Fix null pointer access in is_include_guest_event() Vinay Belgaumkar
2026-09-02  0:54   ` Mi, Dapeng
2026-09-01 21:19 ` [PATCH v5 2/2] perf: Add checks to prevent null ptr access Vinay Belgaumkar
2026-09-01 21:41   ` sashiko-bot
2026-09-02  0:55   ` Mi, Dapeng

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