[PATCH v6 12/13] ACPI: APEI: GHES: Bound AER info copy and sanitize software metadata
Dave Jiang <[email protected]>
| Newsgroups | org.kernel.vger.linux-acpi,org.kernel.vger.linux-cxl |
|---|---|
| Message-ID | <[email protected]> |
ghes_handle_aer() copies sizeof(struct aer_capability_regs) out of the fixed 96-byte pcie_err->aer_info. The struct is larger, so the copy reads past the section, and it fills the software-only header_len and flit fields of the embedded struct pcie_tlp_log from firmware bytes. pcie_print_tlp_log() uses both to bound a loop over dw[], so a large value walks past the array. Nothing checks the section can hold a struct cper_sec_pcie either. Validate error_data_length, zero the destination, and map aer_info onto the struct as extlog_print_pcie() does: copy up to the four Header Log DWORDs, then place the TLP Prefix Log from its own offset. The rest stays zero, covering header_len and flit. Reported-by: [email protected] Closes: https://sashiko.dev/#/patchset/[email protected]?part=3 Fixes: 7e077e6707b3 ("PCI/ERR: Handle TLP Log in Flit mode") Reviewed-by: Alison Schofield <[email protected]> Reviewed-by: Shuai Xue <[email protected]> Reviewed-by: Hanjun Guo <[email protected]> Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Dave Jiang <[email protected]> --- v6: - Also place the TLP Prefix Log from its own hardware offset rather than leaving it zero. It is the one field the print path reads that the shortened copy missed (sashiko). --- drivers/acpi/apei/ghes.c | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c index 2077cc90569a..08c985e729d6 100644 --- a/drivers/acpi/apei/ghes.c +++ b/drivers/acpi/apei/ghes.c @@ -642,11 +642,14 @@ static void ghes_handle_aer(struct acpi_hest_generic_data *gdata) #ifdef CONFIG_ACPI_APEI_PCIEAER struct cper_sec_pcie *pcie_err = acpi_hest_get_payload(gdata); + if (gdata->error_data_length < sizeof(*pcie_err)) + return; + if (pcie_err->validation_bits & CPER_PCIE_VALID_DEVICE_ID && pcie_err->validation_bits & CPER_PCIE_VALID_AER_INFO) { + struct aer_capability_regs *aer_info; unsigned int devfn; int aer_severity; - u8 *aer_info; devfn = PCI_DEVFN(pcie_err->device_id.device, pcie_err->device_id.function); @@ -664,13 +667,25 @@ static void ghes_handle_aer(struct acpi_hest_generic_data *gdata) sizeof(struct aer_capability_regs)); if (!aer_info) return; - memcpy(aer_info, pcie_err->aer_info, sizeof(struct aer_capability_regs)); + + /* + * Map aer_info onto the struct as extlog_print_pcie() does: + * copy up to the four Header Log DWORDs, then place the TLP + * Prefix Log from where the hardware keeps it. The rest stays + * zero, so firmware cannot drive the pcie_print_tlp_log() loop + * over dw[] out of bounds. + */ + memset(aer_info, 0, sizeof(struct aer_capability_regs)); + memcpy(aer_info, pcie_err->aer_info, + offsetof(struct aer_capability_regs, header_log) + + PCIE_STD_NUM_TLP_HEADERLOG * sizeof(u32)); + memcpy(aer_info->header_log.prefix, + pcie_err->aer_info + PCI_ERR_PREFIX_LOG, + sizeof(aer_info->header_log.prefix)); aer_recover_queue(pcie_err->device_id.segment, pcie_err->device_id.bus, - devfn, aer_severity, - (struct aer_capability_regs *) - aer_info); + devfn, aer_severity, aer_info); } #endif } -- 2.54.0