Re: [PATCH v4 10/11] perf/cxl: Don't log through pmu.dev in the overflow interrupt handler

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

Pre-existing issues:
- [High] Use-After-Free in `cxl_pmu_offline_cpu()` due to devres teardown order.
--

--- Patch [10]: [PATCH v4 10/11] perf/cxl: Don't log through pmu.dev in the overflow interrupt handler ---
commit 9f5d830c7ee5748e40bbe955adff5fe5abd56b9f
Author: Dave Jiang <[email protected]>

perf/cxl: Don't log through pmu.dev in the overflow interrupt handler

This commit changes the overflow interrupt handler to log through
info->pmu.parent instead of info->pmu.dev. This prevents a use-after-free
if an overflow occurs just before teardown, as perf_pmu_unregister() frees
pmu->dev without clearing the pointer.

> diff --git a/drivers/perf/cxl_pmu.c b/drivers/perf/cxl_pmu.c
> index 84ebc7da181d5..45edef53a983d 100644
> --- a/drivers/perf/cxl_pmu.c
> +++ b/drivers/perf/cxl_pmu.c
> @@ -805,7 +805,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,

[Severity: High]
This is a pre-existing issue, but since we are fixing teardown races with
freed pmu structures, does a similar use-after-free exist in
cxl_pmu_offline_cpu() during concurrent cpu hotplug events?

In cxl_pmu_probe(), the devm actions are ordered such that the pmu is
unregistered before the cpu hotplug instance is removed:

drivers/perf/cxl_pmu.c:cxl_pmu_probe() {
	...
	rc = cpuhp_state_add_instance(cxl_pmu_cpuhp_state_num, &info->node);
	if (rc)
		return rc;

	rc = devm_add_action_or_reset(dev, cxl_pmu_cpuhp_remove, info);
	if (rc)
		return rc;

	rc = perf_pmu_register(&info->pmu, info->pmu.name, -1);
	if (rc)
		return rc;

	rc = devm_add_action_or_reset(dev, cxl_pmu_perf_unregister, info);
	...
}

Because devm executes teardown in a reverse order, cxl_pmu_perf_unregister()
runs first. This calls perf_pmu_unregister(), which frees info->pmu.dev
and info->pmu.cpu_pmu_context.

However, the cpu hotplug instance remains active until cxl_pmu_cpuhp_remove()
runs. If a cpu offline event occurs in this window, cxl_pmu_offline_cpu()
could be invoked:

drivers/perf/cxl_pmu.c:cxl_pmu_offline_cpu() {
	...
		dev_err(info->pmu.dev, "Unable to find a suitable CPU\n");
		return 0;
	}

	perf_pmu_migrate_context(&info->pmu, cpu, target);
	...
}

Could this lead to a use-after-free when dereferencing the freed
info->pmu.dev in the error log, or info->pmu.cpu_pmu_context inside
perf_pmu_migrate_context()?

>  				"overflow but on non enabled counter %d\n", i);
>  			continue;
>  		}

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