[PATCH v3 09/10] 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]>
sashiko-bot flagged that ghes_handle_aer() has the same unvalidated
AER buffer handling plus a 4-byte over-read.

ghes_handle_aer() copies sizeof(struct aer_capability_regs) from the
fixed 96-byte pcie_err->aer_info, reading past the section since the
struct is larger. It also fills the software-only header_len and flit
fields of the embedded struct pcie_tlp_log from raw firmware bytes;
pcie_print_tlp_log() uses them to bound a loop over dw[], so a large
value walks past the array. There is also no check that the section can
hold a struct cper_sec_pcie.

Validate error_data_length, zero the destination, bound the copy to the
96-byte source, and clear header_len and flit. This mirrors the
extlog_print_pcie() fix.

Reported-by: [email protected]
Closes: https://sashiko.dev/#/patchset/[email protected]?part=3
Fixes: 7e077e6707b3 ("PCI/ERR: Handle TLP Log in Flit mode")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Dave Jiang <[email protected]>
---
v3:
- New patch. sashiko's review of v2 flagged the same unvalidated AER
  buffer handling in ghes_handle_aer() that v2 fixed for extlog.
---
 drivers/acpi/apei/ghes.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 1d2966a437bd..bd53509dcab3 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,22 @@ 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));
+
+		/*
+		 * aer_info is a fixed 96-byte buffer, smaller than struct
+		 * aer_capability_regs, so bound the copy to the source. Clear
+		 * the software-only header_len and flit fields afterwards so
+		 * firmware bytes 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, sizeof(pcie_err->aer_info));
+		aer_info->header_log.header_len = 0;
+		aer_info->header_log.flit = false;
 
 		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.55.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.