Skip to content

Commit

Permalink
feat(hal): use exposed provenance in vaddr<->ptr conversion (#509)
Browse files Browse the repository at this point in the history
This changes the `VAddr` and `PAddr` types to use
`ptr::expose_provenance` and `ptr::with_exposed_provenance` when
converting to/from raw pointers. Hopefully, this will make things a
little bit less UB.

Fixes #366, more or less.
  • Loading branch information
hawkw committed Jan 11, 2025
1 parent 1be23cb commit 312b382
Show file tree
Hide file tree
Showing 6 changed files with 53 additions and 12 deletions.
2 changes: 1 addition & 1 deletion hal-core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ version = "0.1.0"
authors = ["Eliza Weisman <[email protected]>", "iximeow <[email protected]>"]
edition = "2021"
license = "MIT"
rust-version = "1.81.0"
rust-version = "1.84.0"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

Expand Down
51 changes: 46 additions & 5 deletions hal-core/src/addr.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use core::{fmt, ops};
use core::{fmt, ops, ptr};

pub trait Address:
Copy
Expand Down Expand Up @@ -117,19 +117,40 @@ pub trait Address:
self.is_aligned(core::mem::align_of::<T>())
}

/// Converts this address into a const pointer to a value of type `T`.
///
/// # Panics
///
/// - If `self` is not aligned for a `T`-typed value.
#[inline]
#[track_caller]
fn as_ptr<T>(self) -> *const T {
// Some architectures permit unaligned reads, but Rust considers
// dereferencing a pointer that isn't type-aligned to be UB.
assert!(
self.is_aligned_for::<T>(),
"assertion failed: self.is_aligned_for::<{}>();\n\tself={self:?}",
core::any::type_name::<T>(),
);
ptr::with_exposed_provenance(self.as_usize())
}

/// Converts this address into a mutable pointer to a value of type `T`.
///
/// # Panics
///
/// - If `self` is not aligned for a `T`-typed value.
#[inline]
#[track_caller]
fn as_ptr<T>(self) -> *mut T {
fn as_mut_ptr<T>(self) -> *mut T {
// Some architectures permit unaligned reads, but Rust considers
// dereferencing a pointer that isn't type-aligned to be UB.
assert!(
self.is_aligned_for::<T>(),
"assertion failed: self.is_aligned_for::<{}>();\n\tself={self:?}",
core::any::type_name::<T>(),
);
self.as_usize() as *mut T
ptr::with_exposed_provenance_mut(self.as_usize())
}
}

Expand Down Expand Up @@ -327,13 +348,27 @@ macro_rules! impl_addrs {
Address::is_aligned_for::<T>(self)
}

/// Converts this address into a const pointer to a value of type `T`.
///
/// # Panics
///
/// - If `self` is not aligned for a `T`-typed value.
#[inline]
pub fn as_ptr<T>(self) -> *mut T {
#[track_caller]
pub fn as_ptr<T>(self) -> *const T {
Address::as_ptr(self)
}

/// Converts this address into a mutable pointer to a value of type `T`.
///
/// # Panics
///
/// - If `self` is not aligned for a `T`-typed value.
#[inline]
#[track_caller]
pub fn as_mut_ptr<T>(self) -> *mut T {
Address::as_mut_ptr(self)
}
}
)+
}
Expand Down Expand Up @@ -392,9 +427,15 @@ impl VAddr {
Self(u)
}

/// Constructs a `VAddr` from a `*const T` pointer, exposing its provenance.
#[inline]
pub fn from_ptr<T: ?Sized>(ptr: *const T) -> Self {
Self::from_usize(ptr.expose_provenance())
}

#[inline]
pub fn of<T: ?Sized>(pointee: &T) -> Self {
Self::from_usize(pointee as *const _ as *const () as usize)
Self::from_ptr(pointee as *const T)
}
}

Expand Down
4 changes: 2 additions & 2 deletions hal-core/src/mem/page.rs
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,7 @@ impl<A: Address, S: Size> Page<A, S> {
/// When calling this method, ensure that the page will not be mutated
/// concurrently, including by user code.
pub unsafe fn as_slice(&self) -> &[u8] {
let start = self.base.as_ptr() as *const u8;
let start = self.base.as_mut_ptr() as *const u8;
slice::from_raw_parts::<u8>(start, self.size.as_usize())
}

Expand All @@ -495,7 +495,7 @@ impl<A: Address, S: Size> Page<A, S> {
/// When calling this method, ensure that the page will not be read or mutated
/// concurrently, including by user code.
pub unsafe fn as_slice_mut(&mut self) -> &mut [u8] {
let start = self.base.as_ptr::<u8>() as *mut _;
let start = self.base.as_mut_ptr::<u8>() as *mut _;
slice::from_raw_parts_mut::<u8>(start, self.size.as_usize())
}
}
Expand Down
2 changes: 1 addition & 1 deletion hal-x86_64/src/interrupt/apic/ioapic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ impl IoApic {
tracing::debug!("mapped I/O APIC MMIO page!");
}

let registers = unsafe { Volatile::new(&mut *base.as_ptr::<MmioRegisters>()) };
let registers = unsafe { Volatile::new(&mut *base.as_mut_ptr::<MmioRegisters>()) };
let mut ioapic = Self { registers };
tracing::info!(
addr = ?base,
Expand Down
2 changes: 1 addition & 1 deletion hal-x86_64/src/interrupt/apic/local.rs
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,7 @@ impl LocalApic {
addr.is_aligned(16usize),
"Local APIC memory-mapped registers must be 16-byte aligned!"
);
let reference = &mut *addr.as_ptr::<T>();
let reference = &mut *addr.as_mut_ptr::<T>();
LocalApicRegister::<T, A>::volatile(reference)
}
}
Expand Down
4 changes: 2 additions & 2 deletions hal-x86_64/src/mm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ impl<R: level::Recursive> PageTable<R> {
tracing::trace!(next.addr = ?vaddr, "found next table virtual address");
// XXX(eliza): this _probably_ could be be a `new_unchecked`...if, after
// all this, the next table address is null...we're probably pretty fucked!
Some(unsafe { &*NonNull::new(vaddr.as_ptr())?.as_ptr() })
Some(unsafe { &*NonNull::new(vaddr.as_mut_ptr())?.as_ptr() })
}

#[inline]
Expand Down Expand Up @@ -893,7 +893,7 @@ mycotest::decl_test! {
};
tracing::info!(?page, "page mapped!");

let page_ptr = page.base_addr().as_ptr::<u64>();
let page_ptr = page.base_addr().as_mut_ptr::<u64>();
unsafe { page_ptr.offset(400).write_volatile(0x_f021_f077_f065_f04e)};

tracing::info!("wow, it didn't fault");
Expand Down

0 comments on commit 312b382

Please sign in to comment.