Re: [PATCH v19 12/14] cxl: Add port and dport identifiers to CXL AER trace events

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing device lock in cxl_cper_handle_prot_err() leads to Use-After-Free of devres-managed objects like 'dport' during concurrent driver unbind.
--

--- Patch [12]: [PATCH v19 12/14] cxl: Add port and dport identifiers to CXL AER trace events ---
commit 3fa84eaf0343ed1c7fb39fd22762efda3659de2c
Author: Dan Williams <djbw@kernel.org>

cxl: Add port and dport identifiers to CXL AER trace events

This commit modifies CXL AER trace events to use struct cxl_port and struct
cxl_dport pointers instead of deriving device strings at the caller. It
unifies the correctable and uncorrectable error trace events and updates
cxl_cper_handle_prot_err() to use these new helpers while dropping a device
lock it considered redundant.

> Changes in v18->v19:
> - Drop redundant device lock in cxl_cper_handle_prot_err(); the port
>   reference already keeps the object alive and no RAS iomap is accessed.

[Severity: High]
Does dropping the device lock here leave devres-managed objects unprotected?

The commit message notes that the port reference keeps the object alive, but a
device reference only protects the struct device allocation itself, not the
driver binding state or devm actions.

[ ... ]

> diff --git a/drivers/cxl/core/ras.c b/drivers/cxl/core/ras.c
> index 5183b3c532952..0254b7ec64c30 100644
> --- a/drivers/cxl/core/ras.c
> +++ b/drivers/cxl/core/ras.c

[ ... ]

> @@ -108,44 +76,32 @@ static struct cxl_port *find_cxl_port_by_dev(struct device *dev, struct cxl_dpor
>  
>  void cxl_cper_handle_prot_err(struct cxl_cper_prot_err_work_data *data)
>  {

[ ... ]

> -	guard(device)(&pdev->dev);
> -	if (!pdev->dev.driver)
> +	struct cxl_port *port __free(put_cxl_port) = find_cxl_port_by_dev(&pdev->dev, NULL);
> +	if (!port) {
> +		dev_err_ratelimited(&pdev->dev,
> +				    "Failed to find parent port device in CXL topology\n");
>  		return;
> +	}
>  
> -	struct device *mem_dev __free(put_device) = bus_find_device(
> -		&cxl_bus_type, NULL, pdev, match_memdev_by_parent);
> -	if (!mem_dev)
> -		return;
> +	/* dport is NULL for Endpoint and Upstream Port devices */
> +	dport = cxl_find_dport_by_dev(port, &pdev->dev);
>  
> -	cxlmd = to_cxl_memdev(mem_dev);
>  	if (data->severity == AER_CORRECTABLE)
> -		cxl_cper_trace_corr_prot_err(cxlmd, data->ras_cap);
> +		cxl_cper_trace_corr_prot_err(port, dport, pdev->dsn,
> +					     &data->ras_cap);
>  	else
> -		cxl_cper_trace_uncorr_prot_err(cxlmd, data->ras_cap);
> +		cxl_cper_trace_uncorr_prot_err(port, dport, pdev->dsn,
> +					       &data->ras_cap);
>  }

[Severity: High]
Can accessing dport here race with a driver unbind?

If a concurrent CXL port driver unbind occurs (via sysfs or device hot-remove),
the devres release actions will execute while cxl_cper_handle_prot_err() is
still running.

This could result in the devm-managed dport being freed, leaving the dport
pointer referencing freed memory. This pointer is then passed into the trace
events where cxl_trace_dport_name() attempts to read dport->dport_dev:

drivers/cxl/core/trace.c:cxl_trace_dport_name() {
    ...
    if (dport)
        return dev_name(dport->dport_dev);
    ...
}

Could this result in a use-after-free?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260803221810.3685703-1-terry.bowman@amd.com?part=12