Re: [PATCH] PCI/PTM: fix refcount leak in pci_enable_ptm()

[sashiko-bot@kernel.org]

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

New issues:
- [Low] The patch introduces a strict compilation failure because the variable `parent` is referenced outside of its declared scope in the error path.

Pre-existing issues:
- [High] Concurrent calls to `pci_enable_ptm()` can lead to a lockless TOCTOU race condition where a caller incorrectly assumes PTM is enabled before the hardware initialization completes.
--

commit 98f3a56b29bf607b6f9174e20f676b1549c2aaaa
Author: Wentao Liang <vulab@iscas.ac.cn>

PCI/PTM: fix refcount leak in pci_enable_ptm()

This commit addresses a refcount leak in pci_enable_ptm() when the function
fails to enable PTM on a child device. It does this by adding a call to
pci_disable_ptm() for the parent device in the error path.

Cc: stable@vger.kernel.org

> diff --git a/drivers/pci/pcie/ptm.c b/drivers/pci/pcie/ptm.c
> index a41ffd1914de4..01f6c7da7ca91 100644
> --- a/drivers/pci/pcie/ptm.c
> +++ b/drivers/pci/pcie/ptm.c
> @@ -201,6 +201,8 @@ int pci_enable_ptm(struct pci_dev *dev)
>  
>  	rc = __pci_enable_ptm(dev);

[Severity: High]
This is a pre-existing issue, but can the lockless check just above this
call lead to a race condition?

Looking at pci_enable_ptm(), the counter check bypasses hardware
configuration locklessly:

        if (atomic_inc_return(&dev->ptm_enable_cnt) > 1)
                return 0;
        
        rc = __pci_enable_ptm(dev);

If multiple downstream devices probe concurrently, two threads might call
pci_enable_ptm() on their shared parent simultaneously. If the second thread
increments the counter to 2 and returns immediately, it will assume the
parent is fully initialized. It could then proceed to configure the child
before the first thread finishes hardware configuration in __pci_enable_ptm(),
potentially causing Unsupported Request errors or bus resets.

>  	if (rc) {
> +		if (!dev->ptm_root)
> +			pci_disable_ptm(parent);

[Severity: Low]
Does this cause a build failure? The variable parent is referenced here,
but looking at pci_enable_ptm(), parent is declared locally inside an earlier
if block:

        if (!dev->ptm_root) {
                struct pci_dev *parent;

                parent = pci_upstream_ptm(dev);
                ...
        }

This makes parent out of scope in this error path, which will break the build.

>  		atomic_dec(&dev->ptm_enable_cnt);
>  		return rc;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260616141733.1688264-1-vulab@iscas.ac.cn?part=1