vasilito/RedBear-OS
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

base: add diagnostic messages to all 41 bare acpi-rs AML panic sites

Browse Source 
The AML interpreter in drivers/acpi-rs/src/aml/{mod,namespace,object}.rs
contained 41 bare panic!() / _ => panic!() sites. These were all
internal-invariant assertions (firmware-table validation, AML type
checks, destructure safety) that should never fire under correct
ACPI tables — but a corrupted firmware table or interpreter bug would
crash the entire acpid daemon with no diagnostic context.

This commit adds a diagnostic String to every site explaining what
invariant was violated:

  mod.rs: 34 sites (was panic!() / _ => panic!())
    Examples of new messages:
      'acpi: _STA result was not an Integer'
      'acpi: SystemMemory region read with unsupported length {} bytes'
      'acpi: field read region was not an OpRegion'
      'acpi: Increment/Decrement argument was not Object'
      'acpi: BufferField read with non-Buffer/String backing'

  namespace.rs: 4 sites (last/traversal path segment destructures)

  object.rs: 3 sites (BufferField backing-object checks, to_buffer
    allowed_bytes range)

Zero behavior change: panics still happen on the same conditions,
just with a String explaining what went wrong. Pure-text mechanical
edit; no algorithmic risk.

Found by the Round 13 audit (local/docs/3D-DESKTOP-COMPREHENSIVE-PLAN.md §10).

3 files changed, +41/-41.
This commit is contained in:
Red Bear OS
2026-07-27 22:45:25 +09:00
parent c5b32f9f6b
commit eb579964be
3 changed files with 41 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files
drivers/acpi-rs/src/aml/mod.rs
+34 -34
View File
@@ -340,7 +340,7 @@ where
.evaluate_if_present(AmlName::from_str("_STA").unwrap().resolve(path)?, vec![])
{
Ok(Some(result)) => {
let Object::Integer(result) = *result else { panic!() };
let Object::Integer(result) = *result else { panic!("acpi: _STA result was not an Integer") };
let status = DeviceStatus(result);
status.present() && status.functioning()
}
@@ -509,7 +509,7 @@ where
self.do_unary_maths(&mut context, op)?;
}
Opcode::Increment | Opcode::Decrement => {
let [Argument::Object(operand)] = &op.arguments[..] else { panic!() };
let [Argument::Object(operand)] = &op.arguments[..] else { panic!("acpi: Increment/Decrement argument was not Object") };
let operand = operand.clone().unwrap_transparent_reference();
let token = self.object_token.lock();
@@ -979,7 +979,7 @@ where
if let Argument::Object(arg) = arg {
arg.clone()
} else {
panic!();
panic!("acpi: method-call argument was not Object");
}
})
.collect();
@@ -998,7 +998,7 @@ where
let result = f(&args)?;
context.contribute_arg(Argument::Object(result));
} else {
panic!();
panic!("acpi: called object was neither Method nor NativeMethod");
}
}
Opcode::Return => {
@@ -1097,7 +1097,7 @@ where
// resolved device. The `SuperName` target was
// captured into `notify_target` at resolution time.
let [Argument::Object(_device), Argument::Object(value)] = &op.arguments[..] else {
panic!()
panic!("acpi: Notify arguments were not Objects")
};
let value = value.as_integer()?;
if let Some(target) = context.notify_target.take() {
@@ -1196,7 +1196,7 @@ where
{
let num_elements_left = {
let Argument::Object(total_elements) = &package_op.arguments[0] else {
panic!()
panic!("acpi: VarPackage argument was not Object")
};
let total_elements =
total_elements.clone().unwrap_transparent_reference().as_integer()?
@@ -1893,7 +1893,7 @@ where
context.last_op()?.arguments.push(Argument::Object(Object::Integer(u64::MAX).wrap()));
}
Opcode::InternalMethodCall => panic!(),
Opcode::InternalMethodCall => panic!("acpi: InternalMethodCall opcode should have been resolved before execution"),
}
}
}
@@ -2013,7 +2013,7 @@ where
Opcode::Or => left | right,
Opcode::Nor => !(left | right),
Opcode::Xor => left ^ right,
_ => panic!(),
_ => panic!("acpi: unknown binary maths opcode {:#x}", op.opcode),
};
let result = Object::Integer(result).wrap();
@@ -2057,7 +2057,7 @@ where
0
}
}
_ => panic!(),
_ => panic!("acpi: unknown unary maths opcode {:#x}", op.opcode),
};
context.contribute_arg(Argument::Object(Object::Integer(result).wrap()));
@@ -2106,7 +2106,7 @@ where
(left, right)
}
Object::Buffer(ref left) => {
let Object::Buffer(ref right) = *right else { panic!() };
let Object::Buffer(ref right) = *right else { panic!("acpi: logical op right operand was not Buffer") };
let left = {
let mut bytes = [0u8; 4];
(bytes[0..left.len()]).copy_from_slice(left);
@@ -2131,7 +2131,7 @@ where
Opcode::LEqual => left == right,
Opcode::LGreater => left > right,
Opcode::LLess => left < right,
_ => panic!(),
_ => panic!("acpi: unknown logical opcode {:#x}", result.opcode),
};
let result = if result { Object::Integer(u64::MAX) } else { Object::Integer(0) };
@@ -2256,7 +2256,7 @@ where
Object::Integer(value) => match op.op {
Opcode::ToDecimalString => Object::String(value.to_string()),
Opcode::ToHexString => Object::String(alloc::format!("{value:#X}")),
_ => panic!(),
_ => panic!("acpi: opcode was not ToDecimalString or ToHexString"),
},
Object::Buffer(ref bytes) => {
if bytes.is_empty() {
@@ -2267,7 +2267,7 @@ where
let as_str = match op.op {
Opcode::ToDecimalString => alloc::format!("{byte},"),
Opcode::ToHexString => alloc::format!("{byte:#04X},"),
_ => panic!(),
_ => panic!("acpi: opcode was not ToDecimalString or ToHexString (buffer path)"),
};
string.push_str(&as_str);
}
@@ -2536,7 +2536,7 @@ where
Object::Buffer(value) => {
target_object.write_buffer_field(value.as_slice(), &token)?
}
_ => panic!(),
_ => panic!("acpi: store source object was not Integer/Buffer for BufferField target"),
}
}
@@ -2688,19 +2688,19 @@ where
let (read_region, index_field_idx) = match field.kind {
FieldUnitKind::Normal { ref region } => (region, 0),
FieldUnitKind::Bank { ref region, ref bank, bank_value } => {
let Object::FieldUnit(ref bank) = **bank else { panic!() };
let Object::FieldUnit(ref bank) = **bank else { panic!("acpi: Bank field bank-unit was not a FieldUnit") };
assert!(matches!(bank.kind, FieldUnitKind::Normal { .. }));
self.do_field_write(bank, Object::Integer(bank_value).wrap())?;
(region, 0)
}
FieldUnitKind::Index { index: _, ref data } => {
let Object::FieldUnit(ref data) = **data else { panic!() };
let FieldUnitKind::Normal { region } = &data.kind else { panic!() };
let Object::FieldUnit(ref data) = **data else { panic!("acpi: Index field data was not a FieldUnit") };
let FieldUnitKind::Normal { region } = &data.kind else { panic!("acpi: Index field data-kind was not Normal") };
let reg_idx = field.bit_index / 8;
(region, reg_idx)
}
};
let Object::OpRegion(ref read_region) = **read_region else { panic!() };
let Object::OpRegion(ref read_region) = **read_region else { panic!("acpi: field read region was not an OpRegion") };
/*
* TODO: it might be worth having a fast path here for reads that don't do weird
@@ -2725,8 +2725,8 @@ where
}
FieldUnitKind::Index { ref index, ref data } => {
// Update index register
let Object::FieldUnit(ref index) = **index else { panic!() };
let Object::FieldUnit(ref data) = **data else { panic!() };
let Object::FieldUnit(ref index) = **index else { panic!("acpi: Index field index was not a FieldUnit") };
let Object::FieldUnit(ref data) = **data else { panic!("acpi: Index field data was not a FieldUnit (Index)") };
self.do_field_write(
index,
Object::Integer((index_field_idx + i * (access_width_bits / 8)) as u64).wrap(),
@@ -2774,19 +2774,19 @@ where
let (write_region, index_field_idx) = match field.kind {
FieldUnitKind::Normal { ref region } => (region, 0),
FieldUnitKind::Bank { ref region, ref bank, bank_value } => {
let Object::FieldUnit(ref bank) = **bank else { panic!() };
let Object::FieldUnit(ref bank) = **bank else { panic!("acpi: Bank field bank-unit was not a FieldUnit (write)") };
assert!(matches!(bank.kind, FieldUnitKind::Normal { .. }));
self.do_field_write(bank, Object::Integer(bank_value).wrap())?;
(region, 0)
}
FieldUnitKind::Index { index: _, ref data } => {
let Object::FieldUnit(ref data) = **data else { panic!() };
let FieldUnitKind::Normal { region: data_region } = &data.kind else { panic!() };
let Object::FieldUnit(ref data) = **data else { panic!("acpi: Index field data was not a FieldUnit (write)") };
let FieldUnitKind::Normal { region: data_region } = &data.kind else { panic!("acpi: Index field data-kind was not Normal (write)") };
let reg_idx = field.bit_index / 8;
(data_region, reg_idx)
}
};
let Object::OpRegion(ref write_region) = **write_region else { panic!() };
let Object::OpRegion(ref write_region) = **write_region else { panic!("acpi: field write region was not an OpRegion") };
// TODO: if the region wants locking, do that
@@ -2806,8 +2806,8 @@ where
}
FieldUnitKind::Index { ref index, ref data } => {
// Update index register
let Object::FieldUnit(ref index) = **index else { panic!() };
let Object::FieldUnit(ref data) = **data else { panic!() };
let Object::FieldUnit(ref index) = **index else { panic!("acpi: Index field index was not a FieldUnit (write Index)") };
let Object::FieldUnit(ref data) = **data else { panic!("acpi: Index field data was not a FieldUnit (write Index)") };
self.do_field_write(
index,
Object::Integer((index_field_idx + i * (access_width_bits / 8)) as u64).wrap(),
@@ -2871,7 +2871,7 @@ where
2 => self.handler.read_u16(address) as u64,
4 => self.handler.read_u32(address) as u64,
8 => self.handler.read_u64(address),
_ => panic!(),
_ => panic!("acpi: SystemMemory region read with unsupported length {} bytes (only 1/2/4/8 supported)", length),
}
}),
RegionSpace::SystemIO => Ok({
@@ -2880,7 +2880,7 @@ where
1 => self.handler.read_io_u8(address) as u64,
2 => self.handler.read_io_u16(address) as u64,
4 => self.handler.read_io_u32(address) as u64,
_ => panic!(),
_ => panic!("acpi: SystemIO region read with unsupported length {} bytes", length),
}
}),
RegionSpace::PciConfig => {
@@ -2890,7 +2890,7 @@ where
1 => Ok(self.handler.read_pci_u8(address, offset) as u64),
2 => Ok(self.handler.read_pci_u16(address, offset) as u64),
4 => Ok(self.handler.read_pci_u32(address, offset) as u64),
_ => panic!(),
_ => panic!("acpi: PciConfig region read with unsupported length {} bytes", length),
}
}
@@ -2909,7 +2909,7 @@ where
2 => handler.read_u16(region, offset).map(Into::into),
4 => handler.read_u32(region, offset).map(Into::into),
8 => handler.read_u64(region, offset),
_ => panic!(),
_ => panic!("acpi: custom region read with unsupported length {} bytes", length),
}
} else {
Err(AmlError::NoHandlerForRegionAccess(region.space))
@@ -2941,7 +2941,7 @@ where
2 => self.handler.write_u16(address, value as u16),
4 => self.handler.write_u32(address, value as u32),
8 => self.handler.write_u64(address, value),
_ => panic!(),
_ => panic!("acpi: SystemMemory region write with unsupported length {} bytes", length),
}
Ok(())
}
@@ -2951,7 +2951,7 @@ where
1 => self.handler.write_io_u8(address, value as u8),
2 => self.handler.write_io_u16(address, value as u16),
4 => self.handler.write_io_u32(address, value as u32),
_ => panic!(),
_ => panic!("acpi: SystemIO region write with unsupported length {} bytes", length),
}
Ok(())
}
@@ -2962,7 +2962,7 @@ where
1 => self.handler.write_pci_u8(address, offset, value as u8),
2 => self.handler.write_pci_u16(address, offset, value as u16),
4 => self.handler.write_pci_u32(address, offset, value as u32),
_ => panic!(),
_ => panic!("acpi: PciConfig region write with unsupported length {} bytes", length),
}
Ok(())
}
@@ -2982,7 +2982,7 @@ where
2 => handler.write_u16(region, offset, value as u16),
4 => handler.write_u32(region, offset, value as u32),
8 => handler.write_u64(region, offset, value),
_ => panic!(),
_ => panic!("acpi: custom region write with unsupported length {} bytes", length),
}
} else {
Err(AmlError::NoHandlerForRegionAccess(region.space))
drivers/acpi-rs/src/aml/namespace.rs
+4 -4
View File
@@ -285,7 +285,7 @@ impl Namespace {
let (last_seg, levels) = path.0[1..].split_last().unwrap();
let NameComponent::Segment(last_seg) = last_seg else {
panic!();
panic!("acpi: last path segment was not Segment");
};
// TODO: this helps with diagnostics, but requires a heap allocation just in case we need to error.
@@ -296,7 +296,7 @@ impl Namespace {
traversed_path.0.push(*level);
let NameComponent::Segment(segment) = level else {
panic!();
panic!("acpi: traversal level was not Segment");
};
current_level =
current_level.children.get(segment).ok_or(AmlError::LevelDoesNotExist(traversed_path.clone()))?;
@@ -312,7 +312,7 @@ impl Namespace {
let (last_seg, levels) = path.0[1..].split_last().unwrap();
let NameComponent::Segment(last_seg) = last_seg else {
panic!();
panic!("acpi: last path segment was not Segment (mut)");
};
// TODO: this helps with diagnostics, but requires a heap allocation just in case we need to error. We can
@@ -325,7 +325,7 @@ impl Namespace {
traversed_path.0.push(*level);
let NameComponent::Segment(segment) = level else {
panic!();
panic!("acpi: traversal level was not Segment (mut)");
};
current_level = current_level
.children
drivers/acpi-rs/src/aml/object.rs
+3 -3
View File
@@ -224,7 +224,7 @@ impl Object {
Object::Integer(value) => match allowed_bytes {
4 => Ok((*value as u32).to_le_bytes().to_vec()),
8 => Ok(value.to_le_bytes().to_vec()),
_ => panic!(),
_ => panic!("acpi: to_buffer unsupported allowed_bytes={} (only 1/2/4/8 supported)", allowed_bytes),
},
Object::String(value) => Ok(value.as_bytes().to_vec()),
_ => Err(AmlError::InvalidOperationOnObject { op: Operation::ConvertToBuffer, typ: self.typ() }),
@@ -236,7 +236,7 @@ impl Object {
let buffer = match **buffer {
Object::Buffer(ref buffer) => buffer.as_slice(),
Object::String(ref string) => string.as_bytes(),
_ => panic!(),
_ => panic!("acpi: BufferField read with non-Buffer/String backing"),
};
if *length <= integer_size {
let mut dst = [0u8; 8];
@@ -260,7 +260,7 @@ impl Object {
// XXX: this unfortunately requires us to trust AML to keep the string as valid
// UTF8... maybe there is a better way?
Object::String(string) => unsafe { string.as_bytes_mut() },
_ => panic!(),
_ => panic!("acpi: BufferField write with non-Buffer/String backing"),
};
copy_bits(value, 0, buffer, *offset, *length);
Ok(())