Re: [PATCH v3 4/4] drivers/perf: Fix the condition where the root port is runtime suspended

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Ignored return value of runtime PM resume leading to potential hardware access while in D3cold
- [High] System-wide PCI device wake-up during module initialization
- [High] Use-After-Free of PCI device structure due to mixing goto with guard() cleanup
--

--- Patch [4]: [PATCH v3 4/4] drivers/perf: Fix the condition where the root port is runtime suspended ---
commit 366af2eb4d6dbafe5fed360f0136d4ac3c45f174
Author: Yicong Yang <yang.yicong@picoheart.com>

drivers/perf: Fix the condition where the root port is runtime suspended

The patch adds runtime PM get/put when initializing and destroying PMU
events to ensure the PCIe root port is not suspended during VSEC capability
access. It also adds runtime PM guards during probe and system-wide
initialization.

Link: https://lore.kernel.org/linux-pci/20260629094234.F25E71F000E9@smtp.kernel.org/

> diff --git a/drivers/perf/dwc_pcie_pmu.c b/drivers/perf/dwc_pcie_pmu.c
> index 7b4ee71b966b0..aea387d0d0fe3 100644
> --- a/drivers/perf/dwc_pcie_pmu.c
> +++ b/drivers/perf/dwc_pcie_pmu.c

[ ... ]

> @@ -776,6 +794,7 @@ static int dwc_pcie_pmu_probe(struct faux_device *fdev)
>  	if (!name)
>  		return -ENOMEM;
>  
> +	guard(pm_runtime_active_auto)(&pdev->dev);

[Severity: High]
The guard macro will resume the device, but its return value is ignored.

If the device fails to resume, could we end up reading the PCI
configuration space of a suspended device in dwc_pcie_des_cap() right
after this?

>  	vsec = dwc_pcie_des_cap(pdev);
>  	if (!vsec)
>  		return -ENODEV;

[ ... ]

> @@ -909,6 +928,8 @@ static int __init dwc_pcie_pmu_init(void)
>  	dwc_pcie_pmu_hp_state = ret;
>  
>  	for_each_pci_dev(pdev) {
> +		guard(pm_runtime_active_auto)(&pdev->dev);
> +

[Severity: High]
Because this loop iterates over every PCI device in the system, does this
guard force every suspended PCI device to synchronously wake up before
we can check if it is actually the target device type?

Does this also share the same ignored return value issue as
dwc_pcie_pmu_probe(), where a failed resume goes unchecked?

Additionally, could this guard cause a use-after-free on the error path
later in the loop?

In dwc_pcie_pmu_init(), if dwc_pcie_register_dev() fails, we drop the
device reference and jump out of the block:

	ret = dwc_pcie_register_dev(pdev);
	if (ret) {
		pci_dev_put(pdev);
		goto err_cleanup;
	}

Since goto jumps out of the loop block, the compiler runs the cleanup
function for the guard (pm_runtime_put_autosuspend) after the manual
pci_dev_put() call.

If the manual put drops the last reference, does the cleanup function
dereference a freed pointer?

>  		if (!dwc_pcie_des_cap(pdev))
>  			continue;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708084507.98242-1-yang.yicong@picoheart.com?part=4