Linux ACPI
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: ikaros <void0red@gmail.com>,
	"Rafael J. Wysocki" <rafael.j.wysocki@intel.com>,
	Sasha Levin <sashal@kernel.org>,
	rafael@kernel.org, maciej.wieczor-retman@intel.com,
	pawel.chmielewski@intel.com, linux-acpi@vger.kernel.org,
	acpica-devel@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: [PATCH AUTOSEL 6.18-5.10] ACPICA: Fix condition check in acpi_ps_parse_loop()
Date: Mon, 31 Aug 2026 09:23:38 -0400	[thread overview]
Message-ID: <20260831133314.4125787-190-sashal@kernel.org> (raw)
In-Reply-To: <20260831133314.4125787-1-sashal@kernel.org>

From: ikaros <void0red@gmail.com>

[ Upstream commit 8de27e2d83c0d07ae9443c6304575b0609394bfd ]

Fix condition check for AML_ELSE_OP in acpi_ps_parse_loop() to prevent
out-of-bounds access.

Link: https://github.com/acpica/acpica/commit/3b537b92336e
Signed-off-by: ikaros <void0red@gmail.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Link: https://patch.msgid.link/1959692.tdWV9SEqCh@rafael.j.wysocki
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

# Stable Backport Analysis: ACPICA Fix condition check in
acpi_ps_parse_loop()

**Local tree:** `stable/linux-6.18.y` at `v6.18.44` (Makefile: 6.18.44)

---

## PHASE 1: COMMIT MESSAGE FORENSICS

### Step 1.1: Subject Line
**Record:** `[ACPICA]` `[Fix]` — correct the `AML_ELSE_OP` condition
check in `acpi_ps_parse_loop()` to prevent out-of-bounds access.

### Step 1.2: Commit Message Tags
**Record:**
- **Link:** https://github.com/acpica/acpica/commit/3b537b92336e
  (upstream ACPICA commit)
- **Signed-off-by:** ikaros <void0red@gmail.com> (author)
- **Signed-off-by:** Rafael J. Wysocki <rafael.j.wysocki@intel.com>
  (ACPI maintainer)
- **Link:** https://patch.msgid.link/1959692.tdWV9SEqCh@rafael.j.wysocki
  (kernel submission; could not fetch — Anubis bot protection)
- No `Fixes:`, `Reported-by:`, `Cc: stable@vger.kernel.org`, `Tested-
  by:`, or `Reviewed-by:` tags
- Notable: Rafael Wysocki sign-off indicates ACPI maintainer acceptance
  for kernel integration

### Step 1.3: Commit Body Analysis
**Record:**
- **Bug:** After skipping a failed If/While block, the code checks
  `*walk_state->aml == AML_ELSE_OP` without verifying `walk_state->aml`
  is within the AML buffer.
- **Symptom:** Out-of-bounds read (1 byte past buffer end).
- **Root cause:** `acpi_ps_get_next_package_end()` can advance the AML
  pointer to or past `parser_state->aml_end` on malformed/truncated AML;
  the subsequent dereference is unchecked.
- **Version info:** None in commit message; upstream ACPICA issue #1078
  documents ASan reproduction.

### Step 1.4: Hidden Bug Fix Detection
**Record:** Not disguised — explicitly labeled a fix for an out-of-
bounds access. Genuine memory-safety bug fix.

---

## PHASE 2: DIFF ANALYSIS

### Step 2.1: Change Inventory
**Record:**
- **File:** `drivers/acpi/acpica/psloop.c` (+3 / -1 lines)
- **Function:** `acpi_ps_parse_loop()`
- **Scope:** Single-file, surgical fix in an error-recovery path

### Step 2.2: Code Flow Change
**Record:**
- **Before:** After skipping a failed If/While body, unconditionally
  dereferenced `*walk_state->aml` to test for `AML_ELSE_OP`.
- **After:** Only dereferences if `walk_state->aml <
  parser_state->aml_end` AND the byte equals `AML_ELSE_OP`.
- **Path affected:** Error recovery when `acpi_ps_get_arguments()` fails
  inside an If/While control structure during module-level ACPI table
  parsing.

### Step 2.3: Bug Mechanism
**Record:**
- **Category:** Buffer overflow / out-of-bounds read (memory safety)
- **Mechanism:** `acpi_ps_get_next_package_end()` returns a pointer past
  the package end. On malformed AML at the buffer boundary,
  `walk_state->aml` can equal or exceed `parser_state->aml_end`. The old
  code read one byte past the allocated AML buffer. The fix adds the
  same bounds guard used by the main parse loop at line 300.

### Step 2.4: Fix Quality
**Record:**
- **Quality:** Obviously correct; mirrors the existing
  `parser_state->aml < parser_state->aml_end` pattern at line 300.
- **Regression risk:** Very low — only skips the Else-block skip when
  already past the buffer end (correct behavior).
- **Red flags:** None.

---

## PHASE 3: GIT HISTORY INVESTIGATION

### Step 3.1: Blame
**Record:** Buggy line introduced in `5088814a6e931` ("ACPICA: AML
parser: attempt to continue loading table after error") by Erik Kaneda,
2018-06-01. Confirmed ancestor of HEAD. Present in `v6.18.44`.

### Step 3.2: Fixes: Tag
**Record:** N/A — no `Fixes:` tag. Upstream ACPICA issue #1078
references the bug; the introducing commit is `5088814a6e931` (2018).

### Step 3.3: Related File History
**Record:** Recent `psloop.c` history is copyright updates and unrelated
parser cleanups. No prior fix for this issue in this tree. The Else-skip
logic has been unchanged since 2018.

### Step 3.4: Author Context
**Record:** Author ikaros (void0red) reported the bug via ACPICA
fuzzing. Rafael Wysocki (ACPI maintainer) signed off. ACPICA maintainer
SaketADumbre merged upstream PR #1087 with positive review ("minimal but
the right changes").

### Step 3.5: Dependencies
**Record:** No dependencies. Standalone 3-line fix. No patch series.
Applies cleanly to current `psloop.c` in this tree.

---

## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH

### Step 4.1: Original Discussion
**Record:** `b4 dig -c 3b537b92336e` failed — commit not in Linux git
history (ACPICA-only commit). Upstream discussion found at:
- ACPICA issue #1078: ASan heap-buffer-overflow at `psloop.c:569`
  (fuzzed AML via `acpiexec`)
- ACPICA PR #1087: merged 2026-02-21
- Kernel lore/patch.msgid.link blocked by Anubis — could not read thread

### Step 4.2: Reviewers
**Record:** Rafael Wysocki signed off (kernel ACPI maintainer).
SaketADumbre (ACPICA maintainer) reviewed and merged upstream. No NAKs
found.

### Step 4.3: Bug Report
**Record:** ACPICA issue #1078 — ASan READ of size 1 at address 0 bytes
past a 1293-byte heap region. Reproducible with fuzzed AML
(`fuzz_178.aml`). Severity: confirmed memory safety bug via sanitizer.

### Step 4.4: Related Patches
**Record:** Standalone fix. Not part of a multi-patch series.

### Step 4.5: Stable Mailing List
**Record:** Could not search lore (Anubis protection). No stable-
specific discussion found via other sources.

---

## PHASE 5: CODE SEMANTIC ANALYSIS

### Step 5.1: Key Functions
**Record:** `acpi_ps_parse_loop()` — modified.
`acpi_ps_get_next_package_end()` — called just before the buggy check.

### Step 5.2: Callers
**Record:** `acpi_ps_parse_loop()` called from `acpi_ps_parse_aml()` in
`psparse.c:475`. Reachable during ACPI table loading and method
execution.

### Step 5.3: Callees
**Record:** `acpi_ps_get_arguments()`, `acpi_ps_complete_op()`,
`acpi_ps_get_next_package_end()`, `acpi_ut_pop_generic_state()`.

### Step 5.4: Call Chain (Reachability)
**Record:**
```
Boot: acpi_ns_load_table() → acpi_ns_parse_table() →
acpi_ns_execute_table()
  → acpi_ps_execute_table() [sets ACPI_METHOD_MODULE_LEVEL]
  → acpi_ps_parse_aml() → acpi_ps_parse_loop()
```
Module-level ACPI table parsing (DSDT/SSDT) uses this error-recovery
path. Malformed firmware AML that fails If/While argument parsing can
reach the buggy dereference. **Reachable during boot on all ACPI-enabled
systems.**

### Step 5.5: Similar Patterns
**Record:** Main parse loop at line 300 uses `parser_state->aml <
parser_state->aml_end`. The Else check at line 428 was the only
unguarded dereference in this error path. No similar fix already present
in this tree (`git log -S 'walk_state->aml <'` returned nothing).

---

## PHASE 6: CROSS-REFERENCING AGAINST LOCAL TREE

### Step 6.1: Buggy Code Exists?
**Record:** **YES.** Line 428 in `drivers/acpi/acpica/psloop.c` has the
unguarded `if (*walk_state->aml == AML_ELSE_OP)`. Confirmed in
`v6.18.44` tag. Bug present since 2018 (commit `5088814a6e931`).

### Step 6.2: Backport Complications
**Record:** **Clean apply expected.** File structure unchanged around
the hunk. No conflicting recent changes in this area.

### Step 6.3: Related Fixes Already Present?
**Record:** **No.** Fix not in this tree. `grep` for the bounds-check
pattern returns no matches.

---

## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT

### Step 7.1: Subsystem Criticality
**Record:** **ACPI / ACPICA** — **CORE**. ACPI table parsing runs at
boot on essentially all x86 and many ARM systems. Affects firmware table
loading.

### Step 7.2: Subsystem Activity
**Record:** Active — regular ACPICA syncs and copyright updates, but
this code path has been stable since 2018.

---

## PHASE 8: IMPACT AND RISK ASSESSMENT

### Step 8.1: Who Is Affected
**Record:** All systems with ACPI enabled that load AML tables
containing If/While constructs. Trigger requires malformed ACPI AML
(common in buggy firmware) combined with a parse failure in the If/While
predicate.

### Step 8.2: Trigger Conditions
**Record:**
- If/While argument parsing fails during module-level table load
- `acpi_ps_get_next_package_end()` advances AML pointer to or past
  buffer end
- Unprivileged users cannot directly inject ACPI tables, but **malicious
  or buggy firmware ACPI tables** can trigger this at boot
- Likelihood: Low in practice, but the error-recovery path exists
  specifically for malformed AML

### Step 8.3: Failure Mode Severity
**Record:** Out-of-bounds read of 1 byte past AML buffer. **Severity:
HIGH** — potential kernel oops/crash or information leak. ASan-confirmed
heap-buffer-overflow.

### Step 8.4: Risk-Benefit
**Record:**
- **Benefit:** HIGH — prevents OOB read in core ACPI parser on malformed
  firmware tables
- **Risk:** VERY LOW — 3-line bounds check matching existing code
  pattern
- **Ratio:** Strongly favors backport

---

## PHASE 9: FINAL SYNTHESIS

### Step 9.1: Evidence Summary

**FOR backport:**
- Real, ASan-confirmed out-of-bounds read
- Small (3 lines), obviously correct fix
- Bug present in this tree since 2018
- Affects core ACPI table loading at boot
- ACPI maintainer (Rafael Wysocki) signed off
- No dependencies; clean apply
- Error-recovery path designed for malformed AML — exactly when this bug
  triggers

**AGAINST backport:**
- Found via acpiexec fuzzing, not a reported production kernel crash
- Requires malformed ACPI + specific parse failure (narrow trigger)
- ACPI tables come from firmware, not direct userspace input (reduced
  attack surface, but buggy firmware is common)

**UNRESOLVED:**
- Kernel lore thread content (Anubis blocked)
- Whether any production kernel crash has been attributed to this
  specific bug

### Step 9.2: Stable Rules Checklist
1. Obviously correct and tested? **PASS** — matches existing bounds-
   check pattern; ASan-verified fix upstream
2. Fixes a real bug? **PASS** — confirmed OOB read
3. Important issue? **PASS** — memory safety in core boot path (HIGH)
4. Small and contained? **PASS** — 3 lines, one function
5. No new features/APIs? **PASS**
6. Can apply to local tree? **PASS** — buggy code confirmed present,
   clean apply expected

### Step 9.3: Exception Categories
**Record:** N/A — standard bug fix, not a device ID/quirk/DT/build/doc
exception.

### Step 9.4: Decision Rationale

This commit fixes a genuine out-of-bounds read in the ACPI AML parser's
error-recovery path. The bug has existed since 2018 and is present in
the local `6.18.44` tree. While the trigger requires malformed ACPI AML
with a failed If/While parse, that is precisely the scenario the 2018
error-recovery code was written to handle — and buggy ACPI firmware is a
known real-world problem. The fix is minimal, mirrors an existing bounds
check in the same function, and carries negligible regression risk.
Memory-safety fixes in core boot-path code are appropriate for stable
trees.

---

## Verification

- **[Phase 1]** Parsed commit message: subsystem ACPICA, action "Fix",
  OOB access described; Rafael Wysocki SOB confirmed
- **[Phase 1]** No Reported-by/syzbot; upstream ACPICA issue #1078
  provides bug report
- **[Phase 2]** Diff: +3/-1 in `psloop.c`, adds `walk_state->aml <
  parser_state->aml_end` guard before `AML_ELSE_OP` check
- **[Phase 3]** `git blame -L 428,428`: buggy line from `5088814a6e931`
  (2018-06-01)
- **[Phase 3]** `git show 5088814a6e931`: introduced Else-skip logic
  without bounds check
- **[Phase 3]** `git merge-base --is-ancestor 5088814a6e931 HEAD`:
  confirmed buggy commit in tree
- **[Phase 3]** `git log --oneline -20 -- drivers/acpi/acpica/psloop.c`:
  no prior fix for this issue
- **[Phase 3]** `git log -S 'walk_state->aml <' --
  drivers/acpi/acpica/`: no similar fix already applied
- **[Phase 4]** `b4 dig -c 3b537b92336e`: FAILED — commit not in Linux
  git
- **[Phase 4]** ACPICA issue #1078: ASan heap-buffer-overflow confirmed
  at psloop.c
- **[Phase 4]** ACPICA PR #1087: merged, maintainer approved minimal fix
- **[Phase 4]** lore.kernel.org / patch.msgid.link: BLOCKED by Anubis —
  UNVERIFIED for kernel list discussion
- **[Phase 5]** `acpi_ps_parse_loop` callers: `psparse.c:475` via
  `acpi_ps_parse_aml`
- **[Phase 5]** Call chain: `acpi_ns_parse_table` →
  `acpi_ns_execute_table` → `acpi_ps_execute_table` (sets
  `ACPI_METHOD_MODULE_LEVEL`) → `acpi_ps_parse_loop`
- **[Phase 5]** `nsparse.c:98`: `ACPI_METHOD_MODULE_LEVEL` set during
  table execution
- **[Phase 6]** `git describe HEAD`: v6.18.44 on `stable/linux-6.18.y`
- **[Phase 6]** `git show v6.18.44:drivers/acpi/acpica/psloop.c` lines
  426-432: buggy unguarded check confirmed
- **[Phase 6]** `grep` for fix pattern in tree: no matches — fix not
  present
- **[Phase 8]** Failure mode: OOB read, severity HIGH; trigger on
  malformed ACPI during boot table load

**YES**

 drivers/acpi/acpica/psloop.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/acpica/psloop.c b/drivers/acpi/acpica/psloop.c
index c989cadf271ce..35111ff2526b1 100644
--- a/drivers/acpi/acpica/psloop.c
+++ b/drivers/acpi/acpica/psloop.c
@@ -425,7 +425,10 @@ acpi_status acpi_ps_parse_loop(struct acpi_walk_state *walk_state)
 
 					ACPI_ERROR((AE_INFO,
 						    "Skipping While/If block"));
-					if (*walk_state->aml == AML_ELSE_OP) {
+					if ((walk_state->aml <
+					     parser_state->aml_end)
+					    && (*walk_state->aml ==
+						AML_ELSE_OP)) {
 						ACPI_ERROR((AE_INFO,
 							    "Skipping Else block"));
 						walk_state->parser_state.aml =
-- 
2.53.0


  parent reply	other threads:[~2026-08-31 13:39 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20260831133314.4125787-1-sashal@kernel.org>
2026-08-31 13:20 ` [PATCH AUTOSEL 6.18-6.12] spi: dw-mmio: Add ACPI ID LECA0002 for LECARC SoCs Sasha Levin
2026-08-31 13:20 ` [PATCH AUTOSEL 6.18-5.10] ACPICA: Prevent adding invalid references Sasha Levin
2026-08-31 13:21 ` [PATCH AUTOSEL 6.18-5.10] ACPICA: Improve argument parsing in acpi_ps_get_next_simple_arg() Sasha Levin
2026-08-31 13:21 ` [PATCH AUTOSEL 6.18-5.10] ACPICA: Fix NULL pointer dereference in acpi_ns_custom_package() Sasha Levin
2026-08-31 13:23 ` Sasha Levin [this message]
2026-08-31 13:24 ` [PATCH AUTOSEL 6.18-5.10] ACPICA: validate handler object type in two places Sasha Levin
2026-08-31 13:24 ` [PATCH AUTOSEL 6.18-5.10] ACPICA: add boundary checks " Sasha Levin
2026-08-31 13:24 ` [PATCH AUTOSEL 6.18] gpiolib: acpi: Add robust bounds-checking for GPIO pin resources Sasha Levin
2026-08-31 13:25 ` [PATCH AUTOSEL 6.18-5.10] ACPICA: Fix integer overflow in acpi_ex_opcode_3A_1T_1R() (mid_op) Sasha Levin
2026-08-31 13:26 ` [PATCH AUTOSEL 6.18-5.10] ACPICA: Add package limit checks in parser functions Sasha Levin
2026-08-31 13:26 ` [PATCH AUTOSEL 6.18-5.10] ACPICA: Fix use-after-free in acpi_ds_terminate_control_method() Sasha Levin
2026-08-31 13:27 ` [PATCH AUTOSEL 6.18-5.10] ACPICA: validate byte_count in acpi_ps_get_next_package_length() Sasha Levin
2026-08-31 13:28 ` [PATCH AUTOSEL 6.18-5.15] ACPI: PCI: Clear _DEP dependencies after PCI root bridge attach Sasha Levin
2026-08-31 13:28 ` [PATCH AUTOSEL 6.18-6.1] ACPICA: Enhance OEM ID and Table ID validation in acpi_ex_load_table_op() Sasha Levin
2026-08-31 13:28 ` [PATCH AUTOSEL 6.18-5.10] ACPICA: Add validation for node in acpi_ns_build_normalized_path() Sasha Levin
2026-08-31 13:29 ` [PATCH AUTOSEL 6.18-5.10] ACPICA: add boundary checks in acpi_ps_get_next_field() Sasha Levin
2026-08-31 13:29 ` [PATCH AUTOSEL 6.18-6.12] ACPI: scan: Honor _DEP for ACPI0016 PCI/CXL host bridge Sasha Levin
2026-08-31 13:31 ` [PATCH AUTOSEL 6.18-5.10] ACPICA: Enhance buffer validation in acpi_ut_walk_aml_resources() Sasha Levin

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=20260831133314.4125787-190-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=acpica-devel@lists.linux.dev \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maciej.wieczor-retman@intel.com \
    --cc=patches@lists.linux.dev \
    --cc=pawel.chmielewski@intel.com \
    --cc=rafael.j.wysocki@intel.com \
    --cc=rafael@kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=void0red@gmail.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox