* [PATCH 1/4] perf: nvidia_t410_cmem_latency: fix cpuhp state leak on init failure
@ 2026-05-14 1:06 Saurav Sachidanand
2026-05-14 1:06 ` [PATCH 2/4] perf: nvidia_t410_cmem_latency: handle PERF_EF_UPDATE in stop Saurav Sachidanand
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Saurav Sachidanand @ 2026-05-14 1:06 UTC (permalink / raw)
To: Will Deacon
Cc: Mark Rutland, Besar Wicaksono, Ilkka Koskinen, Andi Shyti,
linux-arm-kernel, linux-perf-users, linux-kernel, aghayev, juew,
Saurav Sachidanand
If platform_driver_register() fails, the cpuhp multi-state registered
by cpuhp_setup_state_multi() is never cleaned up. Add
cpuhp_remove_multi_state() on the error path, mirroring the cleanup
in cmem_lat_pmu_exit().
Fixes: 429b7638b2df ("perf: add NVIDIA Tegra410 CPU Memory Latency PMU")
Signed-off-by: Saurav Sachidanand <sauravsc@amazon.com>
---
drivers/perf/nvidia_t410_cmem_latency_pmu.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/perf/nvidia_t410_cmem_latency_pmu.c b/drivers/perf/nvidia_t410_cmem_latency_pmu.c
index acb8f5571522c..e27bf31b2b366 100644
--- a/drivers/perf/nvidia_t410_cmem_latency_pmu.c
+++ b/drivers/perf/nvidia_t410_cmem_latency_pmu.c
@@ -719,7 +719,11 @@ static int __init cmem_lat_pmu_init(void)
cmem_lat_pmu_cpuhp_state = ret;
- return platform_driver_register(&cmem_lat_pmu_driver);
+ ret = platform_driver_register(&cmem_lat_pmu_driver);
+ if (ret)
+ cpuhp_remove_multi_state(cmem_lat_pmu_cpuhp_state);
+
+ return ret;
}
static void __exit cmem_lat_pmu_exit(void)
--
2.47.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/4] perf: nvidia_t410_cmem_latency: handle PERF_EF_UPDATE in stop
2026-05-14 1:06 [PATCH 1/4] perf: nvidia_t410_cmem_latency: fix cpuhp state leak on init failure Saurav Sachidanand
@ 2026-05-14 1:06 ` Saurav Sachidanand
2026-05-14 12:49 ` sashiko-bot
2026-05-14 1:06 ` [PATCH 3/4] perf: nvidia_t410_c2c: fix cpuhp state leak on init failure Saurav Sachidanand
2026-05-14 1:06 ` [PATCH 4/4] perf: nvidia_t410_c2c: handle PERF_EF_UPDATE in stop Saurav Sachidanand
2 siblings, 1 reply; 5+ messages in thread
From: Saurav Sachidanand @ 2026-05-14 1:06 UTC (permalink / raw)
To: Will Deacon
Cc: Mark Rutland, Besar Wicaksono, Ilkka Koskinen, Andi Shyti,
linux-arm-kernel, linux-perf-users, linux-kernel, aghayev, juew,
Saurav Sachidanand
cmem_lat_pmu_stop() does not read the final counter value when called
with PERF_EF_UPDATE. When perf core calls pmu->del() -> pmu->stop()
with PERF_EF_UPDATE, the last counter delta is lost because the event
is marked stopped without reading hardware.
Add the standard PMU stop pattern: bail out if already stopped, call
the event update function when PERF_EF_UPDATE is requested, then mark
the event stopped.
Fixes: 429b7638b2df ("perf: add NVIDIA Tegra410 CPU Memory Latency PMU")
Signed-off-by: Saurav Sachidanand <sauravsc@amazon.com>
---
drivers/perf/nvidia_t410_cmem_latency_pmu.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/perf/nvidia_t410_cmem_latency_pmu.c b/drivers/perf/nvidia_t410_cmem_latency_pmu.c
index e27bf31b2b366..c7fa54c7a7c9e 100644
--- a/drivers/perf/nvidia_t410_cmem_latency_pmu.c
+++ b/drivers/perf/nvidia_t410_cmem_latency_pmu.c
@@ -303,6 +303,12 @@ static void cmem_lat_pmu_start(struct perf_event *event, int pmu_flags)
static void cmem_lat_pmu_stop(struct perf_event *event, int pmu_flags)
{
+ if (event->hw.state & PERF_HES_STOPPED)
+ return;
+
+ if (pmu_flags & PERF_EF_UPDATE)
+ cmem_lat_pmu_event_update(event);
+
event->hw.state |= PERF_HES_STOPPED;
}
--
2.47.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 3/4] perf: nvidia_t410_c2c: fix cpuhp state leak on init failure
2026-05-14 1:06 [PATCH 1/4] perf: nvidia_t410_cmem_latency: fix cpuhp state leak on init failure Saurav Sachidanand
2026-05-14 1:06 ` [PATCH 2/4] perf: nvidia_t410_cmem_latency: handle PERF_EF_UPDATE in stop Saurav Sachidanand
@ 2026-05-14 1:06 ` Saurav Sachidanand
2026-05-14 1:06 ` [PATCH 4/4] perf: nvidia_t410_c2c: handle PERF_EF_UPDATE in stop Saurav Sachidanand
2 siblings, 0 replies; 5+ messages in thread
From: Saurav Sachidanand @ 2026-05-14 1:06 UTC (permalink / raw)
To: Will Deacon
Cc: Mark Rutland, Besar Wicaksono, Ilkka Koskinen, Andi Shyti,
linux-arm-kernel, linux-perf-users, linux-kernel, aghayev, juew,
Saurav Sachidanand
If platform_driver_register() fails, the cpuhp multi-state registered
by cpuhp_setup_state_multi() is never cleaned up. Add
cpuhp_remove_multi_state() on the error path, mirroring the cleanup
in nv_c2c_pmu_exit().
Fixes: 2f89b7f78c50 ("perf: add NVIDIA Tegra410 C2C PMU")
Signed-off-by: Saurav Sachidanand <sauravsc@amazon.com>
---
drivers/perf/nvidia_t410_c2c_pmu.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/perf/nvidia_t410_c2c_pmu.c b/drivers/perf/nvidia_t410_c2c_pmu.c
index 411987153ff3f..662fa1bc833a5 100644
--- a/drivers/perf/nvidia_t410_c2c_pmu.c
+++ b/drivers/perf/nvidia_t410_c2c_pmu.c
@@ -1034,7 +1034,12 @@ static int __init nv_c2c_pmu_init(void)
return ret;
nv_c2c_pmu_cpuhp_state = ret;
- return platform_driver_register(&nv_c2c_pmu_driver);
+
+ ret = platform_driver_register(&nv_c2c_pmu_driver);
+ if (ret)
+ cpuhp_remove_multi_state(nv_c2c_pmu_cpuhp_state);
+
+ return ret;
}
static void __exit nv_c2c_pmu_exit(void)
--
2.47.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 4/4] perf: nvidia_t410_c2c: handle PERF_EF_UPDATE in stop
2026-05-14 1:06 [PATCH 1/4] perf: nvidia_t410_cmem_latency: fix cpuhp state leak on init failure Saurav Sachidanand
2026-05-14 1:06 ` [PATCH 2/4] perf: nvidia_t410_cmem_latency: handle PERF_EF_UPDATE in stop Saurav Sachidanand
2026-05-14 1:06 ` [PATCH 3/4] perf: nvidia_t410_c2c: fix cpuhp state leak on init failure Saurav Sachidanand
@ 2026-05-14 1:06 ` Saurav Sachidanand
2 siblings, 0 replies; 5+ messages in thread
From: Saurav Sachidanand @ 2026-05-14 1:06 UTC (permalink / raw)
To: Will Deacon
Cc: Mark Rutland, Besar Wicaksono, Ilkka Koskinen, Andi Shyti,
linux-arm-kernel, linux-perf-users, linux-kernel, aghayev, juew,
Saurav Sachidanand
nv_c2c_pmu_stop() does not read the final counter value when called
with PERF_EF_UPDATE. The last counter delta is lost when perf core
removes the event.
Add the standard PMU stop pattern: bail out if already stopped, call
nv_c2c_pmu_event_update() when PERF_EF_UPDATE is requested, then mark
the event stopped.
Fixes: 2f89b7f78c50 ("perf: add NVIDIA Tegra410 C2C PMU")
Signed-off-by: Saurav Sachidanand <sauravsc@amazon.com>
---
drivers/perf/nvidia_t410_c2c_pmu.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/perf/nvidia_t410_c2c_pmu.c b/drivers/perf/nvidia_t410_c2c_pmu.c
index 662fa1bc833a5..39d0e661e4c5c 100644
--- a/drivers/perf/nvidia_t410_c2c_pmu.c
+++ b/drivers/perf/nvidia_t410_c2c_pmu.c
@@ -394,6 +394,12 @@ static void nv_c2c_pmu_start(struct perf_event *event, int pmu_flags)
static void nv_c2c_pmu_stop(struct perf_event *event, int pmu_flags)
{
+ if (event->hw.state & PERF_HES_STOPPED)
+ return;
+
+ if (pmu_flags & PERF_EF_UPDATE)
+ nv_c2c_pmu_event_update(event);
+
event->hw.state |= PERF_HES_STOPPED;
}
--
2.47.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/4] perf: nvidia_t410_cmem_latency: handle PERF_EF_UPDATE in stop
2026-05-14 1:06 ` [PATCH 2/4] perf: nvidia_t410_cmem_latency: handle PERF_EF_UPDATE in stop Saurav Sachidanand
@ 2026-05-14 12:49 ` sashiko-bot
0 siblings, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-05-14 12:49 UTC (permalink / raw)
To: Saurav Sachidanand; +Cc: linux-perf-users
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] Performing MMIO reads on clock-gated hardware in pmu->stop() will trigger a system crash (SError).
- [High] API Contract Violation and conditional update logic error for free-running PMU counters.
- [High] Use-After-Free of PMU context during CPU hotplug teardown.
--
commit 1c17f7a02936b83432204f0c38f9d1d22f6f4acd
Author: Saurav Sachidanand <sauravsc@amazon.com>
perf: nvidia_t410_cmem_latency: handle PERF_EF_UPDATE in stop
This commit updates cmem_lat_pmu_stop to return early if the event is
already stopped, and conditionally updates the event if the update flag is
requested.
> cmem_lat_pmu_stop() does not read the final counter value when called
> with PERF_EF_UPDATE. When perf core calls pmu->del() -> pmu->stop()
> with PERF_EF_UPDATE, the last counter delta is lost because the event
> is marked stopped without reading hardware.
Is this description accurate?
Looking at cmem_lat_pmu_disable(), it explicitly stops the hardware, reads
and updates all active events, and clears the counters. Since pmu->stop()
is called within the perf_pmu_disable() block, the final count should already
be unconditionally read before shutting down the hardware, meaning the last
counter delta might not be lost.
> diff --git a/drivers/perf/nvidia_t410_cmem_latency_pmu.c b/drivers/perf/nvidia_t410_cmem_latency_pmu.c
> --- a/drivers/perf/nvidia_t410_cmem_latency_pmu.c
> +++ b/drivers/perf/nvidia_t410_cmem_latency_pmu.c
> @@ -303,6 +303,12 @@ static void cmem_lat_pmu_start(struct perf_event *event, int pmu_flags)
>
> static void cmem_lat_pmu_stop(struct perf_event *event, int pmu_flags)
> {
> + if (event->hw.state & PERF_HES_STOPPED)
> + return;
Does this early return violate the perf core API contract by ignoring
subsequent PERF_EF_UPDATE requests for already-stopped events?
> +
> + if (pmu_flags & PERF_EF_UPDATE)
> + cmem_lat_pmu_event_update(event);
Will this trigger an asynchronous external abort (SError) and crash
the system?
The perf core calls pmu->stop() within a perf_pmu_disable() block.
In this driver, cmem_lat_pmu_disable() gates the module clock. Calling
cmem_lat_pmu_event_update() performs MMIO reads on the PMU registers.
Since this happens while the PMU clock is gated, these MMIO accesses will
target unclocked registers.
Additionally, does this conditional update cause regressions for free-running
PMU counters?
Since this PMU uses free-running global counters without per-event hardware
stops, conditionally updating the event only when PERF_EF_UPDATE is set means
calls like pmu->stop(event, 0) will fail to capture the counter delta. This
allows the hardware counter to continue incrementing while the event is
logically stopped, which could corrupt subsequent readings.
> +
> event->hw.state |= PERF_HES_STOPPED;
> }
While reviewing this driver, I noticed an existing regression in the hotplug
teardown path.
Can a use-after-free occur during CPU hotplug teardown?
In cmem_lat_pmu_device_remove(), perf_pmu_unregister() is called before
cpuhp_state_remove_instance(). perf_pmu_unregister() detaches all events and
frees internal core PMU resources.
When cpuhp_state_remove_instance() runs sequentially after it, it executes
the hotplug teardown callback cmem_lat_pmu_cpu_teardown(), which invokes
perf_pmu_migrate_context() with the already unregistered PMU pointer. This
accesses partially freed perf core state, which might cause a kernel panic
upon unbind or failed initialization.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260514010629.76558-1-sauravsc@amazon.com?part=2
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-05-14 12:49 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-14 1:06 [PATCH 1/4] perf: nvidia_t410_cmem_latency: fix cpuhp state leak on init failure Saurav Sachidanand
2026-05-14 1:06 ` [PATCH 2/4] perf: nvidia_t410_cmem_latency: handle PERF_EF_UPDATE in stop Saurav Sachidanand
2026-05-14 12:49 ` sashiko-bot
2026-05-14 1:06 ` [PATCH 3/4] perf: nvidia_t410_c2c: fix cpuhp state leak on init failure Saurav Sachidanand
2026-05-14 1:06 ` [PATCH 4/4] perf: nvidia_t410_c2c: handle PERF_EF_UPDATE in stop Saurav Sachidanand
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox