Re: [PATCH v10 05/12] cxl: Cache endpoint decoder settings during PCI enumeration

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] A malicious CXL device can trigger an out-of-bounds MMIO read by changing the HDM decoder count between the sizing read and the enumeration read.
- [High] Manually enabling PCI_COMMAND_MEMORY bypasses proper device and bridge enablement flows, risking fatal PCIe errors.
--

--- Patch [5]: [PATCH v10 05/12] cxl: Cache endpoint decoder settings during PCI enumeration ---
commit 14e10e2fbfc0ad9b68f5138b15e8cb4944bab222
Author: Srirangan Madhavan <smadhavan@nvidia.com>

cxl: Cache endpoint decoder settings during PCI enumeration

This patch populates the pci_dev->hdm for CXL.mem functions early during
PCI enumeration from pci_bus_add_device(). This provides an early HDM
snapshot for driver-free reset paths while temporarily enabling Memory Space.

> diff --git a/drivers/cxl/core/resource.c b/drivers/cxl/core/resource.c
> index 97cb136cb2aeb..7f5946d3f2c4e 100644
> --- a/drivers/cxl/core/resource.c
> +++ b/drivers/cxl/core/resource.c
[ ... ]
> +static int cxl_pci_hdm_read_info(struct pci_dev *pdev,
> +				 struct cxl_register_map *map,
> +				 struct cxl_hdm_info *info)
> +{
[ ... ]
> +	hdm = cxl_pci_hdm_map(pdev, map, info);
> +	if (IS_ERR(hdm))
> +		return PTR_ERR(hdm);
> +
> +	decoder_count = cxl_hdm_decoder_count(readl(hdm +
> +						    CXL_HDM_DECODER_CAP_OFFSET));

[Severity: High]
Could a malicious CXL device trigger an out-of-bounds MMIO read here?

During the capability initialization, the component register block is mapped
using a size calculated from a first read of the capabilities. When
cxl_pci_hdm_read_info() maps this memory, it reads the capability register a
second time to get decoder_count.

If the hardware returns a larger count on this second read, the subsequent
loop over decoder_count will read from MMIO offsets that exceed the ioremap
size. If this crosses a page boundary into an unmapped page, it could cause
a synchronous page fault or kernel panic.

[ ... ]
> +static int __pci_cxl_hdm_init(struct pci_dev *pdev)
> +{
[ ... ]
> +	rc = pci_read_config_word(pdev, PCI_COMMAND, &command);
> +	if (rc) {
> +		rc = pcibios_err_to_errno(rc);
> +		goto out_free_info;
> +	}
> +
> +	restore_command = !(command & PCI_COMMAND_MEMORY);
> +	if (restore_command) {
> +		rc = pci_write_config_word(pdev, PCI_COMMAND,
> +					   command | PCI_COMMAND_MEMORY);

[Severity: High]
Does manually enabling PCI_COMMAND_MEMORY here risk fatal PCIe errors?

Because __pci_cxl_hdm_init() is called from pci_bus_add_device() before
drivers are bound and before pci_enable_device_mem() has enabled the
upstream bridge windows, asserting PCI_COMMAND_MEMORY directly on the
endpoint might create a PCIe topology hazard.

If the upstream bridges are not fully enabled (for example, with hotplugged
devices), they will drop the subsequent MMIO reads to the decoder registers.
This results in an Unsupported Request (UR) completion, which typically
triggers a Machine Check Exception or synchronous external abort.

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