Re: [PATCH v5 05/13] ACPI: extlog: Avoid populating software AER metadata from raw hardware buffer

[sashiko-bot@kernel.org]

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

New issues:
- [Medium] Data loss of TLP Prefix Log / Extended Flit Log due to arbitrary payload truncation.

Pre-existing issues:
- [High] Out-of-bounds MMIO read and stack array bounds poisoning in `cxl_rch_get_aer_info()`.
--

commit 99282f77735a9b69f6cccf74ca39147b80930bf8
Author: Dave Jiang <dave.jiang@intel.com>
ACPI: extlog: Avoid populating software AER metadata from raw hardware buffer

This commit addresses an issue in extlog_print_pcie() where casting the raw
CPER aer_info buffer to a software struct populates software-only fields
with raw hardware bytes. It copies only the leading registers and the
Header Log DWORDs into a zeroed local struct to prevent out-of-bounds loop
reads.

Link: https://lore.kernel.org/linux-cxl/20260709165457.8BA181F000E9@smtp.kernel.org/

> diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c
> --- a/drivers/acpi/acpi_extlog.c
> +++ b/drivers/acpi/acpi_extlog.c
[ ... ]
> @@ -149,7 +149,17 @@ static void extlog_print_pcie(struct cper_sec_pcie *pcie_err,
>  		return;
>  
>  	aer_severity = cper_severity_to_aer(severity);
> -	aer = (struct aer_capability_regs *)pcie_err->aer_info;
> +
> +	/*
> +	 * struct pcie_tlp_log is larger than the hardware layout, so only the
> +	 * leading registers and the four Header Log DWORDs of aer_info map onto
> +	 * the struct. Copy that much and leave the rest zero, which covers the
> +	 * software-only header_len and flit.
> +	 */
> +	memcpy(&aer_regs, pcie_err->aer_info,
> +	       offsetof(struct aer_capability_regs, header_log) +
> +	       PCIE_STD_NUM_TLP_HEADERLOG * sizeof(u32));

[Severity: Medium]
Will this truncated copy result in the permanent loss of the TLP Prefix Log?

The CPER aer_info array contains a raw 96-byte snapshot of the hardware AER
capability, with the TLP Prefix Log located at offset 0x38 (56 bytes). By
truncating the memcpy to 44 bytes, the remainder of the CPER payload is
ignored, which silently drops the hardware-provided TLP Prefix Logs or
Extended Flit Logs during PCIe error handling.

Would it be better to explicitly extract the prefix log from offset 56 in
the CPER record into aer_regs.header_log.prefix to avoid data loss?

[ ... ]

This isn't a bug introduced by this patch, but I noticed a similar 
pre-existing AER software struct issue in drivers/cxl/core/ras_rch.c.

drivers/cxl/core/ras_rch.c:cxl_rch_get_aer_info()
> 	int read_cnt = sizeof(struct aer_capability_regs) / sizeof(u32);
> 	u32 *aer_regs_buf = (u32 *)aer_regs;
> 	int n;
> ...
> 	for (n = 0; n < read_cnt; n++)
> 		aer_regs_buf[n] = readl(aer_base + n * sizeof(u32));

[Severity: High]
Does this loop perform an out-of-bounds MMIO read by using the size of the
software structure?

Using sizeof(struct aer_capability_regs) (104 bytes) instead of the actual
hardware capability size directly maps MMIO data into software fields
(header_len and flit at offset 84). Later, when pcie_print_tlp_log() runs,
it uses the MMIO-poisoned header_len (which can be up to 255) as a loop
bound over the 14-element stack array log->dw[]. This leads to a massive
out-of-bounds stack read that can log arbitrary stack memory.

Additionally, in the same file:

drivers/cxl/core/ras_rch.c:cxl_rch_get_aer_severity()
> 		if (aer_regs->uncor_status & PCI_ERR_ROOT_FATAL_RCV)
> 			*severity = AER_FATAL;

[Severity: High]
Does this code misinterpret the flow control protocol error bit?

It tests the Root Error Status flag PCI_ERR_ROOT_FATAL_RCV (0x40) against
the uncor_status register. In the uncor_status register context, 0x40
actually represents PCI_ERR_UNC_FCP, meaning the severity is incorrectly
computed.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260827203726.3027541-1-dave.jiang@intel.com?part=5