Merge pull request #1988 from rust-osdev/safetycomments

treewide: enforce safety comments

Author: phip1611

uefi-raw/src/lib.rs

@@ -15,8 +15,10 @@
 #![deny(
     clippy::all,
     clippy::missing_const_for_fn,
+    clippy::missing_safety_doc,
     clippy::must_use_candidate,
     clippy::ptr_as_ptr,
+    clippy::undocumented_unsafe_blocks,
     clippy::use_self,
     missing_debug_implementations,
     unused

uefi-raw/src/net.rs

@@ -321,11 +321,15 @@ mod tests {
     fn test_ip_addr_conversion() {
         let core_addr = core::net::IpAddr::V4(core::net::Ipv4Addr::from(TEST_IPV4));
         let uefi_addr = IpAddress::from(core_addr);
-        assert_eq!(unsafe { uefi_addr.v4.0 }, TEST_IPV4);
+        // SAFETY: `IpAddress::from` constructed the IPv4 variant above.
+        let octets = unsafe { uefi_addr.v4.0 };
+        assert_eq!(octets, TEST_IPV4);
 
         let core_addr = core::net::IpAddr::V6(core::net::Ipv6Addr::from(TEST_IPV6));
         let uefi_addr = IpAddress::from(core_addr);
-        assert_eq!(unsafe { uefi_addr.v6.0 }, TEST_IPV6);
+        // SAFETY: `IpAddress::from` constructed the IPv6 variant above.
+        let octets = unsafe { uefi_addr.v6.0 };
+        assert_eq!(octets, TEST_IPV6);
     }
 
     /// Test conversions between `MacAddress` and octet arrays.
@@ -373,40 +377,42 @@ mod tests {
             let octets = [0_u8, 1, 2, 3];
             assert_eq!(Ipv4Address::from(octets), Ipv4Address(octets));
             let uefi_addr = IpAddress::from(octets);
+            // SAFETY: `from([u8; 4])` constructs the IPv4 variant.
             assert_eq!(&octets, &unsafe { uefi_addr.v4.octets() });
         }
         // octets -> Ipv6Address
         {
             let octets = [0_u8, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15];
             assert_eq!(Ipv6Address::from(octets), Ipv6Address(octets));
             let uefi_addr = IpAddress::from(octets);
+            // SAFETY: `from([u8; 16])` constructs the IPv6 variant.
             assert_eq!(&octets, &unsafe { uefi_addr.v6.octets() });
         }
         // StdIpv4Addr -> Ipv4Address
         {
             let octets = [7, 5, 3, 1];
             let core_ipv4_addr = core::net::Ipv4Addr::from(octets);
             assert_eq!(Ipv4Address::from(core_ipv4_addr).octets(), octets);
-            assert_eq!(
-                unsafe { IpAddress::from(core_ipv4_addr).v4.octets() },
-                octets
-            );
+            // SAFETY: The `Ipv4Addr` conversion selects the IPv4 variant.
+            let converted = unsafe { IpAddress::from(core_ipv4_addr).v4.octets() };
+            assert_eq!(converted, octets);
         }
         // StdIpv6Addr -> Ipv6Address
         {
             let octets = [7, 5, 3, 1, 6, 3, 8, 5, 2, 5, 2, 7, 3, 5, 2, 6];
             let core_ipv6_addr = core::net::Ipv6Addr::from(octets);
             assert_eq!(Ipv6Address::from(core_ipv6_addr).octets(), octets);
-            assert_eq!(
-                unsafe { IpAddress::from(core_ipv6_addr).v6.octets() },
-                octets
-            );
+            // SAFETY: The `Ipv6Addr` conversion selects the IPv6 variant.
+            let converted = unsafe { IpAddress::from(core_ipv6_addr).v6.octets() };
+            assert_eq!(converted, octets);
         }
         // StdIpAddr -> IpAddress
         {
             let octets = [8, 8, 2, 6];
             let core_ip_addr = core::net::IpAddr::from(octets);
-            assert_eq!(unsafe { IpAddress::from(core_ip_addr).v4.octets() }, octets);
+            // SAFETY: This `IpAddr` is built from four octets, so it is IPv4.
+            let converted = unsafe { IpAddress::from(core_ip_addr).v4.octets() };
+            assert_eq!(converted, octets);
         }
         // octets <-> MacAddress
         {
@@ -435,15 +441,17 @@ mod tests {
     #[test]
     fn test_uefi_flow() {
         fn efi_retrieve_efi_ip_addr(addr: *mut IpAddress, is_ipv6: bool) {
+            // SAFETY: The caller passes a valid, writable `IpAddress` pointer.
             let addr = unsafe { addr.as_mut().unwrap() };
-            // SAFETY: Alignment is guaranteed and memory is initialized.
+            // SAFETY: The selected variant is the one initialized by the caller.
             unsafe {
                 addr.v4.0[0] = 42;
                 addr.v4.0[1] = 42;
                 addr.v4.0[2] = 42;
                 addr.v4.0[3] = 42;
             }
             if is_ipv6 {
+                // SAFETY: The IPv6 branch writes the bytes that the caller will read back.
                 unsafe {
                     addr.v6.0[14] = 42;
                     addr.v6.0[15] = 42;
@@ -454,6 +462,7 @@ mod tests {
         fn high_level_retrieve_ip(is_ipv6: bool) -> core::net::IpAddr {
             let mut efi_ip_addr = IpAddress::ZERO;
             efi_retrieve_efi_ip_addr(&mut efi_ip_addr, is_ipv6);
+            // SAFETY: The helper initialized the variant selected by `is_ipv6`.
             unsafe { efi_ip_addr.into_core_addr(is_ipv6) }
         }
 

uefi-raw/src/protocol/hii/image.rs

@@ -50,11 +50,14 @@ pub union ImageOutputDest {
 
 impl fmt::Debug for ImageOutputDest {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        // All union fields are pointers.
-        f.debug_struct("ImageOutputDest")
-            .field("bitmap", unsafe { &self.bitmap })
-            .field("screen", unsafe { &self.screen })
-            .finish()
+        // SAFETY: Both union members are raw pointers, so reading either field
+        // only copies the stored address.
+        unsafe {
+            f.debug_struct("ImageOutputDest")
+                .field("bitmap", &self.bitmap)
+                .field("screen", &self.screen)
+                .finish()
+        }
     }
 }
 

uefi-raw/src/protocol/network/pxe.rs

@@ -302,18 +302,24 @@ impl Debug for PxeBaseCodePacket {
 
 impl AsRef<[u8; 1472]> for PxeBaseCodePacket {
     fn as_ref(&self) -> &[u8; 1472] {
+        // SAFETY: The packet union is defined with `raw` as the byte view for
+        // the full storage, and any byte pattern is valid here.
         unsafe { &self.raw }
     }
 }
 
 impl AsRef<PxeBaseCodeDhcpV4Packet> for PxeBaseCodePacket {
     fn as_ref(&self) -> &PxeBaseCodeDhcpV4Packet {
+        // SAFETY: The caller chooses this view; the union stores the packet
+        // bytes in a layout compatible with the DHCPv4 packet type.
         unsafe { &self.dhcpv4 }
     }
 }
 
 impl AsRef<PxeBaseCodeDhcpV6Packet> for PxeBaseCodePacket {
     fn as_ref(&self) -> &PxeBaseCodeDhcpV6Packet {
+        // SAFETY: The caller chooses this view; the union stores the packet
+        // bytes in a layout compatible with the DHCPv6 packet type.
         unsafe { &self.dhcpv6 }
     }
 }

uefi/src/allocator.rs

@@ -62,13 +62,15 @@ fn alloc_pool_aligned(memory_type: MemoryType, size: usize, align: usize) -> *mu
         offset = align;
     }
 
-    // Before returning the aligned allocation, store a pointer to the
-    // full unaligned allocation in the bytes just before the aligned
-    // allocation. We know we have at least eight bytes there due to
-    // adding `align` to the memory allocation size. We also know the
-    // write is appropriately aligned for a `*mut u8` pointer because
-    // `align_ptr` is aligned, and alignments are always powers of two
-    // (as enforced by the `Layout` type).
+    // SAFETY: `full_alloc_ptr` points to a valid allocation large enough for
+    // `size + align + size_of::<*mut u8>()`. Adding `offset` keeps
+    // `aligned_ptr` within that allocation and yields the requested alignment.
+    // The allocation size includes enough extra space to reserve the bytes
+    // immediately preceding `aligned_ptr`, where we store `full_alloc_ptr` so
+    // the original allocation can later be recovered. Because `aligned_ptr` is
+    // aligned and valid alignments are powers of two as enforced by `Layout`,
+    // the slot at `aligned_ptr.cast::<*mut u8>().sub(1)` is properly aligned
+    // for writing a `*mut u8`.
     unsafe {
         let aligned_ptr = full_alloc_ptr.add(offset);
         (aligned_ptr.cast::<*mut u8>()).sub(1).write(full_alloc_ptr);
@@ -100,6 +102,9 @@ const fn layout_allows_page_alloc_shortcut(layout: &Layout) -> bool {
 #[derive(Debug)]
 pub struct Allocator;
 
+// SAFETY: `alloc` delegates to UEFI's pool allocator, which returns correctly
+// aligned, non-aliasing memory. `dealloc` only frees pointers previously returned
+// by the matching `alloc` call with the same layout.
 unsafe impl GlobalAlloc for Allocator {
     /// Allocate memory using the UEFI boot services.
     ///
@@ -151,19 +156,23 @@ unsafe impl GlobalAlloc for Allocator {
                 // To spammy, but useful for manual testing.
                 // log::trace!("Taking PAGE_SIZE shortcut for layout={layout:?}");
                 let count = layout.size().div_ceil(PAGE_SIZE);
+                // SAFETY: This pointer was allocated by the matching UEFI allocator.
                 unsafe { boot::free_pages(ptr, count).unwrap() }
             }
             (false, 0..=8 /* UEFI default alignment */) => {
                 // Warning: this will panic after exiting boot services.
+                // SAFETY: This pointer was allocated by the matching UEFI allocator.
                 unsafe { boot::free_pool(ptr) }.unwrap();
             }
             (false, 9..) => {
                 let ptr = ptr.as_ptr().cast::<*mut u8>();
-                // Retrieve the pointer to the full allocation that was packed right
-                // before the aligned allocation in `alloc`.
+                // SAFETY: Retrieve the in-bounds pointer to the full allocation
+                // that was packed right before the aligned allocation in
+                // `alloc()`.
                 let actual_alloc_ptr = unsafe { ptr.sub(1).read() };
                 let ptr = NonNull::new(actual_alloc_ptr).unwrap();
                 // Warning: this will panic after exiting boot services.
+                // SAFETY: This pointer was allocated by the matching UEFI allocator.
                 unsafe { boot::free_pool(ptr) }.unwrap();
             }
         }