[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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.