[PATCH v4 01/13] efi/cper: Reject CPER records with an out-of-range error_data_length

Dave Jiang <[email protected]>
Newsgroups org.kernel.vger.linux-cxl,org.kernel.vger.linux-acpi
Message-ID <[email protected]>
cper_estatus_check() sizes each section with acpi_hest_get_record_size(),
which adds the firmware-controlled u32 error_data_length to the header size
as a signed int (see <acpi/ghes.h>). A value in the top sizeof(*gdata)
bytes of the u32 range wraps the sum small rather than large, so it slips
past the "record_size > data_len" check: against the 72-byte v300 header,
0xffffffb9 sizes the record at 1 and acpi_hest_get_next() walks it a byte
at a time, off the end. 0xffffffb8 sizes it at 0 and loops forever.

Sum in u64 so the check sees the real size, and reject a size the int
helpers cannot carry, since the walk advances by their return value.

This is the per-section upper bound the later "len < sizeof(*foo)" guards
rely on; they are lower bounds only.

Reported-by: [email protected]
Closes: https://sashiko.dev/#/patchset/[email protected]?part=1
Fixes: 45b14a4ffcc1 ("efi: cper: Fix possible out-of-bounds access")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Dave Jiang <[email protected]>
---
v4:
- Do the arithmetic in u64 at the choke point instead of bounding the u32
  error_data_length against data_len, so the check no longer depends on the
  signed helpers in <acpi/ghes.h> behaving (Tony Luck). Bounding the u32
  still left a window when data_length itself was within a header of
  U32_MAX, and it did not stop acpi_hest_get_next() advancing by a wrapped
  int. The v3 "< 0" arm was dead either way, since data_len is unsigned and
  promoted the int back (Tony Luck, Shuai Xue).
- Dropped Alison's and Shuai's Reviewed-by; the check was reworked after
  they reviewed it.
---
 drivers/firmware/efi/cper.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/firmware/efi/cper.c b/drivers/firmware/efi/cper.c
index 06b4fdb59917..ec092729cacc 100644
--- a/drivers/firmware/efi/cper.c
+++ b/drivers/firmware/efi/cper.c
@@ -752,7 +752,7 @@ EXPORT_SYMBOL_GPL(cper_estatus_check_header);
 int cper_estatus_check(const struct acpi_hest_generic_status *estatus)
 {
 	struct acpi_hest_generic_data *gdata;
-	unsigned int data_len, record_size;
+	unsigned int data_len;
 	int rc;
 
 	rc = cper_estatus_check_header(estatus);
@@ -762,11 +762,21 @@ int cper_estatus_check(const struct acpi_hest_generic_status *estatus)
 	data_len = estatus->data_length;
 
 	apei_estatus_for_each_section(estatus, gdata) {
+		u64 record_size;
+
 		if (acpi_hest_get_size(gdata) > data_len)
 			return -EINVAL;
 
-		record_size = acpi_hest_get_record_size(gdata);
-		if (record_size > data_len)
+		/*
+		 * acpi_hest_get_record_size() sums these as a signed int (see
+		 * <acpi/ghes.h>), which wraps small for a huge
+		 * error_data_length and slips past the check below. Sum in u64,
+		 * and reject what those helpers cannot carry, since the walk
+		 * advances by their return value.
+		 */
+		record_size = (u64)acpi_hest_get_size(gdata) +
+			      gdata->error_data_length;
+		if (record_size > data_len || record_size > INT_MAX)
 			return -EINVAL;
 
 		data_len -= record_size;
-- 
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.