Re: [PATCH v5] cxl/pci: Skip reset detection for DVSEC emulated decoders

[sashiko-bot@kernel.org]

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

New issues:
- [High] Bypassing reset detection for DVSEC-emulated decoders masks hardware configuration loss on Secondary Bus Reset (SBR).

Pre-existing issues:
- [High] Concurrent read-modify-write data race on cxld->flags in cxl_endpoint_decoder_clear_reset_flags.
- [High] Missing exclusion against cxl_port unbind causes a potential use-after-free of devm-allocated cxlhdm.
--

commit 5cb99f1a9d781a36990dc77bd01012f502a6380b
Author: Guixin Liu <kanie@linux.alibaba.com>

cxl/pci: Skip reset detection for DVSEC emulated decoders

This commit changes the CXL PCIe reset detection to skip checking the
hardware reset state for HDM decoders that are emulated from DVSEC range
registers. It does this by returning early from
__cxl_endpoint_decoder_reset_detected() if cxld->commit is NULL.

Link: https://lore.kernel.org/linux-cxl/20260811113608.2815625-1-kanie@linux.alibaba.com/

> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
> index 9d807c1a002ca..d8b07f86bab09 100644
> --- a/drivers/cxl/core/pci.c
> +++ b/drivers/cxl/core/pci.c
> @@ -683,6 +683,13 @@ static int __cxl_endpoint_decoder_reset_detected(struct device *dev, void *data)
>  	if ((cxld->flags & CXL_DECODER_F_ENABLE) == 0)
>  		return 0;
>  
> +	/*
> +	 * Decoders emulated from the DVSEC range registers have no commit
> +	 * callback and no HDM decoder registers to consult.
> +	 */
> +	if (!cxld->commit)
> +		return 0;
> +

[Severity: High]
Does bypassing reset detection for DVSEC-emulated decoders in
__cxl_endpoint_decoder_reset_detected() mask hardware configuration loss
on Secondary Bus Reset (SBR)?

When a bus reset occurs on a CXL endpoint with DVSEC-emulated HDM
decoders, this early return unconditionally skips hardware reset
verification. On physical hardware, SBR wipes non-sticky PCI configuration
registers, disabling the DVSEC memory mapping. By skipping verification,
the OS will attempt to use disconnected memory, which could lead to a
system crash (Machine Check Exception) or data corruption.

>  	cxlhdm = dev_get_drvdata(&port->dev);
>  	hdm = cxlhdm->regs.hdm_decoder;

[Severity: High]
This is a pre-existing issue, but does the missing exclusion against
cxl_port unbind cause a potential use-after-free of the devm-allocated
cxlhdm?

In __cxl_endpoint_decoder_reset_detected(), a concurrent unbind of the
cxl_port driver while cxl_reset_done() iterates endpoints could allow the
driver core devres to free cxlhdm mid-execution, since the reset handler
fails to lock the port device. This use-after-free could lead to a kernel
panic, undefined behavior, or memory corruption.

>  	ctrl = readl(hdm + CXL_HDM_DECODER0_CTRL_OFFSET(cxld->id));

[ ... ]

Additionally, there is another pre-existing issue in the reset handling
path in drivers/cxl/pci.c:

drivers/cxl/pci.c:cxl_endpoint_decoder_clear_reset_flags() {
...
	cxled = to_cxl_endpoint_decoder(dev);
	cxled->cxld.flags &= ~CXL_DECODER_F_RESET_MASK;
...
}

[Severity: High]
This isn't a bug introduced by this patch, but does this non-atomic
read-modify-write operation access shared state concurrently modified
by region paths?

When concurrent device reset handling (calling
cxl_endpoint_decoder_clear_reset_flags) and region management paths
(like commit/teardown) modify the same cxld->flags word without holding
a shared lock (e.g., cxl_rwsem.region), concurrent flag changes can be
overwritten. This can cause state bits like CXL_DECODER_F_ENABLE or
CXL_DECODER_F_LOCK to be lost, leading to mapping issues or crashes.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831110449.719086-1-kanie@linux.alibaba.com?part=1