Enable various new clippy lints

rust/bindings/lib.rs

@@ -15,6 +15,7 @@
 #![cfg_attr(test, allow(unsafe_op_in_unsafe_fn))]
 #![allow(
     clippy::all,
+    clippy::undocumented_unsafe_blocks,
     missing_docs,
     non_camel_case_types,
     non_upper_case_globals,

rust/kernel/allocator.rs

@@ -9,17 +9,22 @@ use crate::bindings;
 
 struct KernelAllocator;
 
+// SAFETY: this implementation meets `GlobalAlloc` safety invariants:
+// - `kalloc` does not unwind
+// - `kalloc` has no stricter safety requirements than those of `GlobalAlloc` itself
+// - Allocating has no further side effects
 unsafe impl GlobalAlloc for KernelAllocator {
     unsafe fn alloc(&self, layout: Layout) -> *mut u8 {
         // `krealloc()` is used instead of `kmalloc()` because the latter is
         // an inline function and cannot be bound to as a result.
-        unsafe { bindings::krealloc(ptr::null(), layout.size(), bindings::GFP_KERNEL) as *mut u8 }
+        // SAFETY: `krealloc` is a FFI call with no invariants to meet
+        unsafe { bindings::krealloc(ptr::null(), layout.size(), bindings::GFP_KERNEL).cast() }
     }
 
     unsafe fn dealloc(&self, ptr: *mut u8, _layout: Layout) {
-        unsafe {
-            bindings::kfree(ptr as *const core::ffi::c_void);
-        }
+        // SAFETY: `dealloc` has the invariant that `ptr` was allocated by this
+        // allocator. `kfree` has no additional invariants.
+        unsafe { bindings::kfree(ptr.cast()) };
     }
 }
 
@@ -33,32 +38,31 @@ static ALLOCATOR: KernelAllocator = KernelAllocator;
 // Note that `#[no_mangle]` implies exported too, nowadays.
 #[no_mangle]
 fn __rust_alloc(size: usize, _align: usize) -> *mut u8 {
-    unsafe { bindings::krealloc(core::ptr::null(), size, bindings::GFP_KERNEL) as *mut u8 }
+    // SAFETY: `krealloc` is a FFI call with no invariants to meet
+    unsafe { bindings::krealloc(core::ptr::null(), size, bindings::GFP_KERNEL).cast() }
 }
 
 #[no_mangle]
 fn __rust_dealloc(ptr: *mut u8, _size: usize, _align: usize) {
-    unsafe { bindings::kfree(ptr as *const core::ffi::c_void) };
+    // SAFETY: `ptr` only ever comes from `krealloc` via `__rust_alloc`
+    unsafe { bindings::kfree(ptr.cast()) };
 }
 
 #[no_mangle]
 fn __rust_realloc(ptr: *mut u8, _old_size: usize, _align: usize, new_size: usize) -> *mut u8 {
-    unsafe {
-        bindings::krealloc(
-            ptr as *const core::ffi::c_void,
-            new_size,
-            bindings::GFP_KERNEL,
-        ) as *mut u8
-    }
+    // SAFETY: `ptr` only ever comes from `krealloc` via `__rust_alloc`
+    unsafe { bindings::krealloc(ptr.cast(), new_size, bindings::GFP_KERNEL).cast() }
 }
 
 #[no_mangle]
 fn __rust_alloc_zeroed(size: usize, _align: usize) -> *mut u8 {
+    // SAFETY: `krealloc` can handle zero-sized allocations with `__GFP_ZERO`
     unsafe {
         bindings::krealloc(
             core::ptr::null(),
             size,
             bindings::GFP_KERNEL | bindings::__GFP_ZERO,
-        ) as *mut u8
+        )
+        .cast()
     }
 }

rust/kernel/error.rs

@@ -166,9 +166,11 @@ impl fmt::Debug for Error {
         match self.name() {
             // Print out number if no name can be found.
             None => f.debug_tuple("Error").field(&-self.0).finish(),
-            // SAFETY: These strings are ASCII-only.
             Some(name) => f
-                .debug_tuple(unsafe { core::str::from_utf8_unchecked(name) })
+                .debug_tuple(
+                    // SAFETY: These strings are ASCII-only.
+                    unsafe { core::str::from_utf8_unchecked(name) },
+                )
                 .finish(),
         }
     }
@@ -281,18 +283,19 @@ pub(crate) fn from_err_ptr<T>(ptr: *mut T) -> Result<*mut T> {
     // SAFETY: The FFI function does not deref the pointer.
     if unsafe { bindings::IS_ERR(const_ptr) } {
         // SAFETY: The FFI function does not deref the pointer.
-        let err = unsafe { bindings::PTR_ERR(const_ptr) };
+        let errno = unsafe { bindings::PTR_ERR(const_ptr) };
         // CAST: If `IS_ERR()` returns `true`,
         // then `PTR_ERR()` is guaranteed to return a
         // negative value greater-or-equal to `-bindings::MAX_ERRNO`,
         // which always fits in an `i16`, as per the invariant above.
         // And an `i16` always fits in an `i32`. So casting `err` to
         // an `i32` can never overflow, and is always valid.
         //
+        #[allow(clippy::unnecessary_cast)]
         // SAFETY: `IS_ERR()` ensures `err` is a
         // negative value greater-or-equal to `-bindings::MAX_ERRNO`.
-        #[allow(clippy::unnecessary_cast)]
-        return Err(unsafe { Error::from_errno_unchecked(err as core::ffi::c_int) });
+        let err = unsafe { Error::from_errno_unchecked(errno as core::ffi::c_int) };
+        return Err(err);
     }
     Ok(ptr)
 }

rust/kernel/init.rs

@@ -621,6 +621,7 @@ macro_rules! try_pin_init {
         // no possibility of returning without `unsafe`.
         struct __InitOk;
         // Get the pin data from the supplied type.
+        // SAFETY: TODO
         let data = unsafe {
             use $crate::init::__internal::HasPinData;
             $t$(::<$($generics),*>)?::__pin_data()
@@ -664,6 +665,7 @@ macro_rules! try_pin_init {
         let init = move |slot| -> ::core::result::Result<(), $err> {
             init(slot).map(|__InitOk| ())
         };
+        // SAFETY: TODO
         let init = unsafe { $crate::init::pin_init_from_closure::<_, $err>(init) };
         init
     }};
@@ -735,8 +737,9 @@ macro_rules! try_pin_init {
         @acc($($acc:tt)*),
     ) => {
         // Endpoint, nothing more to munch, create the initializer.
-        // Since we are in the `if false` branch, this will never get executed. We abuse `slot` to
-        // get the correct type inference here:
+
+        // SAFETY: Since we are in the `if false` branch, this will never get executed. We abuse
+        // `slot` to get the correct type inference here:
         unsafe {
             ::core::ptr::write($slot, $t {
                 $($acc)*
@@ -777,6 +780,7 @@ macro_rules! try_pin_init {
     (forget_guards:
         @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
     ) => {
+        // SAFETY: forgetting the guard is intentional
         unsafe { $crate::init::__internal::DropGuard::forget($field) };
 
         $crate::try_pin_init!(forget_guards:
@@ -786,6 +790,7 @@ macro_rules! try_pin_init {
     (forget_guards:
         @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
     ) => {
+        // SAFETY: forgetting the guard is intentional
         unsafe { $crate::init::__internal::DropGuard::forget($field) };
 
         $crate::try_pin_init!(forget_guards:
@@ -891,6 +896,7 @@ macro_rules! try_init {
         // no possibility of returning without `unsafe`.
         struct __InitOk;
         // Get the init data from the supplied type.
+        // SAFETY: `HasInitData` is a marker trait which we are allowed to use in this context
         let data = unsafe {
             use $crate::init::__internal::HasInitData;
             $t$(::<$($generics),*>)?::__init_data()
@@ -933,6 +939,7 @@ macro_rules! try_init {
         let init = move |slot| -> ::core::result::Result<(), $err> {
             init(slot).map(|__InitOk| ())
         };
+        // SAFETY: TODO
         let init = unsafe { $crate::init::init_from_closure::<_, $err>(init) };
         init
     }};
@@ -999,8 +1006,8 @@ macro_rules! try_init {
         @acc($($acc:tt)*),
     ) => {
         // Endpoint, nothing more to munch, create the initializer.
-        // Since we are in the `if false` branch, this will never get executed. We abuse `slot` to
-        // get the correct type inference here:
+        // SAFETY: Since we are in the `if false` branch, this will never get
+        // executed. We abuse `slot` to get the correct type inference here:
         unsafe {
             ::core::ptr::write($slot, $t {
                 $($acc)*
@@ -1041,6 +1048,7 @@ macro_rules! try_init {
     (forget_guards:
         @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
     ) => {
+        // SAFETY: forgetting the guard is intentional
         unsafe { $crate::init::__internal::DropGuard::forget($field) };
 
         $crate::try_init!(forget_guards:
@@ -1050,6 +1058,7 @@ macro_rules! try_init {
     (forget_guards:
         @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
     ) => {
+        // SAFETY: forgetting the guard is intentional
         unsafe { $crate::init::__internal::DropGuard::forget($field) };
 
         $crate::try_init!(forget_guards:
@@ -1197,6 +1206,7 @@ pub fn uninit<T, E>() -> impl Init<MaybeUninit<T>, E> {
 // SAFETY: Every type can be initialized by-value.
 unsafe impl<T, E> Init<T, E> for T {
     unsafe fn __init(self, slot: *mut T) -> Result<(), E> {
+        // SAFETY: `slot` is valid per the invariants of `__init`
         unsafe { slot.write(self) };
         Ok(())
     }
@@ -1371,8 +1381,12 @@ pub fn zeroed<T: Zeroable>() -> impl Init<T> {
     }
 }
 
+/// # Safety
+///
+/// This can only be used on types that meet `Zeroable`'s guidelines
 macro_rules! impl_zeroable {
     ($($({$($generics:tt)*})? $t:ty, )*) => {
+        // SAFETY: upheld by documented use
         $(unsafe impl$($($generics)*)? Zeroable for $t {})*
     };
 }

rust/kernel/init/__internal.rs

@@ -7,6 +7,8 @@
 //! - `macros/pin_data.rs`
 //! - `macros/pinned_drop.rs`
 
+#![allow(clippy::undocumented_unsafe_blocks)]
+
 use super::*;
 
 /// See the [nomicon] for what subtyping is. See also [this table].

rust/kernel/init/macros.rs

@@ -841,6 +841,7 @@ macro_rules! __pin_data {
                 }
             }
 
+            // SAFETY: `__ThePinData` has the correct projections
             unsafe impl<$($impl_generics)*>
                 $crate::init::__internal::PinData for __ThePinData<$($ty_generics)*>
             where $($whr)*
@@ -965,6 +966,7 @@ macro_rules! __pin_data {
                     slot: *mut $p_type,
                     init: impl $crate::init::PinInit<$p_type, E>,
                 ) -> ::core::result::Result<(), E> {
+                    // SAFETY: `slot` is valid per this function's contract
                     unsafe { $crate::init::PinInit::__pinned_init(init, slot) }
                 }
             )*
@@ -974,6 +976,7 @@ macro_rules! __pin_data {
                     slot: *mut $type,
                     init: impl $crate::init::Init<$type, E>,
                 ) -> ::core::result::Result<(), E> {
+                    // SAFETY: `slot` is valid per this function's contract
                     unsafe { $crate::init::Init::__init(init, slot) }
                 }
             )*

rust/kernel/print.rs

@@ -9,14 +9,20 @@
 use core::{
     ffi::{c_char, c_void},
     fmt,
+    ptr::addr_of,
 };
 
 use crate::str::RawFormatter;
 
 #[cfg(CONFIG_PRINTK)]
 use crate::bindings;
 
-// Called from `vsprintf` with format specifier `%pA`.
+/// Called from `vsprintf` with format specifier `%pA`.
+///
+/// # Safety
+///
+/// - Memory within `buf..end` must be valid for write
+/// - `ptr` must point to a valid instance of `core::fmt::Arguments`
 #[no_mangle]
 unsafe extern "C" fn rust_fmt_argument(
     buf: *mut c_char,
@@ -26,7 +32,8 @@ unsafe extern "C" fn rust_fmt_argument(
     use fmt::Write;
     // SAFETY: The C contract guarantees that `buf` is valid if it's less than `end`.
     let mut w = unsafe { RawFormatter::from_ptrs(buf.cast(), end.cast()) };
-    let _ = w.write_fmt(unsafe { *(ptr as *const fmt::Arguments<'_>) });
+    // SAFETY: Upheld by this function's safety contract
+    let _ = w.write_fmt(unsafe { *(ptr.cast::<fmt::Arguments<'_>>()) });
     w.pos().cast()
 }
 
@@ -105,13 +112,14 @@ pub unsafe fn call_printk(
     module_name: &[u8],
     args: fmt::Arguments<'_>,
 ) {
-    // `_printk` does not seem to fail in any path.
     #[cfg(CONFIG_PRINTK)]
+    // `_printk` does not seem to fail in any path.
+    // SAFETY: This function's invariants make this call safe.
     unsafe {
         bindings::_printk(
-            format_string.as_ptr() as _,
+            format_string.as_ptr().cast(),
             module_name.as_ptr(),
-            &args as *const _ as *const c_void,
+            addr_of!(args).cast::<c_void>(),
         );
     }
 }
@@ -124,14 +132,13 @@ pub unsafe fn call_printk(
 #[doc(hidden)]
 #[cfg_attr(not(CONFIG_PRINTK), allow(unused_variables))]
 pub fn call_printk_cont(args: fmt::Arguments<'_>) {
-    // `_printk` does not seem to fail in any path.
-    //
-    // SAFETY: The format string is fixed.
     #[cfg(CONFIG_PRINTK)]
+    // `_printk` does not seem to fail in any path.
+    // SAFETY: `CONT` and `args` meet `_printk`'s safety contract
     unsafe {
         bindings::_printk(
-            format_strings::CONT.as_ptr() as _,
-            &args as *const _ as *const c_void,
+            format_strings::CONT.as_ptr().cast(),
+            addr_of!(args).cast::<c_void>(),
         );
     }
 }

rust/kernel/str.rs

@@ -72,10 +72,10 @@ impl CStr {
     /// Returns the length of this string with `NUL`.
     #[inline]
     pub const fn len_with_nul(&self) -> usize {
-        // SAFETY: This is one of the invariant of `CStr`.
-        // We add a `unreachable_unchecked` here to hint the optimizer that
-        // the value returned from this function is non-zero.
         if self.0.is_empty() {
+            // SAFETY: This is one of the invariant of `CStr`.
+            // We add a `unreachable_unchecked` here to hint the optimizer that
+            // the value returned from this function is non-zero.
             unsafe { core::hint::unreachable_unchecked() };
         }
         self.0.len()
@@ -198,6 +198,7 @@ impl CStr {
     /// ```
     #[inline]
     pub unsafe fn as_str_unchecked(&self) -> &str {
+        // SAFETY: invariant is upheld by this function's safety contract
         unsafe { core::str::from_utf8_unchecked(self.as_bytes()) }
     }
 

rust/kernel/sync/condvar.rs

@@ -81,8 +81,8 @@ pub struct CondVar {
     _pin: PhantomPinned,
 }
 
-// SAFETY: `CondVar` only uses a `struct wait_queue_head`, which is safe to use on any thread.
 #[allow(clippy::non_send_fields_in_send_ty)]
+// SAFETY: `CondVar` only uses a `struct wait_queue_head`, which is safe to use on any thread.
 unsafe impl Send for CondVar {}
 
 // SAFETY: `CondVar` only uses a `struct wait_queue_head`, which is safe to use on multiple threads

rust/kernel/sync/lock.rs

@@ -144,9 +144,9 @@ impl<T: ?Sized, B: Backend> Guard<'_, T, B> {
         // SAFETY: The caller owns the lock, so it is safe to unlock it.
         unsafe { B::unlock(self.lock.state.get(), &self.state) };
 
-        // SAFETY: The lock was just unlocked above and is being relocked now.
-        let _relock =
-            ScopeGuard::new(|| unsafe { B::relock(self.lock.state.get(), &mut self.state) });
+        let _relock = ScopeGuard::new(||
+            // SAFETY: The lock was just unlocked above and is being relocked now.
+            unsafe { B::relock(self.lock.state.get(), &mut self.state) });
 
         cb();
     }

rust/rust_common_flags

@@ -67,8 +67,12 @@
 -Dclippy::same_functions_in_if_condition
 -Dclippy::stable_sort_primitive
 -Dclippy::too_many_lines
+-Dclippy::undocumented_unsafe_blocks
 -Dclippy::unicode_not_nfc
 -Dclippy::unnecessary_join
+# FIXME: enable this at the next version bump. Disabled because of false
+# positive in macros: https://github.com/rust-lang/rust-clippy/issues/10084
+# -Dclippy::unnecessary_safety_comment
 -Dclippy::unnested_or_patterns
 
 # Clippy lints from restriction

rust/uapi/lib.rs

@@ -14,6 +14,7 @@
 #![cfg_attr(test, allow(unsafe_op_in_unsafe_fn))]
 #![allow(
     clippy::all,
+    clippy::undocumented_unsafe_blocks,
     missing_docs,
     non_camel_case_types,
     non_upper_case_globals,