From 330175d80197567b19bfde398fa508738b569f8d Mon Sep 17 00:00:00 2001 From: vasilito Date: Mon, 27 Jul 2026 14:42:23 +0900 Subject: [PATCH] redox-driver-sys: add # Safety docs to dma.rs unsafe blocks Documents the safety contracts for: - alloc_zeroed: matching layout between alloc/dealloc; non-zero size - fmap: valid Map struct and open region_fd - munmap: matches previously successful fmap exactly - dealloc: same layout as matching alloc_zeroed; no concurrent use - Send + Sync impls: process-local mapping, no aliasing across processes Closes the documentation gap for dma.rs unsafe blocks. --- .../redox-driver-sys/source/src/dma.rs | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/local/recipes/drivers/redox-driver-sys/source/src/dma.rs b/local/recipes/drivers/redox-driver-sys/source/src/dma.rs index e784361c1e..3d3be1ebac 100644 --- a/local/recipes/drivers/redox-driver-sys/source/src/dma.rs +++ b/local/recipes/drivers/redox-driver-sys/source/src/dma.rs @@ -182,6 +182,10 @@ impl DmaBuffer { let layout = std::alloc::Layout::from_size_align(size, align) .map_err(|e| DriverError::Other(format!("invalid DMA layout: {e}")))?; + // SAFETY: `alloc_zeroed` is unsafe because the caller must ensure + // `layout` has non-zero size (guaranteed by `size > 0` precondition on the + // public allocate() API) and that the returned allocation will be + // freed with the matching `dealloc` (see Drop impl using the same layout). let ptr = unsafe { std::alloc::alloc_zeroed(layout) }; let ptr = NonNull::new(ptr).ok_or_else(|| { DriverError::Other(format!( @@ -213,6 +217,10 @@ impl DmaBuffer { }; // Map it into our address space through SYS_FMAP with combined map+prot flags. + // SAFETY: `fmap` is unsafe because the caller must ensure `map` is a + // valid Map struct (it is — we constructed it above) and that `region_fd` + // refers to an open physical-memory region. After fmap returns successfully + // the returned pointer is valid for reads/writes within `map.size` bytes. let ptr = unsafe { redox_syscall::call::fmap(region_fd as usize, &map) }.map_err(|e| { let _ = libredox::call::close(region_fd as usize); DriverError::MappingFailed { @@ -296,16 +304,31 @@ impl Drop for DmaBuffer { size, region_fd, } => { + // SAFETY: `munmap` is unsafe because the caller must ensure `ptr..ptr+size` + // exactly matches a previously successful `fmap` mapping. The Drop + // impl runs exactly once and owns this mapping, so the invariant holds. let _ = unsafe { libredox::call::munmap(ptr.as_ptr() as *mut (), *size) }; let _ = libredox::call::close(*region_fd as usize); } DmaStorage::Heap { ptr, layout } => { + // SAFETY: `dealloc` is unsafe because the caller must ensure `ptr` was + // allocated with `alloc_zeroed` using the SAME `layout` (it is — Drop + // owns the layout field) and that the allocation is not in use elsewhere. unsafe { std::alloc::dealloc(ptr.as_ptr(), *layout) }; } } } } +// SAFETY: DmaBuffer is Send + Sync because: +// - The underlying DMA mapping is process-local and the kernel guarantees +// no aliasing between processes. +// - Concurrent MMIO/DMA access from different threads to the SAME buffer +// is racy at the *device* level (the device driver must serialize), but +// Rust's aliasing rules are satisfied: each NonNull pointer is unique. +// - DMA ordering is enforced at the hardware level by the IOMMU/device, not +// by Rust's memory model — concurrent access here is therefore the +// caller's responsibility, not a soundness violation. unsafe impl Send for DmaBuffer {} unsafe impl Sync for DmaBuffer {}