[PATCH AUTOSEL 6.18-5.15] ACPI: FPDT: validate table and record lengths

[Sasha Levin <sashal@kernel.org>]

From: Pengpeng Hou <pengpeng@iscas.ac.cn>

[ Upstream commit 458b40fcdd04a9a0334830de5a536d8425454e4b ]

FPDT records are supplied by firmware and are walked through
length fields stored in the table itself.

Check the main-table entry length before reading an entry, check
the mapped subtable length before remapping it, and check each
record length before saving a typed record pointer for sysfs.

This keeps malformed firmware records from being interpreted as
complete resume, suspend, or boot records when the current item
is shorter than the structure consumed by the driver.

Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
Link: https://patch.msgid.link/20260706094141.82438-1-pengpeng@iscas.ac.cn
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

I have enough verified history, diff, and review context to judge this
against the local 7.3-rc4 tree.

## Phase 1: Commit message forensics

**Step 1.1: Subject line**
Record: **[ACPI / FPDT] [validate] check firmware-supplied FPDT
table/record lengths before walking them or exposing typed records via
sysfs**

**Step 1.2: Tags**
Record:
- `Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>` (author)
- `Link:
  https://patch.msgid.link/20260706094141.82438-1-pengpeng@iscas.ac.cn`
- `Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>` (ACPI
  maintainer)
- No `Fixes:`, `Reported-by:`, `Tested-by:`, `Reviewed-by:`, `Acked-
  by:`, or `Cc: stable` (absence expected here)
- Ignore any pipeline `Signed-off-by: Sasha Levin` / `[Upstream commit
  ...]` markers

**Step 1.3: Body**
Record:
- **Bug:** FPDT entries/records are walked using length fields supplied
  by firmware. The driver previously trusted those lengths.
- **Symptom:** A truncated/malformed record can be treated as a full
  resume, suspend, or boot record. Sysfs then consumes a larger C struct
  than the firmware item actually occupies.
- **Root cause:** Missing bounds checks before mapping the subtable,
  before advancing the walker, and before storing typed pointers.
- **Versions:** none stated. Introduced with FPDT support, not by a
  later feature.

**Step 1.4: Hidden bug fix?**
Record: Not hidden. It is an explicit firmware bounds-check plus a
signature-mismatch unmap. That is a real OOB/leak fix, not style
cleanup.

## Phase 2: Diff analysis

**Step 2.1: Inventory**
Record:
- File: `drivers/acpi/acpi_fpdt.c` only (`+47 / -6`)
- Functions: `fpdt_process_subtable()`, `acpi_init_fpdt()`
- Scope: single-file surgical fix

**Step 2.2: Code flow**
Record (verified against parent `458b40fcdd04a^`):

1. **Signature mismatch:** mapped header was returned without unmap →
   now unmaps (fixes ioremap leak).
2. **Subtable remap:** `length` from firmware was passed to
   `acpi_os_map_memory()` with no lower bound → reject if `length <
   sizeof(*subtable_header)`.
3. **Record walk:** header was read and `offset +=
   record_header->length` before checking remaining space; only zero
   length was rejected (`a83c68a3bf7c4`) → require a full header,
   `length >= sizeof(header)`, and `length <= remaining`; zero is still
   rejected because `sizeof(*record_header)` is 4.
4. **Typed records:** `RECORD_S3_RESUME` / `SUSPEND` / `BOOT` stored
   pointers used by sysfs (`sizeof` 24 / 20 / 48) with no size check →
   reject if the firmware length is smaller than the struct.
5. **Main FPDT table:** walker did `offset += sizeof(*subtable)` with no
   leftover-bytes check → reject truncated/invalid entry lengths, then
   advance by `subtable->length` (spec length field).

**Step 2.3: Bug mechanism**
Record:
- **Category:** memory safety (OOB read) + resource leak
- **Mechanism:** firmware-controlled lengths were used as cursor and as
  justification to treat a short blob as a full
  `resume_performance_record` / `suspend_performance_record` /
  `boot_performance_record`. Sysfs `*_show` helpers then read fields
  past the real record. Truncated headers could also read past the
  mapped subtable.

**Step 2.4: Fix quality**
Record: Obviously correct, localized, no API/feature change. Regression
risk is low. The one behavioral change is walking the main table by
`subtable->length` instead of a fixed 16-byte stride. Spec pointer
records are 16 bytes (`struct fpdt_subtable_entry` /
`acpi_fpdt_boot_pointer`). Firmware with a garbage non-zero length in an
otherwise packed 16-byte table could now skip or fail FPDT init instead
of parsing later entries; that fails closed (no FPDT sysfs), not a
kernel crash. No follow-up on this file after `458b40fcdd04a`.

## Phase 3: Git history

**Step 3.1: Blame**
Record: Buggy walker comes from `d1eb86e59be09` (“ACPI: tables:
introduce support for FPDT table”, 2021-01-29), first in **v5.12-rc1**.
Zero-length hang was later fixed by `a83c68a3bf7c4` (2023), which did
**not** check truncated non-zero lengths or typed-struct size.

**Step 3.2: Fixes: tag**
Record: none. The incomplete predecessor is `a83c68a3bf7c4` (`Fixes:
d1eb86e59be0`, `Cc: stable`). That confirms the class (malformed FPDT
firmware) is real; this commit finishes the length checks.

**Step 3.3: File history**
Record (newest first):
- `458b40fcdd04a` this commit
- `8dcb4485e7672` expose FBPT/S3PT via sysfs (already in this tree;
  ancestor of the fix)
- `a83c68a3bf7c4` zero-length / invalid subtables
- `211391bf04b3c` invalid phys address
- `d1eb86e59be09` introduce driver

Standalone length-validation patch, not `N/M`.

**Step 3.4: Author**
Record: Pengpeng Hou; same period also `2c50ffdc73f3a` “ACPI: processor:
validate MADT IOAPIC entry bounds” (same firmware-table bounds pattern).
Not the ACPI maintainer; Rafael applied it.

**Step 3.5: Dependencies**
Record: No extra helpers. Needs the FPDT driver (`d1eb86e59be09`),
present here. `8dcb4485e7672` is already an ancestor (`git merge-base
--is-ancestor` succeeded). Can apply in this tree; it is already an
ancestor of HEAD.

## Phase 4: Mailing list / external

**Step 4.1:** `b4 dig -c 458b40fcdd04a` →
https://patch.msgid.link/20260706094141.82438-1-pengpeng@iscas.ac.cn
`b4 dig -a`: **v1 only**. Thread saved to mbox (2 messages).
Rafael: **“Applied as 7.3 material, thanks!”** No NAK, no requested
respin, no `Cc: stable` in review.

**Step 4.2:** `b4 dig -w` recipients: Pengpeng Hou, Rafael J. Wysocki,
Len Brown, linux-acpi, linux-kernel. Maintainer was on To/Cc and merged
it.

**Step 4.3:** No bugzilla/syzbot/Reported-by. No extra crash report
beyond the commit text. Related real hardware: `a83c68a3bf7c4` already
documented buggy FPDT BIOS tables (zero-length S3PT record, duplicated
FBPT signature).

**Step 4.4:** Not a multi-patch series.

**Step 4.5:** lore.kernel.org/stable search returned HTTP 403.
**UNVERIFIED:** whether stable@ discussed this hash.

## Phase 5: Semantic / reachability

**Step 5.1:** `fpdt_process_subtable`, `acpi_init_fpdt`.

**Step 5.2:** `acpi_init_fpdt` is `fs_initcall`; only caller of
`fpdt_process_subtable` is that init function. Boot path, not
interrupt/hot path.

**Step 5.3:** `acpi_get_table` / `acpi_os_map_memory` /
`acpi_os_unmap_memory` / `sysfs_create_group`. Records are later read by
sysfs `*_show` using `record_boot` / `record_suspend` / `record_resume`.

**Step 5.4:** Reachable when `CONFIG_ACPI_FPDT=y` and firmware publishes
FPDT. Userspace cannot craft the ACPI table on a locked-down platform;
VMs and broken BIOS can. Sysfs is the consumer of the typed pointers.

**Step 5.5:** Same pattern as the author’s MADT IOAPIC length checks.
Prior FPDT fix only handled length==0.

## Phase 6: This checkout

**Step 6.1:** `git describe HEAD` → **v7.3-rc4-27-gf0100363d8c37**.
`make kernelversion` → **7.3.0-rc4**.
`git merge-base --is-ancestor 458b40fcdd04a HEAD` → yes.
Current `drivers/acpi/acpi_fpdt.c` already contains the new checks
(lines 190–219, 223–226, 243–246, 262–265, 347–360, 374).
Buggy walker has existed in this lineage since v5.12.

**Step 6.2:** Backport difficulty **clean apply** here (already merged).
No later conflicting edits on this file.

**Step 6.3:** Related fix `a83c68a3bf7c4` is present (zero-length only).
It does not replace this patch.

## Phase 7: Subsystem

**Step 7.1:** ACPI FPDT driver under `drivers/acpi/`. **IMPORTANT**
(common x86_64/ARM64/LOONGARCH firmware table), not mm/core.
`CONFIG_ACPI_FPDT` is opt-in (`bool`, not `default y`); local `.config`
has it unset. Not staging.

**Step 7.2:** File is mature (2021–2026, few commits). This is hardening
of long-standing parse code, not a brand-new subsystem.

## Phase 8: Impact and risk

**Step 8.1:** Users who enable `CONFIG_ACPI_FPDT` on
x86_64/ARM64/LOONGARCH and whose firmware exposes FPDT. Not universal.

**Step 8.2:** Trigger is boot-time FPDT parse (`fs_initcall`) with a
malformed table. Unprivileged userspace cannot normally inject ACPI
tables; broken BIOS and untrusted VM firmware can. Real malformed FPDT
firmware is already documented.

**Step 8.3:** Failure mode:
- Read past mapped subtable → possible oops during boot
  (**HIGH/CRITICAL** if it faults)
- Short record accepted → sysfs reads adjacent firmware bytes (wrong
  data / info leak, **HIGH** memory-safety)
- Signature-mismatch ioremap leak at init (**LOW**, one-shot)

Severity: **HIGH** (firmware-controlled OOB on an enabled driver).

**Step 8.4:** Benefit high for FPDT-enabled systems with bad tables;
risk low (47 lines, fail closed, maintainer-acked, no revert).

## Phase 9: Synthesis

**For backporting**
- Firmware-supplied lengths used as walk cursor and as struct size:
  classic OOB.
- Completes a known FPDT firmware-bug class (`a83c68a3bf7c4` already
  proved bad BIOSes and was sent to stable).
- Also fixes a real `acpi_os_map_memory` leak on signature mismatch.
- Small, single-file, obviously correct; Rafael merged it.
- Driver and buggy code exist in this tree; no missing prerequisites.

**Against**
- No crash log / syzbot / `Fixes:` / explicit stable nomination.
- `CONFIG_ACPI_FPDT` is opt-in, not defconfig.
- Rafael labeled it “7.3 material,” not stable, but that is merge-window
  wording, not a NAK of the bug.
- Main-table walk-by-length could hide later entries if firmware lies
  about length; fail-closed.

**Unresolved**
- UNVERIFIED: stable@ thread (lore 403).
- UNVERIFIED: which distros set `CONFIG_ACPI_FPDT=y`.

**Stable rules**
1. Obviously correct and tested? **PASS** (v1 applied as-is by ACPI
   maintainer; in 7.3-rc4 with no follow-up revert).
2. Real user-affecting bug? **PASS** (OOB + leak on firmware tables;
   prior FPDT BIOS bugs in the wild).
3. Important (crash/security/corruption/deadlock)? **PASS** (OOB read
   from firmware lengths; possible oops or sysfs leak).
4. Small/contained? **PASS** (`+47/-6`, one file).
5. No new features/APIs? **PASS**.
6. Applies to this tree? **PASS**.

**Exception category:** none required (not a device-
ID/quirk/DT/build/doc exception). It is a straightforward memory-safety
fix.

This tree is **7.3.0-rc4**. The FPDT walker has been here since v5.12,
so the bug is in this lineage. The patch is small, correct, and prevents
firmware-controlled out-of-bounds reads (and an ioremap leak) in
`acpi_init_fpdt`. That meets stable criteria for this checkout.

### Verification
- [Phase 1] Parsed tags: author SOB, Rafael SOB, msgid Link; no
  Fixes/Reported-by/Cc stable
- [Phase 2] `git show 458b40fcdd04a`: +47/-6 in
  `drivers/acpi/acpi_fpdt.c`; parent walker lacked remaining/size
  checks; signature path leaked the map
- [Phase 2] Struct sizes: header 4, resume 24, suspend 20, boot 48,
  subtable entry 16
- [Phase 3] `git blame`: checks from `458b40fcdd04a`; original walker
  `d1eb86e59be09` (v5.12-rc1)
- [Phase 3] `git merge-base --is-ancestor 458b40fcdd04a HEAD` → 0
  (present)
- [Phase 3] `a83c68a3bf7c4` only handled zero length; `Cc: stable` on
  that older hang fix
- [Phase 3] `8dcb4485e7672` is ancestor; no later commits on this file
- [Phase 4] `b4 dig -c/-a/-w/-m`: v1 only; Rafael “Applied as 7.3
  material, thanks!”; lists linux-acpi + Rafael + Len Brown
- [Phase 4] lore.kernel.org WebFetch/curl → 403; stable@ discussion
  UNVERIFIED
- [Phase 5] `acpi_init_fpdt` is `fs_initcall`; only caller of
  `fpdt_process_subtable`
- [Phase 6] `git describe HEAD` = v7.3-rc4-27-gf0100363d8c37; current
  file already has the validation
- [Phase 7] `CONFIG_ACPI_FPDT` depends on X86_64||ARM64||LOONGARCH, not
  default y
- [Phase 8] Failure mode: firmware OOB on FPDT parse / sysfs typed reads
- UNVERIFIED: distro `CONFIG_ACPI_FPDT` defaults; stable@ mailing-list
  history

**YES**

 drivers/acpi/acpi_fpdt.c | 53 +++++++++++++++++++++++++++++++++++-----
 1 file changed, 47 insertions(+), 6 deletions(-)

diff --git a/drivers/acpi/acpi_fpdt.c b/drivers/acpi/acpi_fpdt.c
index 271092f2700a1..058d1ad47699a 100644
--- a/drivers/acpi/acpi_fpdt.c
+++ b/drivers/acpi/acpi_fpdt.c
@@ -165,7 +165,7 @@ static int fpdt_process_subtable(u64 address, u32 subtable_type)
 	struct fpdt_subtable_header *subtable_header;
 	struct fpdt_record_header *record_header;
 	char *signature = (subtable_type == SUBTABLE_FBPT ? "FBPT" : "S3PT");
-	u32 length, offset;
+	u32 length, offset, remaining;
 	int result;
 
 	if (!fpdt_address_valid(address)) {
@@ -179,10 +179,17 @@ static int fpdt_process_subtable(u64 address, u32 subtable_type)
 
 	if (strncmp((char *)&subtable_header->signature, signature, 4)) {
 		pr_info(FW_BUG "subtable signature and type mismatch!\n");
+		acpi_os_unmap_memory(subtable_header, sizeof(*subtable_header));
 		return -EINVAL;
 	}
 
 	length = subtable_header->length;
+	if (length < sizeof(*subtable_header)) {
+		pr_err(FW_BUG "Invalid FPDT subtable length %u.\n", length);
+		acpi_os_unmap_memory(subtable_header, sizeof(*subtable_header));
+		return -EINVAL;
+	}
+
 	acpi_os_unmap_memory(subtable_header, sizeof(*subtable_header));
 
 	subtable_header = acpi_os_map_memory(address, length);
@@ -191,17 +198,29 @@ static int fpdt_process_subtable(u64 address, u32 subtable_type)
 
 	offset = sizeof(*subtable_header);
 	while (offset < length) {
-		record_header = (void *)subtable_header + offset;
-		offset += record_header->length;
+		remaining = length - offset;
+		if (remaining < sizeof(*record_header)) {
+			pr_err(FW_BUG "Truncated FPDT record header.\n");
+			result = -EINVAL;
+			goto err;
+		}
 
-		if (!record_header->length) {
-			pr_err(FW_BUG "Zero-length record found in FPTD.\n");
+		record_header = (void *)subtable_header + offset;
+		if (record_header->length < sizeof(*record_header) ||
+		    record_header->length > remaining) {
+			pr_err(FW_BUG "Invalid FPDT record length %u.\n",
+			       record_header->length);
 			result = -EINVAL;
 			goto err;
 		}
+		offset += record_header->length;
 
 		switch (record_header->type) {
 		case RECORD_S3_RESUME:
+			if (record_header->length < sizeof(*record_resume)) {
+				result = -EINVAL;
+				goto err;
+			}
 			if (subtable_type != SUBTABLE_S3PT) {
 				pr_err(FW_BUG "Invalid record %d for subtable %s\n",
 				     record_header->type, signature);
@@ -218,6 +237,10 @@ static int fpdt_process_subtable(u64 address, u32 subtable_type)
 				goto err;
 			break;
 		case RECORD_S3_SUSPEND:
+			if (record_header->length < sizeof(*record_suspend)) {
+				result = -EINVAL;
+				goto err;
+			}
 			if (subtable_type != SUBTABLE_S3PT) {
 				pr_err(FW_BUG "Invalid %d for subtable %s\n",
 				     record_header->type, signature);
@@ -233,6 +256,10 @@ static int fpdt_process_subtable(u64 address, u32 subtable_type)
 				goto err;
 			break;
 		case RECORD_BOOT:
+			if (record_header->length < sizeof(*record_boot)) {
+				result = -EINVAL;
+				goto err;
+			}
 			if (subtable_type != SUBTABLE_FBPT) {
 				pr_err(FW_BUG "Invalid %d for subtable %s\n",
 				     record_header->type, signature);
@@ -289,7 +316,21 @@ static int __init acpi_init_fpdt(void)
 	}
 
 	while (offset < header->length) {
+		if (header->length - offset < sizeof(*subtable)) {
+			pr_err(FW_BUG "Truncated FPDT subtable entry.\n");
+			result = -EINVAL;
+			goto err_subtable;
+		}
+
 		subtable = (void *)header + offset;
+		if (subtable->length < sizeof(*subtable) ||
+		    subtable->length > header->length - offset) {
+			pr_err(FW_BUG "Invalid FPDT subtable entry length %u.\n",
+			       subtable->length);
+			result = -EINVAL;
+			goto err_subtable;
+		}
+
 		switch (subtable->type) {
 		case SUBTABLE_FBPT:
 		case SUBTABLE_S3PT:
@@ -302,7 +343,7 @@ static int __init acpi_init_fpdt(void)
 			/* Other types are reserved in ACPI 6.4 spec. */
 			break;
 		}
-		offset += sizeof(*subtable);
+		offset += subtable->length;
 	}
 	return 0;
 err_subtable:
-- 
2.53.0