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

[email protected] Fri, 31 Jul 2026 23:46:04 +0000
Newsgroups org.kernel.vger.linux-cxl,org.kernel.vger.linux-perf-users
Message-ID <[email protected]>
Thank you for your contribution! Sashiko AI review found 1 potential issue(=
s) to consider:

Pre-existing issues:
- [High] Double counting and cross-event corruption of performance events d=
ue to uncleared pending overflows.
--

--- Patch [9]: [PATCH v3 9/9] perf/cxl: Don't log through pmu.dev in the ov=
erflow interrupt handler ---
commit d6b58846ef576f998be916a54f3a5a31363d2d44
Author: Dave Jiang <[email protected]>

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

This commit fixes a use-after-free in the CXL PMU overflow interrupt handler
by changing the logging device to the parent and clears any stale overflow
status before requesting the IRQ.

Closes: https://sashiko.dev/#/patchset/20260715191454.459673-1-dave@stgolab=
s.net?part=3D1

[ ... ]

> --- a/drivers/perf/cxl_pmu.c
> +++ b/drivers/perf/cxl_pmu.c

[ ... ]

> @@ -903,6 +903,15 @@ static int cxl_pmu_probe(struct device *dev)
>  	if (!irq_name)
>  		return -ENOMEM;
> =20
> +	/*
> +	 * Clear any overflow status left set by firmware or a previous kernel
> +	 * before the handler goes live, so it cannot mistake a stale bit for an
> +	 * overflow on a counter no event owns yet. The register is RW1C, and
> +	 * bits above the implemented counters are RsvdZ, so only write those.
> +	 */
> +	writeq(GENMASK_ULL(info->num_counters - 1, 0),
> +	       info->base + CXL_PMU_OVERFLOW_REG);
> +

[Severity: High]
This isn't a bug introduced by this patch, but since we are clearing stale=
=20
overflow bits here, does cxl_pmu_event_stop() also need to clear the overfl=
ow=20
bit when stopping an event?

Currently, cxl_pmu_event_stop() in drivers/perf/cxl_pmu.c disables the coun=
ter=20
but doesn't clear any pending overflow bit in CXL_PMU_OVERFLOW_REG. If an=20
event overflows just before being stopped (e.g., during PMU multiplexing),=
=20
and a new event is started on the same counter or reset via=20
PERF_EVENT_IOC_RESET, the hardware overflow bit remains set.

When IRQs are re-enabled, cxl_pmu_irq() handles the pending IRQ and calls
__cxl_pmu_read() with overflow=3Dtrue. Because the counter was just reset
by cxl_pmu_event_start(), both new_cnt and prev_cnt will be 0.=20

Since new_cnt (0) >=3D prev_cnt (0), would __cxl_pmu_read() falsely apply t=
he=20
wraparound fixup:

	delta =3D (new_cnt - prev_cnt) & mask;
	if (overflow && new_cnt >=3D prev_cnt)
		delta +=3D mask + 1;

Could this erroneously add a full period (e.g., 2^64 ticks) to the newly
started or reset event and corrupt the counter statistics?

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260731232827.4014=
[email protected]?part=3D9