Quellcode durchsuchen

Remove dependency on typenum and make safer

Replaces the dependency on typenum with const generics. Always nice!

Secondly, changes the representation of "potentially-there" fields to be
safer. The old version clearly didn't miscompile, but I don't think it was
very safe. This explicitly treats the extended fields as uninitialized
memory until a sufficient version is provided, which I think is sound?
Isaac Woods vor 5 Jahren
Ursprung
Commit
d197f990c1
4 geänderte Dateien mit 39 neuen und 40 gelöschten Zeilen
  1. 0 1
      acpi/Cargo.toml
  2. 24 18
      acpi/src/fadt.rs
  3. 1 1
      acpi/src/lib.rs
  4. 14 20
      acpi/src/sdt.rs

+ 0 - 1
acpi/Cargo.toml

@@ -12,4 +12,3 @@ edition = "2018"
 [dependencies]
 log = "0.4"
 bit_field = "0.10"
-typenum = "1"

+ 24 - 18
acpi/src/fadt.rs

@@ -1,6 +1,12 @@
-use crate::{sdt::SdtHeader, Acpi, AcpiError, AcpiHandler, AmlTable, GenericAddress, PhysicalMapping};
-
-type ExtendedField<T> = crate::sdt::ExtendedField<T, typenum::U2>;
+use crate::{
+    sdt::{ExtendedField, SdtHeader, ACPI_VERSION_2_0},
+    Acpi,
+    AcpiError,
+    AcpiHandler,
+    AmlTable,
+    GenericAddress,
+    PhysicalMapping,
+};
 
 /// Represents the Fixed ACPI Description Table (FADT). This table contains various fixed hardware
 /// details, such as the addresses of the hardware register blocks. It also contains a pointer to
@@ -58,19 +64,19 @@ pub struct Fadt {
     reset_value: u8,
     arm_boot_arch: u16,
     fadt_minor_version: u8,
-    x_firmware_ctrl: ExtendedField<u64>,
-    x_dsdt_address: ExtendedField<u64>,
-    x_pm1a_event_block: ExtendedField<GenericAddress>,
-    x_pm1b_event_block: ExtendedField<GenericAddress>,
-    x_pm1a_control_block: ExtendedField<GenericAddress>,
-    x_pm1b_control_block: ExtendedField<GenericAddress>,
-    x_pm2_control_block: ExtendedField<GenericAddress>,
-    x_pm_timer_block: ExtendedField<GenericAddress>,
-    x_gpe0_block: ExtendedField<GenericAddress>,
-    x_gpe1_block: ExtendedField<GenericAddress>,
-    sleep_control_reg: ExtendedField<GenericAddress>,
-    sleep_status_reg: ExtendedField<GenericAddress>,
-    hypervisor_vendor_id: ExtendedField<u64>,
+    x_firmware_ctrl: ExtendedField<u64, ACPI_VERSION_2_0>,
+    x_dsdt_address: ExtendedField<u64, ACPI_VERSION_2_0>,
+    x_pm1a_event_block: ExtendedField<GenericAddress, ACPI_VERSION_2_0>,
+    x_pm1b_event_block: ExtendedField<GenericAddress, ACPI_VERSION_2_0>,
+    x_pm1a_control_block: ExtendedField<GenericAddress, ACPI_VERSION_2_0>,
+    x_pm1b_control_block: ExtendedField<GenericAddress, ACPI_VERSION_2_0>,
+    x_pm2_control_block: ExtendedField<GenericAddress, ACPI_VERSION_2_0>,
+    x_pm_timer_block: ExtendedField<GenericAddress, ACPI_VERSION_2_0>,
+    x_gpe0_block: ExtendedField<GenericAddress, ACPI_VERSION_2_0>,
+    x_gpe1_block: ExtendedField<GenericAddress, ACPI_VERSION_2_0>,
+    sleep_control_reg: ExtendedField<GenericAddress, ACPI_VERSION_2_0>,
+    sleep_status_reg: ExtendedField<GenericAddress, ACPI_VERSION_2_0>,
+    hypervisor_vendor_id: ExtendedField<u64, ACPI_VERSION_2_0>,
 }
 
 pub(crate) fn parse_fadt<H>(
@@ -86,8 +92,8 @@ where
 
     let dsdt_address = unsafe {
         fadt.x_dsdt_address
-            .get(fadt.header.revision())
-            .filter(|p| *p != 0)
+            .access(fadt.header.revision())
+            .filter(|&p| p != 0)
             .or(Some(fadt.dsdt_address as u64))
             .filter(|p| *p != 0)
             .map(|p| p as usize)

+ 1 - 1
acpi/src/lib.rs

@@ -22,7 +22,7 @@
 //! gathered from the static tables, and can be queried to set up hardware etc.
 
 #![no_std]
-#![feature(nll, exclusive_range_pattern)]
+#![feature(nll, exclusive_range_pattern, const_generics)]
 
 extern crate alloc;
 #[cfg_attr(test, macro_use)]

+ 14 - 20
acpi/src/sdt.rs

@@ -1,29 +1,23 @@
 use crate::{fadt::Fadt, hpet::HpetTable, madt::Madt, mcfg::Mcfg, Acpi, AcpiError, AcpiHandler, AmlTable};
-use core::{marker::PhantomData, mem, str};
+use core::{mem, mem::MaybeUninit, str};
 use log::{trace, warn};
-use typenum::Unsigned;
 
-/// A union that represents a field that is not necessarily present and is only present in a later
-/// ACPI version (represented by the compile time number and type parameter `R`)
-#[derive(Copy, Clone)]
+pub const ACPI_VERSION_2_0: u8 = 20;
+
+/// Represents a field which may or may not be present within an ACPI structure, depending on the version of ACPI
+/// that a system supports. If the field is not present, it is not safe to treat the data as initialised.
 #[repr(C, packed)]
-pub union ExtendedField<T: Copy, R: Unsigned> {
-    field: T,
-    nothing: (),
-    _phantom: PhantomData<R>,
-}
+pub struct ExtendedField<T: Copy, const MIN_VERSION: u8>(MaybeUninit<T>);
 
-impl<T: Copy, R: Unsigned> ExtendedField<T, R> {
-    /// Checks that the given revision is greater than `R` (typenum revision number) and then
-    /// returns the field, otherwise returning `None`.
-    ///
-    /// # Unsafety
+impl<T: Copy, const MIN_VERSION: u8> ExtendedField<T, MIN_VERSION> {
+    /// Access the field if it's present for the given ACPI version. You should get this version from another ACPI
+    /// structure, such as the RSDT/XSDT.
     ///
-    /// This is unsafe as the given `revision` could be incorrect or fabricated. However, it should
-    /// be safe if it represents the revision of the table this field is present in.
-    pub unsafe fn get(&self, revision: u8) -> Option<T> {
-        if revision >= R::to_u8() {
-            Some(self.field)
+    /// ### Safety
+    /// If a bogus ACPI version is passed, this function may access uninitialised data, which is unsafe.
+    pub unsafe fn access(&self, version: u8) -> Option<T> {
+        if version >= MIN_VERSION {
+            Some(self.0.assume_init())
         } else {
             None
         }