Add some missing // SAFETY comments

Signed-off-by: Léo Lanteri Thauvin <[email protected]>

Author: LeSeulArtichaut

rust/kernel/file_operations.rs

@@ -100,8 +100,7 @@ unsafe extern "C" fn open_callback<A: FileOpenAdapter, T: FileOpener<A::Arg>>(
         // be a valid non-null pointer.
         let ptr = T::open(unsafe { &*arg })?.into_pointer();
         // SAFETY: `ptr` was previously returned by `T::open`. The returned
-        // value should be a boxed value and should live the length of the
-        // given file.
+        // value should be a boxed value and should outlive the given file.
         unsafe { (*file).private_data = ptr as *mut c_types::c_void };
         Ok(0)
     }
@@ -114,7 +113,7 @@ unsafe extern "C" fn read_callback<T: FileOperations>(
     offset: *mut bindings::loff_t,
 ) -> c_types::c_ssize_t {
     from_kernel_result! {
-        let mut data = unsafe { UserSlicePtr::new(buf as *mut c_types::c_void, len).writer() };
+        let mut data = unsafe { UserSlicePtr::new(buf as *mut c_types::c_void, len) }.writer();
         // SAFETY: `private_data` was initialised by `open_callback` with a value returned by
         // `T::Wrapper::into_pointer`. `T::Wrapper::from_pointer` is only called by the `release`
         // callback, which the C API guarantees that will be called only when all references to

rust/kernel/miscdev.rs

@@ -60,7 +60,7 @@ impl<T: Sync> Registration<T> {
         name: &'static CStr,
         minor: Option<i32>,
     ) -> Result {
-        // SAFETY: We must ensure that we never move out of `this`.
+        // SAFETY: we don't move out of `this`.
         let this = unsafe { self.get_unchecked_mut() };
         if this.registered {
             // Already registered.
@@ -72,6 +72,7 @@ impl<T: Sync> Registration<T> {
         this.mdev.name = name.as_char_ptr();
         this.mdev.minor = minor.unwrap_or(bindings::MISC_DYNAMIC_MINOR as i32);
 
+        // SAFETY: FFI call.
         let ret = unsafe { bindings::misc_register(&mut this.mdev) };
         if ret < 0 {
             return Err(Error::from_kernel_errno(ret));
@@ -87,6 +88,10 @@ impl<T: Sync> FileOpenAdapter for Registration<T> {
     unsafe fn convert(_inode: *mut bindings::inode, file: *mut bindings::file) -> *const Self::Arg {
         // SAFETY: the caller must guarantee that `file` is valid.
         let reg = crate::container_of!(unsafe { (*file).private_data }, Self, mdev);
+        // SAFETY: before calling the `open` callback, `file.private_data` is initialized
+        // by the `miscdev` subsystem to point to the miscellaneous device, which is guaranteed
+        // by the caller to have been registered through `Registration`, so `file.private_data`
+        // is the `mdev` field of a `Registration` object.
         unsafe { &(*reg).context }
     }
 }

rust/kernel/pages.rs

@@ -82,7 +82,7 @@ impl<const ORDER: u32> Pages<ORDER> {
     ///
     /// # Safety
     ///
-    /// Callers must ensure that the destination buffer is valid for the given length.
+    /// Callers must ensure that the destination buffer is valid for writes for the given length.
     /// Additionally, if the raw buffer is intended to be recast, they must ensure that the data
     /// can be safely cast; [`crate::io_buffer::ReadableFromBytes`] has more details about it.
     pub unsafe fn read(&self, dest: *mut u8, offset: usize, len: usize) -> Result {
@@ -93,15 +93,23 @@ impl<const ORDER: u32> Pages<ORDER> {
         }
 
         let mapping = self.kmap(0).ok_or(Error::EINVAL)?;
-        unsafe { ptr::copy((mapping.ptr as *mut u8).add(offset), dest, len) };
+        // SAFETY: we just checked that `offset + len <= PAGE_SIZE`, so `mapping.ptr` and
+        // `mapping.ptr + offset` are in the same allocated object, the page we're
+        // reading from, with no overflow. Also, `offset <= PAGE_LEN < isize::MAX`
+        // so `offset` can't overflow an `isize`.
+        let src = unsafe { (mapping.ptr as *mut u8).add(offset) };
+        // SAFETY: `src` is valid for reads from the type invariants, and `dest` is
+        // guaranteed by the caller to be valid for writes. Because we're copying `u8`s
+        // which have an alignment of 1, `src` and `dest` are always properly aligned.
+        unsafe { ptr::copy(src, dest, len) };
         Ok(())
     }
 
     /// Maps the pages and writes into them from the given bufer.
     ///
     /// # Safety
     ///
-    /// Callers must ensure that the buffer is valid for the given length. Additionally, if the
+    /// Callers must ensure that the source buffer is valid for reads for the given length. Additionally, if the
     /// page is (or will be) mapped by userspace, they must ensure that no kernel data is leaked
     /// through padding if it was cast from another type; [`crate::io_buffer::WritableToBytes`] has
     /// more details about it.
@@ -113,7 +121,15 @@ impl<const ORDER: u32> Pages<ORDER> {
         }
 
         let mapping = self.kmap(0).ok_or(Error::EINVAL)?;
-        unsafe { ptr::copy(src, (mapping.ptr as *mut u8).add(offset), len) };
+        // SAFETY: we just checked that `offset + len <= PAGE_SIZE`, so `mapping.ptr` and
+        // `mapping.ptr + offset` are in the same allocated object, the page we're
+        // reading from, with no overflow. Also, `offset <= PAGE_LEN < isize::MAX`
+        // so `offset` can't overflow an `isize`.
+        let dest = unsafe { (mapping.ptr as *mut u8).add(offset) };
+        // SAFETY: `src` is guaranteed by the caller to be valid for reads, and `dest` is
+        //valid for writes from the type invariants. Because we're copying `u8`s
+        // which have an alignment of 1, `src` and `dest` are always properly aligned.
+        unsafe { ptr::copy(src, dest, len) };
         Ok(())
     }
 

rust/kernel/print.rs

@@ -33,7 +33,7 @@ unsafe fn rust_fmt_argument(buf: *mut c_char, end: *mut c_char, ptr: *const c_vo
             // `buf` goes past `end`.
             let len_to_copy = cmp::min(buf_new, self.end).saturating_sub(self.buf);
 
-            // SAFETY: In any case, `buf` is non-null and properly aligned.
+            // SAFETY: The caller guarantees `buf` is non-null and properly aligned.
             // If `len_to_copy` is non-zero, then we know `buf` has not past
             // `end` yet and so is valid.
             unsafe {
@@ -53,6 +53,7 @@ unsafe fn rust_fmt_argument(buf: *mut c_char, end: *mut c_char, ptr: *const c_vo
         buf: buf as _,
         end: end as _,
     };
+    // SAFETY: the caller must guarantee that `ptr` is valid.
     let _ = w.write_fmt(unsafe { *(ptr as *const fmt::Arguments<'_>) });
     w.buf as _
 }
@@ -132,6 +133,7 @@ pub unsafe fn call_printk(
     args: fmt::Arguments<'_>,
 ) {
     // `_printk` does not seem to fail in any path.
+    // SAFETY: FFI call.
     unsafe {
         bindings::_printk(
             format_string.as_ptr() as _,

rust/kernel/sync/condvar.rs

@@ -127,6 +127,7 @@ impl CondVar {
 
 impl NeedsLockClass for CondVar {
     unsafe fn init(self: Pin<&mut Self>, name: &'static CStr, key: *mut bindings::lock_class_key) {
+        // SAFETY: FFI call.
         unsafe { bindings::__init_waitqueue_head(self.wait_list.get(), name.as_char_ptr(), key) };
     }
 }