acpid: fix AML mutex acquire — recursion, ownership, and timeout units
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.
This commit is contained in:
@@ -146,9 +146,26 @@ pub struct AmlPhysMemHandler {
|
|||||||
|
|
||||||
struct AmlMutexState {
|
struct AmlMutexState {
|
||||||
next_id: u32,
|
next_id: u32,
|
||||||
held: FxHashSet<u32>,
|
/// 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<u32, (std::thread::ThreadId, u32)>,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// 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.
|
/// Read from a physical address.
|
||||||
/// Generic parameter must be u8, u16, u32 or u64.
|
/// Generic parameter must be u8, u16, u32 or u64.
|
||||||
impl AmlPhysMemHandler {
|
impl AmlPhysMemHandler {
|
||||||
@@ -164,7 +181,7 @@ impl AmlPhysMemHandler {
|
|||||||
pci_fd: Arc::new(pci_fd),
|
pci_fd: Arc::new(pci_fd),
|
||||||
mutex_state: Arc::new(Mutex::new(AmlMutexState {
|
mutex_state: Arc::new(Mutex::new(AmlMutexState {
|
||||||
next_id: 1,
|
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> {
|
fn acquire(&self, mutex: Handle, timeout: u16) -> Result<(), AmlError> {
|
||||||
let deadline = std::time::Instant::now()
|
// ACPI `_ACQ` timeouts are expressed in MILLISECONDS, and 0xFFFF is the
|
||||||
+ std::time::Duration::from_millis(u64::from(timeout).saturating_mul(1000));
|
// 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 {
|
loop {
|
||||||
{
|
{
|
||||||
let mut state = self.mutex_state.lock().unwrap();
|
let mut state = self.mutex_state.lock().unwrap();
|
||||||
if !state.held.contains(&mutex.0) {
|
match state.held.get_mut(&mutex.0) {
|
||||||
state.held.insert(mutex.0);
|
None => {
|
||||||
return Ok(());
|
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 {
|
if std::time::Instant::now() >= deadline {
|
||||||
@@ -450,6 +488,12 @@ impl acpi::Handler for AmlPhysMemHandler {
|
|||||||
}
|
}
|
||||||
|
|
||||||
fn release(&self, mutex: Handle) {
|
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);
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user