Re: [PATCH v7 10/11] PCI/cxl: Expose CXL Reset as a PCI reset method

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Accessing devres-managed `pdev->hdm` before holding sibling device locks leads to a Time-of-Check-to-Time-of-Use (TOCTOU) Use-After-Free during concurrent driver unbind or hot-unplug.
--

commit c1746289413d4e364b4a471be081d1821258af44
Author: Srirangan Madhavan <smadhavan@nvidia.com>

PCI/cxl: Expose CXL Reset as a PCI reset method
    
This commit adds the CXL Reset helper to the PCI reset-method table, allowing
userspace to select it via the reset_method ABI. It introduces checks to verify
that cached HDM decoder state is available for the target and mem-capable
siblings before executing the reset.

> diff --git a/drivers/cxl/core/reset.c b/drivers/cxl/core/reset.c
> index d801c91a5cbf8..694d8a3789a42 100644
> --- a/drivers/cxl/core/reset.c
> +++ b/drivers/cxl/core/reset.c
> @@ -1035,6 +1035,34 @@ static int cxl_reset_dvsec(struct pci_dev *pdev)
>  	return dvsec;
>  }
>  
> +static bool cxl_reset_hdm_available(struct pci_dev *pdev)
> +{
> +	struct cxl_hdm_info *info = READ_ONCE(pdev->hdm);
> +
> +	/*
> +	 * pdev->hdm is allocated with PCI-device devres. Reset requests
> +	 * operate on a live pci_dev, so the devres allocation remains valid
> +	 * for this check.
> +	 */
> +	return info && info->regs.hdm_decoder;

[Severity: High]
Does this assumption hold true during a concurrent driver unbind or hot-unplug
event?

Devres is tied to the driver binding lifecycle, which is protected by the
device lock, not just a reference to the live pci_dev.

If a sibling device's driver is unbound concurrently, devres_release_all()
will free the info structure. Since READ_ONCE(pdev->hdm) could read a valid
pointer right before it is freed, couldn't the subsequent access to
info->regs.hdm_decoder result in a use-after-free?

[ ... ]

> @@ -1211,6 +1239,11 @@ int cxl_reset_function(struct pci_dev *pdev, bool probe)
>  	if (rc)
>  		goto out;
>  
> +	if (!cxl_reset_scope_hdm_available(&ctx)) {
> +		rc = -ENOTTY;
> +		goto out;
> +	}
> +

[Severity: High]
Is it safe to call cxl_reset_scope_hdm_available() before holding the sibling
device locks?

Because the check iterates through siblings and accesses their devres-managed
pdev->hdm pointers without the device locks held, there appears to be a race
condition against concurrent unbinds.

Could moving cxl_reset_scope_hdm_available() to after cxl_pci_functions_lock()
protect the devres structures from being freed during this check?

>  	rc = cxl_pci_functions_lock(&ctx);
>  	if (rc)
>  		goto out_unlock;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260623032453.3404772-1-smadhavan@nvidia.com?part=10