Skip to content

Commit 915e816

Browse files
committed
Make the MADT !Unpin to prevent unsoundness
This prevents an `Madt` being moved away from its following, but not represented, dynamic entries. This would previously have caused unsoundness as arbitrary memory would be read from after wherever the `Madt` had ended up. By prevening the user from getting anything other than a `Pin<&Madt>`, this is prevented.
1 parent 753a89a commit 915e816

File tree

1 file changed

+21
-10
lines changed

1 file changed

+21
-10
lines changed

acpi/src/madt.rs

+21-10
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,11 @@ use crate::{
44
AcpiTable,
55
};
66
use bit_field::BitField;
7-
use core::{marker::PhantomData, mem};
7+
use core::{
8+
marker::{PhantomData, PhantomPinned},
9+
mem,
10+
pin::Pin,
11+
};
812

913
#[cfg(feature = "allocator_api")]
1014
use crate::{
@@ -29,16 +33,22 @@ pub enum MadtError {
2933
/// to read each entry from it.
3034
///
3135
/// In modern versions of ACPI, the MADT can detail one of four interrupt models:
32-
/// * The ancient dual-i8259 legacy PIC model
33-
/// * The Advanced Programmable Interrupt Controller (APIC) model
34-
/// * The Streamlined Advanced Programmable Interrupt Controller (SAPIC) model (for Itanium systems)
35-
/// * The Generic Interrupt Controller (GIC) model (for ARM systems)
36-
#[repr(C, packed)]
36+
/// - The ancient dual-i8259 legacy PIC model
37+
/// - The Advanced Programmable Interrupt Controller (APIC) model
38+
/// - The Streamlined Advanced Programmable Interrupt Controller (SAPIC) model (for Itanium systems)
39+
/// - The Generic Interrupt Controller (GIC) model (for ARM systems)
40+
///
41+
/// The MADT is a variable-sized structure consisting of a static header and then a variable number of entries.
42+
/// This type only contains the static portion, and then uses pointer arithmetic to parse the following entries.
43+
/// To make this sound, this type is `!Unpin` - this prevents you from getting anything other than a `Pin<&Madt>`
44+
/// out of a `PhysicalMapping`, thereby preventing a `Madt` from being moved before [`Madt::entries`] is called.
45+
#[repr(C)]
3746
#[derive(Debug)]
3847
pub struct Madt {
3948
pub header: SdtHeader,
4049
pub local_apic_address: u32,
4150
pub flags: u32,
51+
_pinned: PhantomPinned,
4252
}
4353

4454
/// ### Safety: Implementation properly represents a valid MADT.
@@ -62,7 +72,7 @@ impl Madt {
6272

6373
#[cfg(feature = "allocator_api")]
6474
pub fn parse_interrupt_model_in<'a, A>(
65-
&self,
75+
self: Pin<&Self>,
6676
allocator: A,
6777
) -> AcpiResult<(InterruptModel<'a, A>, Option<ProcessorInfo<'a, A>>)>
6878
where
@@ -107,7 +117,7 @@ impl Madt {
107117

108118
#[cfg(feature = "allocator_api")]
109119
fn parse_apic_model_in<'a, A>(
110-
&self,
120+
self: Pin<&Self>,
111121
allocator: A,
112122
) -> AcpiResult<(InterruptModel<'a, A>, Option<ProcessorInfo<'a, A>>)>
113123
where
@@ -311,9 +321,10 @@ impl Madt {
311321
))
312322
}
313323

314-
pub fn entries(&self) -> MadtEntryIter {
324+
pub fn entries(self: Pin<&Self>) -> MadtEntryIter<'_> {
325+
let ptr = unsafe { Pin::into_inner_unchecked(self) as *const Madt as *const u8 };
315326
MadtEntryIter {
316-
pointer: unsafe { (self as *const Madt as *const u8).add(mem::size_of::<Madt>()) },
327+
pointer: unsafe { ptr.add(mem::size_of::<Madt>()) },
317328
remaining_length: self.header.length - mem::size_of::<Madt>() as u32,
318329
_phantom: PhantomData,
319330
}

0 commit comments

Comments
 (0)