Re: [PATCH v3 07/10] ACPI: extlog: Fix CONFIG_ACPI_APEI_PCIEAER guard typo

[sashiko-bot@kernel.org]

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

New issues:
- [Medium] The commit message incorrectly claims to fix CXL protocol error handling, but the patch modifies the standard PCIe error handling function.

Pre-existing issues:
- [High] Software-only metadata in `struct aer_capability_regs` is overwritten with unvalidated hardware data, leading to massive out-of-bounds reads during TLP log printing, compounded by out-of-bounds copies from smaller buffers.
- [High] Missing bounds validation on firmware-provided `data_length` allows heap out-of-bounds read in `extlog_print()`.
- [Medium] Bulk-copying the raw hardware `aer_info` buffer into `struct aer_capability_regs` causes a layout mismatch, resulting in Root Port command/status registers being erroneously printed as End-to-End TLP Prefixes.
- [Medium] AER statistics are updated without locking, leading to potential data corruption and lost increments.
--

--- Patch [7]: [PATCH v3 07/10] ACPI: extlog: Fix CONFIG_ACPI_APEI_PCIEAER guard typo ---
commit 5975445fed69f606ac825570bf67bde4a1f490fd
Author: Dave Jiang <dave.jiang@intel.com>
Date:   Fri Jul 17 16:16:44 2026 +0000

    ACPI: extlog: Fix CONFIG_ACPI_APEI_PCIEAER guard typo

    sashiko-bot flagged that a Kconfig macro typo compiles out the extlog
    CXL protocol error handling entirely.

> sashiko-bot flagged that a Kconfig macro typo compiles out the extlog
> CXL protocol error handling entirely.

[Severity: Medium]
Is this commit message accurate? The modified function is extlog_print_pcie(),
which parses CPER_SEC_PCIE records (standard PCIe errors), rather than
CPER_SEC_CXL_PROT_ERR (CXL protocol errors).

> diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c
> index ae79d090de33b..1fbaf134c04e3 100644
> --- a/drivers/acpi/acpi_extlog.c
> +++ b/drivers/acpi/acpi_extlog.c
> @@ -136,7 +136,7 @@ static int print_extlog_rcd(const char *pfx,
>  static void extlog_print_pcie(struct cper_sec_pcie *pcie_err,
>  			      int severity, u32 len)
>  {
> -#ifdef ACPI_APEI_PCIEAER
> +#ifdef CONFIG_ACPI_APEI_PCIEAER
>  	struct aer_capability_regs aer_regs = {};
>  	struct aer_capability_regs *aer;
>  	struct pci_dev *pdev;

[ ... ]

>  	aer_severity = cper_severity_to_aer(severity);
> 
>  	memcpy(&aer_regs, pcie_err->aer_info, sizeof(pcie_err->aer_info));

[Severity: Medium]
This is a pre-existing issue, but does this bulk copy cause a layout
mismatch? 

The raw hardware aer_info buffer is copied into the Linux struct
aer_capability_regs. Because the software struct embeds a struct pcie_tlp_log
at offset 28, the hardware registers following the 16-byte Header Log might be
erroneously copied into aer_regs.header_log.prefix[]. 

Could this cause pcie_print_tlp_log() to incorrectly print Root Error Command
registers as End-to-End TLP Prefixes?

[Severity: High]
This is a pre-existing issue, but can this overwrite software-only metadata
fields in struct aer_capability_regs?

While this function initializes header_len and flit after the copy, other
functions like ghes_handle_aer() copy 100 bytes (sizeof struct
aer_capability_regs) from a 96-byte hardware buffer, reading memory
out-of-bounds. 

Could unvalidated hardware data corrupting fields like header_len and flit
lead to massive out-of-bounds reads during pcie_print_tlp_log() over the
log->dw[] array?

>  	aer_regs.header_log.header_len = 0;
>  	aer_regs.header_log.flit = false;
>  	aer = &aer_regs;

[Severity: High]
This is a pre-existing issue, but does extlog_print() lack bounds validation
on the firmware-provided data_length?

drivers/acpi/acpi_extlog.c:extlog_print() {
    ...
	memcpy(elog_buf, (void *)estatus, ELOG_ENTRY_LEN);
    ...
	apei_estatus_for_each_section(tmp, gdata) {
    ...
}

Since elog_buf is allocated with a fixed 4096-byte limit, could a
maliciously large data_length from firmware cause the macro to walk off the
end of the heap allocation?

[Severity: Medium]
This is a pre-existing issue, but are the AER statistics updated without
locking?

drivers/pci/pcie/aer.c:pci_dev_aer_stats_incr() {
    ...
    counter[i]++;
    ...
}

Since pci_dev_aer_stats_incr() modifies 64-bit counters concurrently from
multiple execution contexts (like the threaded interrupt handler and
workqueues), could this data race result in corrupted statistics and lost
increments?

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