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

On 8/3/2026 5:42 PM, [email protected] wrote:
> 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 <[email protected]>
> 
> 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?
> 
I will be adding a patch fix to v20 for the existing cxl_dport_map_rch_aer() 
issue reported by sashiko.

- Terry