Re: [PATCH v18 07/13] PCI/CXL: Add RCH support to CXL handlers

On 7/17/2026 5:43 PM, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> New issues:
> - [High] Removal of explicit PCI error handler forwarding for RCH devices leaves RCDs bound after a fatal PCI error.
This is a false positive. device_release_driver() is called in cxl_pci_error_detected() 

> 
> Pre-existing issues:
> - [High] Preexisting lockless dereference of `dport` returned by `cxl_pci_find_port()` and `cxl_mem_find_port()` leading to Use-After-Free.
This is existing issue and is not planned to be fixed in this series.

> --
> 
> --- Patch [7]: [PATCH v18 07/13] PCI/CXL: Add RCH support to CXL handlers ---
> commit 6a99c70b18865d01d350aa367342e701ebe6dde2
> Author: Terry Bowman <[email protected]>
> 
> PCI/CXL: Add RCH support to CXL handlers
> 
> Restricted CXL Host (RCH) error handling is folded into the new CXL Port
> error handling flow. RCH protocol errors are forwarded through the AER-CXL
> kfifo, and the manual CXL.mem error handler invocation is removed.
> 
>> diff --git a/drivers/cxl/core/ras_rch.c b/drivers/cxl/core/ras_rch.c
>> --- a/drivers/cxl/core/ras_rch.c
>> +++ b/drivers/cxl/core/ras_rch.c
>> @@ -96,18 +95,21 @@ static bool cxl_rch_get_aer_severity(struct aer_capability_regs *aer_regs,
>>  	return false;
>>  }
>>  
>> -void cxl_handle_rdport_errors(struct cxl_dev_state *cxlds)
>> +void cxl_handle_rdport_errors(struct pci_dev *pdev)
>>  {
>> -	struct pci_dev *pdev = to_pci_dev(cxlds->dev);
>>  	struct aer_capability_regs aer_regs;
>>  	struct cxl_dport *dport;
>>  	int severity;
>>  
>> -	struct cxl_port *port __free(put_cxl_port) =
>> -		cxl_pci_find_port(pdev, &dport);
>> +	struct cxl_port *port __free(put_cxl_port) = cxl_pci_find_port(pdev, NULL);
>>  	if (!port)
>>  		return;
>>  
>> +	guard(device)(&port->dev);
>> +	dport = cxl_find_dport_by_dev(port, pdev->dev.parent);
> 
> [Severity: High]
> This is a pre-existing issue, but while this change correctly closes a
> lockless Use-After-Free window for dport here, does the same vulnerability
> still exist in cxl_pci_setup_regs() and rcd_pcie_cap_emit()?
> 
> In cxl_pci_setup_regs(), cxl_rcrb_get_comp_regs() dereferences the dport
> returned by cxl_pci_find_port() before any lock is taken.
> 
> Similarly, in rcd_pcie_cap_emit(), dport->regs.rcd_pcie_cap is dereferenced
> entirely locklessly, and then the function takes guard(device)(root_dev)
> instead of the required &root->dev lock.
> 
> Since rcd_pcie_cap_emit() is reachable via sysfs attributes, could an
> unprivileged local user trigger a Use-After-Free by reading sysfs during a
> concurrent device hot-unplug?
> 
> [ ... ]
> 

This is existing issue and is not planned to be fixed in this series.

>> diff --git a/drivers/pci/pcie/aer_cxl_rch.c b/drivers/pci/pcie/aer_cxl_rch.c
>> --- a/drivers/pci/pcie/aer_cxl_rch.c
>> +++ b/drivers/pci/pcie/aer_cxl_rch.c
>> @@ -34,42 +34,37 @@ static bool cxl_error_is_native(struct pci_dev *dev)
>>  	return (pcie_ports_native || host->native_aer);
>>  }
>>  
>> +struct cxl_rch_error_ctx {
>> +	struct aer_err_info *info;
>> +	bool enqueued;
>> +};
>> +
>>  static int cxl_rch_handle_error_iter(struct pci_dev *dev, void *data)
>>  {
>> -	struct aer_err_info *info = (struct aer_err_info *)data;
>> -	const struct pci_error_handlers *err_handler;
>> +	struct cxl_rch_error_ctx *ctx = data;
>>  
>>  	if (!is_cxl_mem_dev(dev) || !cxl_error_is_native(dev))
>>  		return 0;
>>  
>> -	guard(device)(&dev->dev);
>> -
>> -	err_handler = dev->driver ? dev->driver->err_handler : NULL;
>> -	if (!err_handler)
>> -		return 0;
>> -
>> -	if (info->severity == AER_CORRECTABLE) {
>> -		if (err_handler->cor_error_detected)
>> -			err_handler->cor_error_detected(dev);
>> -	} else if (err_handler->error_detected) {
>> -		if (info->severity == AER_NONFATAL)
>> -			err_handler->error_detected(dev, pci_channel_io_normal);
>> -		else if (info->severity == AER_FATAL)
>> -			err_handler->error_detected(dev, pci_channel_io_frozen);
>> -	}
>> +	if (cxl_forward_error(dev, ctx->info))
>> +		ctx->enqueued = true;
> 
> [Severity: High]
> Does removing the explicit invocation of the err_handler leave RCDs bound
> after a fatal PCI error?
> 
> The standard pcie_do_recovery() on an RCEC does not iterate over its
> associated RC_END (RCD) devices, so this manual invocation used to handle the
> CXL.mem driver fallback. Now that errors are routed solely through the CXL
> protocol error work queue (__cxl_proto_err_work_fn()), the work queue only
> panics on uncorrectable cachemem errors.
> 
> Does the work queue replicate the PCI recovery state machine's fallback
> behavior, such as calling device_release_driver() to unbind the CXL.mem
> driver on frozen channel states?
> 
>>  	return 0;
>>  }
> 

The change to forward CXL errors through Kfifo using schedulable work is 
intentional. This series does not exactly *replicate* the PCI handling.

The device_release_driver() is called in cxl_pci_error_detected() for 
pci_channel_io_frozen. cxl_pci_error_detected() serves as the PCIe error 
handler for Endpoints, including RCDs.

Terry