All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Jiang <dave.jiang@intel.com>
To: linux-acpi@vger.kernel.org, linux-cxl@vger.kernel.org
Cc: rafael@kernel.org, tony.luck@intel.com, bp@alien8.de,
	guohanjun@huawei.com, mchehab@kernel.org,
	xueshuai@linux.alibaba.com, terry.bowman@amd.com,
	benjamin.cheatham@amd.com, alison.schofield@intel.com,
	sashiko-bot@kernel.org
Subject: [PATCH v4 01/13] efi/cper: Reject CPER records with an out-of-range error_data_length
Date: Mon, 24 Aug 2026 10:49:24 -0700	[thread overview]
Message-ID: <20260824174936.939059-2-dave.jiang@intel.com> (raw)
In-Reply-To: <20260824174936.939059-1-dave.jiang@intel.com>

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: sashiko-bot@kernel.org
Closes: https://sashiko.dev/#/patchset/20260714231835.303081-1-dave.jiang@intel.com?part=1
Fixes: 45b14a4ffcc1 ("efi: cper: Fix possible out-of-bounds access")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
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


  reply	other threads:[~2026-08-24 17:49 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-24 17:49 [PATCH v4 00/13] ACPI: APEI: GHES: Collection of fixes for issues reported by sashiko Dave Jiang
2026-08-24 17:49 ` Dave Jiang [this message]
2026-08-24 18:10   ` [PATCH v4 01/13] efi/cper: Reject CPER records with an out-of-range error_data_length sashiko-bot
2026-08-24 21:57   ` Jonathan Cameron
2026-08-25 16:31     ` Dave Jiang
2026-08-24 17:49 ` [PATCH v4 02/13] efi/cper: Reject an error status block length that wraps a u32 Dave Jiang
2026-08-24 18:03   ` sashiko-bot
2026-08-24 22:22   ` Jonathan Cameron
2026-08-24 17:49 ` [PATCH v4 03/13] ACPI: extlog: Validate elog record length before walking sections Dave Jiang
2026-08-24 18:07   ` sashiko-bot
2026-08-24 22:28   ` Jonathan Cameron
2026-08-25 17:15     ` Dave Jiang
2026-08-24 17:49 ` [PATCH v4 04/13] ACPI: extlog: Defer CXL protocol error handling to avoid lock inversion Dave Jiang
2026-08-24 18:03   ` sashiko-bot
2026-08-24 22:29   ` Jonathan Cameron
2026-08-24 17:49 ` [PATCH v4 05/13] ACPI: extlog: Avoid populating software AER metadata from raw hardware buffer Dave Jiang
2026-08-24 18:13   ` sashiko-bot
2026-08-24 23:05   ` Jonathan Cameron
2026-08-25 17:38     ` Dave Jiang
2026-08-24 17:49 ` [PATCH v4 06/13] ACPI: extlog: Validate PCIe error section length before payload access Dave Jiang
2026-08-24 18:07   ` sashiko-bot
2026-08-24 23:08   ` Jonathan Cameron
2026-08-24 17:49 ` [PATCH v4 07/13] ACPI: extlog: Fix CONFIG_ACPI_APEI_PCIEAER guard typo Dave Jiang
2026-08-24 18:15   ` sashiko-bot
2026-08-24 23:11   ` Jonathan Cameron
2026-08-24 17:49 ` [PATCH v4 08/13] ACPI: APEI: GHES: Bound CXL event record copy to the firmware section length Dave Jiang
2026-08-24 23:13   ` Jonathan Cameron
2026-08-24 17:49 ` [PATCH v4 09/13] ACPI: APEI: GHES: Validate CXL protocol error section length before RAS cap copy Dave Jiang
2026-08-24 18:10   ` sashiko-bot
2026-08-24 23:14   ` Jonathan Cameron
2026-08-24 17:49 ` [PATCH v4 10/13] efi/cper: Read only validated fields in cper_mem_err_pack() Dave Jiang
2026-08-24 18:08   ` sashiko-bot
2026-08-24 17:49 ` [PATCH v4 11/13] ACPI: APEI: GHES: Validate memory error section length before payload access Dave Jiang
2026-08-24 18:17   ` sashiko-bot
2026-08-24 17:49 ` [PATCH v4 12/13] ACPI: APEI: GHES: Bound AER info copy and sanitize software metadata Dave Jiang
2026-08-24 18:17   ` sashiko-bot
2026-08-24 21:32     ` Dave Jiang
2026-08-24 17:49 ` [PATCH v4 13/13] cxl/ras: Make cxl_cper_handle_prot_err() static Dave Jiang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260824174936.939059-2-dave.jiang@intel.com \
    --to=dave.jiang@intel.com \
    --cc=alison.schofield@intel.com \
    --cc=benjamin.cheatham@amd.com \
    --cc=bp@alien8.de \
    --cc=guohanjun@huawei.com \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-cxl@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=rafael@kernel.org \
    --cc=sashiko-bot@kernel.org \
    --cc=terry.bowman@amd.com \
    --cc=tony.luck@intel.com \
    --cc=xueshuai@linux.alibaba.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.