From 312b38288ef0a434beef46c3714a0c3b5d9a41b9 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Sat, 11 Jan 2025 10:10:20 -0800 Subject: [PATCH] feat(hal): use exposed provenance in vaddr<->ptr conversion (#509) 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. --- hal-core/Cargo.toml | 2 +- hal-core/src/addr.rs | 51 ++++++++++++++++++++++--- hal-core/src/mem/page.rs | 4 +- hal-x86_64/src/interrupt/apic/ioapic.rs | 2 +- hal-x86_64/src/interrupt/apic/local.rs | 2 +- hal-x86_64/src/mm.rs | 4 +- 6 files changed, 53 insertions(+), 12 deletions(-) diff --git a/hal-core/Cargo.toml b/hal-core/Cargo.toml index 018ea636..0560d8de 100644 --- a/hal-core/Cargo.toml +++ b/hal-core/Cargo.toml @@ -4,7 +4,7 @@ version = "0.1.0" authors = ["Eliza Weisman ", "iximeow "] 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 diff --git a/hal-core/src/addr.rs b/hal-core/src/addr.rs index 72dcbc5b..b06780a4 100644 --- a/hal-core/src/addr.rs +++ b/hal-core/src/addr.rs @@ -1,4 +1,4 @@ -use core::{fmt, ops}; +use core::{fmt, ops, ptr}; pub trait Address: Copy @@ -117,11 +117,32 @@ pub trait Address: self.is_aligned(core::mem::align_of::()) } + /// 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(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::(), + "assertion failed: self.is_aligned_for::<{}>();\n\tself={self:?}", + core::any::type_name::(), + ); + 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(self) -> *mut T { + fn as_mut_ptr(self) -> *mut T { // Some architectures permit unaligned reads, but Rust considers // dereferencing a pointer that isn't type-aligned to be UB. assert!( @@ -129,7 +150,7 @@ pub trait Address: "assertion failed: self.is_aligned_for::<{}>();\n\tself={self:?}", core::any::type_name::(), ); - self.as_usize() as *mut T + ptr::with_exposed_provenance_mut(self.as_usize()) } } @@ -327,13 +348,27 @@ macro_rules! impl_addrs { Address::is_aligned_for::(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(self) -> *mut T { + #[track_caller] + pub fn as_ptr(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(self) -> *mut T { + Address::as_mut_ptr(self) + } } )+ } @@ -392,9 +427,15 @@ impl VAddr { Self(u) } + /// Constructs a `VAddr` from a `*const T` pointer, exposing its provenance. + #[inline] + pub fn from_ptr(ptr: *const T) -> Self { + Self::from_usize(ptr.expose_provenance()) + } + #[inline] pub fn of(pointee: &T) -> Self { - Self::from_usize(pointee as *const _ as *const () as usize) + Self::from_ptr(pointee as *const T) } } diff --git a/hal-core/src/mem/page.rs b/hal-core/src/mem/page.rs index eb381ff3..10f16509 100644 --- a/hal-core/src/mem/page.rs +++ b/hal-core/src/mem/page.rs @@ -484,7 +484,7 @@ impl Page { /// 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::(start, self.size.as_usize()) } @@ -495,7 +495,7 @@ impl Page { /// 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::() as *mut _; + let start = self.base.as_mut_ptr::() as *mut _; slice::from_raw_parts_mut::(start, self.size.as_usize()) } } diff --git a/hal-x86_64/src/interrupt/apic/ioapic.rs b/hal-x86_64/src/interrupt/apic/ioapic.rs index 0a0426df..f70447a4 100644 --- a/hal-x86_64/src/interrupt/apic/ioapic.rs +++ b/hal-x86_64/src/interrupt/apic/ioapic.rs @@ -348,7 +348,7 @@ impl IoApic { tracing::debug!("mapped I/O APIC MMIO page!"); } - let registers = unsafe { Volatile::new(&mut *base.as_ptr::()) }; + let registers = unsafe { Volatile::new(&mut *base.as_mut_ptr::()) }; let mut ioapic = Self { registers }; tracing::info!( addr = ?base, diff --git a/hal-x86_64/src/interrupt/apic/local.rs b/hal-x86_64/src/interrupt/apic/local.rs index 7e49975c..de6f341a 100644 --- a/hal-x86_64/src/interrupt/apic/local.rs +++ b/hal-x86_64/src/interrupt/apic/local.rs @@ -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::(); + let reference = &mut *addr.as_mut_ptr::(); LocalApicRegister::::volatile(reference) } } diff --git a/hal-x86_64/src/mm.rs b/hal-x86_64/src/mm.rs index af90c7e9..d8500e37 100644 --- a/hal-x86_64/src/mm.rs +++ b/hal-x86_64/src/mm.rs @@ -303,7 +303,7 @@ impl PageTable { 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] @@ -893,7 +893,7 @@ mycotest::decl_test! { }; tracing::info!(?page, "page mapped!"); - let page_ptr = page.base_addr().as_ptr::(); + let page_ptr = page.base_addr().as_mut_ptr::(); unsafe { page_ptr.offset(400).write_volatile(0x_f021_f077_f065_f04e)}; tracing::info!("wow, it didn't fault");