style: restrict to one unsafe operation per block

crates/sample-kmdf-driver/src/lib.rs

@@ -4,11 +4,14 @@
 #![no_std]
 #![cfg_attr(feature = "nightly", feature(hint_must_use))]
 #![deny(warnings)]
+#![deny(unsafe_op_in_unsafe_fn)]
 #![deny(clippy::all)]
 #![deny(clippy::pedantic)]
 #![deny(clippy::nursery)]
 #![deny(clippy::cargo)]
+#![deny(clippy::undocumented_unsafe_blocks)]
 #![deny(clippy::unnecessary_safety_doc)]
+#![deny(clippy::multiple_unsafe_ops_per_block)]
 
 extern crate alloc;
 
@@ -28,6 +31,8 @@ use wdk_sys::{
     NTSTATUS,
     PCUNICODE_STRING,
     ULONG,
+    UNICODE_STRING,
+    WCHAR,
     WDFDEVICE,
     WDFDEVICE_INIT,
     WDFDRIVER,
@@ -54,6 +59,9 @@ pub unsafe extern "system" fn driver_entry(
 ) -> NTSTATUS {
     // This is an example of directly using DbgPrint binding to print
     let string = CString::new("Hello World!\n").unwrap();
+
+    // SAFETY: This is safe because `string` is a valid pointer to a null-terminated
+    // string
     unsafe {
         DbgPrint(string.as_ptr());
     }
@@ -81,24 +89,60 @@ pub unsafe extern "system" fn driver_entry(
     let driver_attributes = WDF_NO_OBJECT_ATTRIBUTES;
     let driver_handle_output = WDF_NO_HANDLE.cast::<*mut wdk_sys::WDFDRIVER__>();
 
-    let wdf_driver_create_ntstatus = unsafe {
-        call_unsafe_wdf_function_binding!(
+    let wdf_driver_create_ntstatus;
+    // SAFETY: This is safe because:
+    //         1. `driver` is provided by `DriverEntry` and is never null
+    //         2. `registry_path` is provided by `DriverEntry` and is never null
+    //         3. `driver_attributes` is allowed to be null
+    //         4. `driver_config` is a valid pointer to a valid `WDF_DRIVER_CONFIG`
+    //         5. `driver_handle_output` is expected to be null
+    unsafe {
+        #![allow(clippy::multiple_unsafe_ops_per_block)]
+        wdf_driver_create_ntstatus = call_unsafe_wdf_function_binding!(
             WdfDriverCreate,
             driver as wdk_sys::PDRIVER_OBJECT,
             registry_path,
             driver_attributes,
             &mut driver_config,
             driver_handle_output,
-        )
-    };
+        );
+    }
 
     // Translate UTF16 string to rust string
-    let registry_path = String::from_utf16_lossy(unsafe {
-        slice::from_raw_parts(
-            (*registry_path).Buffer,
-            (*registry_path).Length as usize / core::mem::size_of_val(&(*(*registry_path).Buffer)),
-        )
-    });
+    let registry_path: UNICODE_STRING =
+        // SAFETY: This dereference is safe since `registry_path` is:
+        //         * provided by `DriverEntry` and is never null
+        //         * a valid pointer to a `UNICODE_STRING`
+        unsafe { *registry_path };
+    let number_of_slice_elements = {
+        registry_path.Length as usize
+            / core::mem::size_of_val(
+                // SAFETY: This dereference is safe since `Buffer` is:
+                //         * provided by `DriverEntry` and is never null
+                //         * a valid pointer to `Buffer`'s type
+                &unsafe { *registry_path.Buffer },
+            )
+    };
+
+    let registry_path = String::from_utf16_lossy(
+        // SAFETY: This is safe because:
+        //         1. `registry_path.Buffer` is valid for reads for `number_of_slice_elements` *
+        //            `core::mem::size_of::<WCHAR>()` bytes, and is guaranteed to be aligned and it
+        //            must be properly aligned.
+        //         2. `registry_path.Buffer` points to `number_of_slice_elements` consecutive
+        //            properly initialized values of type `WCHAR`.
+        //         3. Windows does not mutate the memory referenced by the returned slice for for
+        //            its entire lifetime.
+        //         4. The total size, `number_of_slice_elements` * `core::mem::size_of::<WCHAR>()`,
+        //            of the slice must be no larger than `isize::MAX`. This is proven by the below
+        //            `debug_assert!`.
+        unsafe {
+            debug_assert!(
+                isize::try_from(number_of_slice_elements * core::mem::size_of::<WCHAR>()).is_ok()
+            );
+            slice::from_raw_parts(registry_path.Buffer, number_of_slice_elements)
+        },
+    );
 
     // It is much better to use the println macro that has an implementation in
     // wdk::print.rs to call DbgPrint. The println! implementation in
@@ -117,16 +161,23 @@ extern "C" fn evt_driver_device_add(
 
     let mut device_handle_output: WDFDEVICE = WDF_NO_HANDLE.cast();
 
-    let ntstatus = unsafe {
-        wdk_macros::call_unsafe_wdf_function_binding!(
+    let ntstatus;
+    // SAFETY: This is safe because:
+    //       1. `device_init` is provided by `EvtDriverDeviceAdd` and is never null
+    //       2. the argument receiving `WDF_NO_OBJECT_ATTRIBUTES` is allowed to be
+    //          null
+    //       3. `device_handle_output` is expected to be null
+    unsafe {
+        #![allow(clippy::multiple_unsafe_ops_per_block)]
+        ntstatus = wdk_macros::call_unsafe_wdf_function_binding!(
             WdfDeviceCreate,
             &mut device_init,
             WDF_NO_OBJECT_ATTRIBUTES,
             &mut device_handle_output,
-        )
-    };
-    println!("WdfDeviceCreate NTSTATUS: {ntstatus:#02x}");
+        );
+    }
 
+    println!("WdfDeviceCreate NTSTATUS: {ntstatus:#02x}");
     ntstatus
 }
 

crates/wdk-alloc/src/lib.rs

@@ -17,10 +17,12 @@
 #![no_std]
 #![deny(warnings)]
 #![deny(missing_docs)]
+#![deny(unsafe_op_in_unsafe_fn)]
 #![deny(clippy::all)]
 #![deny(clippy::pedantic)]
 #![deny(clippy::nursery)]
 #![deny(clippy::cargo)]
+#![deny(clippy::multiple_unsafe_ops_per_block)]
 #![deny(clippy::undocumented_unsafe_blocks)]
 #![deny(clippy::unnecessary_safety_doc)]
 #![deny(rustdoc::broken_intra_doc_links)]
@@ -45,6 +47,10 @@ use wdk_sys::{
 
 /// Allocator implementation to use with `#[global_allocator]` to allow use of
 /// [`core::alloc`].
+///
+/// # Safety
+/// This allocator is only safe to use for allocations happening at `IRQL` <=
+/// `DISPATCH_LEVEL`
 pub struct WDKAllocator;
 
 // The value of memory tags are stored in little-endian order, so it is
@@ -67,14 +73,22 @@ lazy_static! {
 //            supported)
 unsafe impl GlobalAlloc for WDKAllocator {
     unsafe fn alloc(&self, layout: Layout) -> *mut u8 {
-        let ptr = ExAllocatePool2(POOL_FLAG_NON_PAGED, layout.size() as SIZE_T, *RUST_TAG);
+        let ptr =
+            // SAFETY: `ExAllocatePool2` is safe to call from any `IRQL` <= `DISPATCH_LEVEL` since its allocating from `POOL_FLAG_NON_PAGED`
+            unsafe {
+                ExAllocatePool2(POOL_FLAG_NON_PAGED, layout.size() as SIZE_T, *RUST_TAG)
+            };
         if ptr.is_null() {
             return core::ptr::null_mut();
         }
         ptr.cast()
     }
 
     unsafe fn dealloc(&self, ptr: *mut u8, _layout: Layout) {
-        ExFreePool(ptr.cast());
+        // SAFETY: `ExFreePool` is safe to call from any `IRQL` <= `DISPATCH_LEVEL`
+        // since its freeing memory allocated from `POOL_FLAG_NON_PAGED` in `alloc`
+        unsafe {
+            ExFreePool(ptr.cast());
+        }
     }
 }

crates/wdk-build/src/lib.rs

@@ -12,10 +12,12 @@
 #![cfg_attr(nightly_toolchain, feature(assert_matches))]
 #![deny(warnings)]
 #![deny(missing_docs)]
+#![deny(unsafe_op_in_unsafe_fn)]
 #![deny(clippy::all)]
 #![deny(clippy::pedantic)]
 #![deny(clippy::nursery)]
 #![deny(clippy::cargo)]
+#![deny(clippy::multiple_unsafe_ops_per_block)]
 #![deny(clippy::undocumented_unsafe_blocks)]
 #![deny(clippy::unnecessary_safety_doc)]
 #![deny(rustdoc::broken_intra_doc_links)]

crates/wdk-build/src/utils.rs

@@ -170,10 +170,18 @@ fn read_registry_key_string_value(
 ) -> Option<String> {
     let mut opened_key_handle = HKEY::default();
     let mut len = 0;
-    // SAFETY: FIXME seperate unsafe blocks
-    unsafe {
-        if RegOpenKeyExA(key_handle, sub_key, 0, KEY_READ, &mut opened_key_handle).is_ok() {
-            if RegGetValueA(
+    if
+    // SAFETY: `&mut opened_key_handle` is coerced to a &raw mut, so the address passed as the
+    // argument is always valid. `&mut opened_key_handle` is coerced to a pointer of the correct
+    // type.
+    unsafe { RegOpenKeyExA(key_handle, sub_key, 0, KEY_READ, &mut opened_key_handle) }.is_ok() {
+        if
+        // SAFETY: `opened_key_handle` is valid key opened with the `KEY_QUERY_VALUE` access right
+        // (included in `KEY_READ`). `&mut len` is coerced to a &raw mut, so the address passed as
+        // the argument is always valid. `&mut len` is coerced to a pointer of the correct
+        // type.
+        unsafe {
+            RegGetValueA(
                 opened_key_handle,
                 None,
                 value,
@@ -182,10 +190,19 @@ fn read_registry_key_string_value(
                 None,
                 Some(&mut len),
             )
-            .is_ok()
-            {
-                let mut buffer = vec![0u8; len as usize];
-                if RegGetValueA(
+        }
+        .is_ok()
+        {
+            let mut buffer = vec![0u8; len as usize];
+            if
+            // SAFETY: `opened_key_handle` is valid key opened with the `KEY_QUERY_VALUE` access
+            // right (included in `KEY_READ`). `&mut buffer` is coerced to a &raw mut,
+            // so the address passed as the argument is always valid. `&mut buffer` is
+            // coerced to a pointer of the correct type. `&mut len` is coerced to a &raw
+            // mut, so the address passed as the argument is always valid. `&mut len` is
+            // coerced to a pointer of the correct type.
+            unsafe {
+                RegGetValueA(
                     opened_key_handle,
                     None,
                     value,
@@ -194,21 +211,31 @@ fn read_registry_key_string_value(
                     Some(buffer.as_mut_ptr().cast()),
                     Some(&mut len),
                 )
-                .is_ok()
-                {
-                    RegCloseKey(opened_key_handle)
-                        .expect("opened_key_handle should be successfully closed");
-                    return Some(
-                        CStr::from_bytes_with_nul_unchecked(&buffer[..len as usize])
-                            .to_str()
-                            .expect("Registry value should be parseable as utf8")
-                            .to_string(),
-                    );
-                }
             }
-            RegCloseKey(opened_key_handle)
-                .expect(r"opened_key_handle should be successfully closed");
+            .is_ok()
+            {
+                // SAFETY: `opened_key_handle` is valid opened key that was opened by
+                // `RegOpenKeyExA`
+                unsafe { RegCloseKey(opened_key_handle) }
+                    .expect("opened_key_handle should be successfully closed");
+                return Some(
+                    CStr::from_bytes_with_nul(&buffer[..len as usize])
+                        .expect(
+                            "RegGetValueA should always return a null terminated string. The read \
+                             string (REG_SZ) from the registry should not contain any interior \
+                             nulls.",
+                        )
+                        .to_str()
+                        .expect("Registry value should be parseable as UTF8")
+                        .to_string(),
+                );
+            }
         }
+
+        // SAFETY: `opened_key_handle` is valid opened key that was opened by
+        // `RegOpenKeyExA`
+        unsafe { RegCloseKey(opened_key_handle) }
+            .expect(r"opened_key_handle should be successfully closed");
     }
     None
 }
@@ -304,7 +331,7 @@ mod tests {
     #[test]
     fn read_reg_key_programfilesdir() {
         let program_files_dir =
-            // SAFETY: FOLDERID_ProgramFiles is a constant from the windows crate, so dereference a pointer re-borrowed from its reference is always valid
+            // SAFETY: FOLDERID_ProgramFiles is a constant from the windows crate, so the pointer (resulting from its reference being coerced) is always valid to be dereferenced
             unsafe { SHGetKnownFolderPath(&FOLDERID_ProgramFiles, KF_FLAG_DEFAULT, None) }
                 .expect("Program Files Folder should always resolve via SHGetKnownFolderPath.");
 

crates/wdk-macros/src/lib.rs

@@ -6,10 +6,12 @@
 #![cfg_attr(feature = "nightly", feature(hint_must_use))]
 #![deny(warnings)]
 #![deny(missing_docs)]
+#![deny(unsafe_op_in_unsafe_fn)]
 #![deny(clippy::all)]
 #![deny(clippy::pedantic)]
 #![deny(clippy::nursery)]
 #![deny(clippy::cargo)]
+#![deny(clippy::multiple_unsafe_ops_per_block)]
 #![deny(clippy::undocumented_unsafe_blocks)]
 #![deny(clippy::unnecessary_safety_doc)]
 #![deny(rustdoc::broken_intra_doc_links)]
@@ -41,6 +43,11 @@ use syn::{
 /// from the WDF function table, and then calls it with the arguments passed to
 /// it
 ///
+/// # Safety
+/// Function arguments must abide by any rules outlined in the WDF
+/// documentation. This macro does not perform any validation of the arguments
+/// passed to it., beyond type validation.
+///
 /// # Examples
 ///
 /// ```rust, no_run
@@ -70,6 +77,7 @@ use syn::{
 ///     }
 /// }
 /// ```
+#[allow(clippy::unnecessary_safety_doc)]
 #[proc_macro]
 pub fn call_unsafe_wdf_function_binding(input_tokens: TokenStream) -> TokenStream {
     call_unsafe_wdf_function_binding_impl(TokenStream2::from(input_tokens)).into()
@@ -108,20 +116,48 @@ fn call_unsafe_wdf_function_binding_impl(input_tokens: TokenStream2) -> TokenStr
         Err(err) => return err.to_compile_error(),
     };
 
+    // let inner_attribute_macros = proc_macro2::TokenStream::from_str(
+    //     "#![allow(unused_unsafe)]\n\
+    //      #![allow(clippy::multiple_unsafe_ops_per_block)]",
+    // ).expect("inner_attribute_macros must be convertible to a valid
+    // TokenStream");
+
     let wdf_function_call_tokens = quote! {
         {
+            // Force the macro to require an unsafe block
+            unsafe fn force_unsafe(){}
+            force_unsafe();
+
             // Get handle to WDF function from the function table
-            let wdf_function: wdk_sys::#function_pointer_type = Some(core::mem::transmute(
-                // FIXME: investigate why _WDFFUNCENUM does not have a generated type alias without the underscore prefix
-                wdk_sys::WDF_FUNCTION_TABLE[wdk_sys::_WDFFUNCENUM::#function_table_index as usize],
-            ));
+            let wdf_function: wdk_sys::#function_pointer_type = Some(
+                // SAFETY: This `transmute` from a no-argument function pointer to a function pointer with the correct
+                //         arguments for the WDF function is safe befause WDF maintains the strict mapping between the
+                //         function table index and the correct function pointer type.
+                #[allow(unused_unsafe)]
+                #[allow(clippy::multiple_unsafe_ops_per_block)]
+                unsafe {
+                    core::mem::transmute(
+                        // FIXME: investigate why _WDFFUNCENUM does not have a generated type alias without the underscore prefix
+                        wdk_sys::WDF_FUNCTION_TABLE[wdk_sys::_WDFFUNCENUM::#function_table_index as usize],
+                    )
+                }
+            );
 
-            // Call the WDF function with the supplied args. This mirrors what happens in the inlined WDF function in the various wdf headers(ex. wdfdriver.h)
+            // Call the WDF function with the supplied args. This mirrors what happens in the inlined WDF function in
+            // the various wdf headers(ex. wdfdriver.h)
             if let Some(wdf_function) = wdf_function {
-                (wdf_function)(
-                    wdk_sys::WdfDriverGlobals,
-                    #function_arguments
-                )
+                // SAFETY: The WDF function pointer is always valid because its an entry in
+                // `wdk_sys::WDF_FUNCTION_TABLE` indexed by `function_table_index` and guarded by the type-safety of
+                // `function_pointer_type`. The passed arguments are also guaranteed to be of a compatible type due to
+                // `function_pointer_type`.
+                #[allow(unused_unsafe)]
+                #[allow(clippy::multiple_unsafe_ops_per_block)]
+                unsafe {
+                    (wdf_function)(
+                        wdk_sys::WdfDriverGlobals,
+                        #function_arguments
+                    )
+                }
             } else {
                 unreachable!("Option should never be None");
             }