Re: [PATCH v10 05/12] cxl: Cache endpoint decoder settings during PCI enumeration
[email protected] Tue, 04 Aug 2026 19:51:51 +0000
| Newsgroups | org.kernel.vger.linux-cxl,org.kernel.vger.linux-pci |
|---|---|
| Message-ID | <[email protected]> |
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 c= hanging the HDM decoder count between the sizing read and the enumeration r= ead. - [High] Manually enabling PCI_COMMAND_MEMORY bypasses proper device and br= idge enablement flows, risking fatal PCIe errors. -- --- Patch [5]: [PATCH v10 05/12] cxl: Cache endpoint decoder settings durin= g PCI enumeration --- commit 14e10e2fbfc0ad9b68f5138b15e8cb4944bab222 Author: Srirangan Madhavan <[email protected]> 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 Spac= e. > 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 =3D cxl_pci_hdm_map(pdev, map, info); > + if (IS_ERR(hdm)) > + return PTR_ERR(hdm); > + > + decoder_count =3D 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 =3D pci_read_config_word(pdev, PCI_COMMAND, &command); > + if (rc) { > + rc =3D pcibios_err_to_errno(rc); > + goto out_free_info; > + } > + > + restore_command =3D !(command & PCI_COMMAND_MEMORY); > + if (restore_command) { > + rc =3D 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. --=20 Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260804192958.1823= [email protected]?part=3D5