Re: [PATCH v19 01/14] cxl/ras: Fix cxl_rch_get_aer_info() out-of-bounds AER register read

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] cxl_dport_map_rch_aer() requests an MMIO region using the software-only size of struct aer_capability_regs, over-reserving memory beyond the physical capability.
--

--- Patch [1]: [PATCH v19 01/14] cxl/ras: Fix cxl_rch_get_aer_info() out-of-bounds AER register read ---
commit b208d5e7d4a18c48dd06f33bc3c5667e9a2c13dd
Author: Terry Bowman <terry.bowman@amd.com>

cxl/ras: Fix cxl_rch_get_aer_info() out-of-bounds AER register read
    
cxl_rch_get_aer_info() copies the RCH Downstream Port AER capability from
the RCRB MMIO block using a readl() loop bounded by sizeof(struct
aer_capability_regs). This struct is a software layout and its embedded
struct pcie_tlp_log is larger than the on-wire AER capability. As a
result the loop reads past the mapped AER register block.

> diff --git a/drivers/cxl/core/ras_rch.c b/drivers/cxl/core/ras_rch.c
> index 0a8b3b9b63884..8a3965edaa12c 100644
> --- a/drivers/cxl/core/ras_rch.c
> +++ b/drivers/cxl/core/ras_rch.c
> @@ -58,13 +58,28 @@ void cxl_disable_rch_root_ints(struct cxl_dport *dport)
>  static bool cxl_rch_get_aer_info(void __iomem *aer_base,
>  				 struct aer_capability_regs *aer_regs)
>  {
> -	int read_cnt = sizeof(struct aer_capability_regs) / sizeof(u32);
> +	/*
> +	 * Bound the copy to the physically-defined AER registers (header
> +	 * through the 16-byte Header Log). struct aer_capability_regs is a
> +	 * software layout whose embedded struct pcie_tlp_log is larger than
> +	 * the on-wire AER capability; copying sizeof(*aer_regs) would
> +	 * over-read the RCRB-mapped MMIO block.
> +	 */
> +	int read_cnt = (PCI_ERR_HEADER_LOG + 16) / sizeof(u32);

[Severity: High]
This is a pre-existing issue, but since you are fixing the oversized read
here, should we also fix the initial mapping size in cxl_dport_map_rch_aer()?

In drivers/cxl/core/ras_rch.c:cxl_dport_map_rch_aer():

	dport->regs.dport_aer =
		devm_cxl_iomap_block(host, aer_phys,
				     sizeof(struct aer_capability_regs));

This uses the same oversized software struct size to request the MMIO region.

If another CXL Extended Capability is densely packed immediately following the
AER block, won't any subsequent driver attempting to map it (e.g.,
cxl_dport_map_rcd_linkcap()) fail with a -EBUSY conflict and break device
initialization?

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