Re: [PATCH v2 8/9] perf/cxl: Don't use pmu.dev in IRQ and hotplug callbacks after unregister

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

Pre-existing issues:
- [High] The patch correctly fixes a UAF in logging statements during teardown but leaves a critical lifecycle race unaddressed. The IRQ is registered via `devm_request_irq` before `cxl_pmu_perf_unregister` is registered. On device removal, `devm` unwinds LIFO, so `perf_pmu_unregister` runs while the shared IRQ is still live. If the IRQ fires concurrently on another CPU (e.g., due to a co-function changing the shared MSI vector's affinity), `cxl_pmu_irq` can read a non-NULL `event` right before it is detached, leading to a NULL pointer dereference in `__cxl_pmu_read` when it accesses `event->pmu`.
--

--- Patch [8]: [PATCH v2 8/9] perf/cxl: Don't use pmu.dev in IRQ and hotplug callbacks after unregister ---
commit 34ab4c1ad03386965fbc73a1d534f56456842698
Author: Dave Jiang <[email protected]>

perf/cxl: Don't use pmu.dev in IRQ and hotplug callbacks after unregister

The patch replaces info->pmu.dev with info->pmu.parent in the IRQ and CPU
hotplug callbacks to fix a use-after-free during device removal.

Closes: https://sashiko.dev/#/patchset/[email protected]?part=1

> diff --git a/drivers/perf/cxl_pmu.c b/drivers/perf/cxl_pmu.c
> index 2e817a52ff1e1..f42238b2b6b0d 100644
> --- a/drivers/perf/cxl_pmu.c
> +++ b/drivers/perf/cxl_pmu.c
> @@ -803,7 +803,7 @@ static irqreturn_t cxl_pmu_irq(int irq, void *data)
>  		struct perf_event *event = info->hw_events[i];
>  
>  		if (!event) {
> -			dev_dbg(info->pmu.dev,
> +			dev_dbg(info->pmu.parent,
>  				"overflow but on non enabled counter %d\n", i);
>  			continue;
>  		}

[Severity: High]
This is a pre-existing issue, but since this patch addresses teardown races,
does it leave a critical lifecycle race unaddressed here?

Because the IRQ is registered via devm before cxl_pmu_perf_unregister() is
registered, the LIFO unwinding order means perf_pmu_unregister() runs while
the shared IRQ is still live.

If a shared MSI interrupt fires concurrently with cxl_pmu_perf_unregister(),
cxl_pmu_irq() can read a valid event right before it is detached:

cxl_pmu_irq() {
        ...
        struct perf_event *event = info->hw_events[i];
        ...
        __cxl_pmu_read(event, true);
}

Meanwhile on another CPU, the unregister thread detaches the events and
synchronously sets event->pmu = NULL. When the IRQ handler calls
__cxl_pmu_read(), it dereferences event->pmu:

__cxl_pmu_read() {
        struct cxl_pmu_info *info = pmu_to_cxl_pmu_info(event->pmu);
        ...
}

Could this lead to a null pointer dereference and a kernel panic when
info->base is subsequently accessed during device unbind or module removal?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=8