[PATCH v5 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 copy only what maps onto the struct: the leading registers and the four Header Log DWORDs, mirroring the extlog_print_pcie() fix. 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]> Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Dave Jiang <[email protected]> --- v5: - Copy only the registers that match the hardware layout rather than all 96 bytes plus explicit clears, matching the same change in the extlog path (Jonathan Cameron). - Note the copy length changed, so the printed TLP prefix log changes too; Alison's and Shuai's Reviewed-by are kept since the intent and location are the same, but please re-check if you disagree. --- drivers/acpi/apei/ghes.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c index 0cc1e6383635..3df62ec20cf9 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,21 @@ 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)); + + /* + * Copy only what maps onto the struct, as extlog_print_pcie() + * does: the leading registers and the four Header Log DWORDs. + * 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)); 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