From d78fd44a07d407274c9327245312492e93949150 Mon Sep 17 00:00:00 2001 From: Red Bear OS Date: Tue, 21 Jul 2026 04:47:00 +0900 Subject: [PATCH] =?UTF-8?q?acpid:=20fix=20AML=20mutex=20acquire=20?= =?UTF-8?q?=E2=80=94=20recursion,=20ownership,=20and=20timeout=20units?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The AML mutex `acquire` handler had two defects that let a routine `_ACQ` on ACPI processor P-state objects (`processor/CPUn/pss`, opened by cpufreqd) deadlock acpid permanently: 1. Timeout units. ACPI `_ACQ` timeouts are milliseconds and 0xFFFF is the spec's "wait forever" (ACPICA ACPI_WAIT_FOREVER, actypes.h:459). The code multiplied the value by 1000, treating it as seconds, so 0xFFFF became ~18 hours. 2. No ownership. ACPI mutexes are recursive: the owning thread may re-acquire, each Acquire paired with a Release (ACPICA acpi_ex_acquire_mutex_object, exmutex.c:140). The old code tracked only a "held" set with no owner, so a nested acquire by the single AML thread waited for a release only it could perform — a self-deadlock. Because acpid is single-threaded and the same thread serves the `acpi` scheme socket, a stuck acquire stops acpid answering scheme requests. The init namespace manager then blocks in the openat it was proxying, and — since every restricted-namespace path resolution goes through that single manager — the whole system's open path wedges (observed: mini never reaches a usable brush login). Fix: track (owner ThreadId, recursion depth) per mutex, treat a nested acquire by the owner as a depth bump, interpret the timeout as milliseconds, and bound any wait to MAX_MUTEX_WAIT (5s) so a misbehaving AML method can never freeze the scheme-serving thread. --- drivers/acpid/src/aml_physmem.rs | 60 +++++++++++++++++++++++++++----- 1 file changed, 52 insertions(+), 8 deletions(-) diff --git a/drivers/acpid/src/aml_physmem.rs b/drivers/acpid/src/aml_physmem.rs index 1f454032ed..e15ff3d739 100644 --- a/drivers/acpid/src/aml_physmem.rs +++ b/drivers/acpid/src/aml_physmem.rs @@ -146,9 +146,26 @@ pub struct AmlPhysMemHandler { struct AmlMutexState { next_id: u32, - held: FxHashSet, + /// Mutex id -> (owning thread, recursion depth). + /// + /// ACPI mutexes are recursive: the owning thread may acquire the same mutex + /// again (nested `Acquire`), and each `Acquire` must be matched by a + /// `Release`. Tracking ownership is therefore mandatory -- a plain "is it + /// held" set makes a nested acquire wait for a release that only the + /// waiting thread itself could perform, which self-deadlocks the AML + /// interpreter. + held: FxHashMap, } +/// Upper bound on how long an AML mutex acquire may wait. +/// +/// acpid is single-threaded: the same thread that evaluates AML also serves the +/// `acpi` scheme socket. A long wait here does not just delay one AML method -- +/// it stops acpid answering scheme requests, which blocks the namespace manager +/// that is waiting on that open, which in turn blocks *every* path resolution in +/// the system. Bounding the wait keeps a misbehaving AML method local. +const MAX_MUTEX_WAIT: std::time::Duration = std::time::Duration::from_secs(5); + /// Read from a physical address. /// Generic parameter must be u8, u16, u32 or u64. impl AmlPhysMemHandler { @@ -164,7 +181,7 @@ impl AmlPhysMemHandler { pci_fd: Arc::new(pci_fd), mutex_state: Arc::new(Mutex::new(AmlMutexState { next_id: 1, - held: FxHashSet::default(), + held: FxHashMap::default(), })), } } @@ -432,14 +449,35 @@ impl acpi::Handler for AmlPhysMemHandler { } fn acquire(&self, mutex: Handle, timeout: u16) -> Result<(), AmlError> { - let deadline = std::time::Instant::now() - + std::time::Duration::from_millis(u64::from(timeout).saturating_mul(1000)); + // ACPI `_ACQ` timeouts are expressed in MILLISECONDS, and 0xFFFF is the + // spec's "wait forever" value (Linux: ACPI_WAIT_FOREVER, + // include/acpi/actypes.h:459). The previous code multiplied the value by + // 1000, treating it as seconds, which turned a routine wait into hours. + const ACPI_WAIT_FOREVER: u16 = 0xFFFF; + let wait = if timeout == ACPI_WAIT_FOREVER { + MAX_MUTEX_WAIT + } else { + std::time::Duration::from_millis(u64::from(timeout)).min(MAX_MUTEX_WAIT) + }; + let deadline = std::time::Instant::now() + wait; + let me = std::thread::current().id(); + loop { { let mut state = self.mutex_state.lock().unwrap(); - if !state.held.contains(&mutex.0) { - state.held.insert(mutex.0); - return Ok(()); + match state.held.get_mut(&mutex.0) { + None => { + state.held.insert(mutex.0, (me, 1)); + return Ok(()); + } + // Nested acquire by the owning thread: bump the depth rather + // than waiting for ourselves to release. Mirrors ACPICA + // acpi_ex_acquire_mutex_object (exmutex.c:140). + Some((owner, depth)) if *owner == me => { + *depth += 1; + return Ok(()); + } + Some(_) => {} } } if std::time::Instant::now() >= deadline { @@ -450,6 +488,12 @@ impl acpi::Handler for AmlPhysMemHandler { } fn release(&self, mutex: Handle) { - self.mutex_state.lock().unwrap().held.remove(&mutex.0); + let mut state = self.mutex_state.lock().unwrap(); + if let Some((_, depth)) = state.held.get_mut(&mutex.0) { + *depth -= 1; + if *depth == 0 { + state.held.remove(&mutex.0); + } + } } }