Add some missing // SAFETY comments

rust/kernel/miscdev.rs

@@ -147,7 +147,7 @@ impl<T: FileOperations> Registration<T> {
         open_data: T::OpenData,
         opts: &Options<'_>,
     ) -> 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.
@@ -170,6 +170,9 @@ impl<T: FileOperations> Registration<T> {
         this.registered = true;
         this.open_data.write(open_data);
 
+        // SAFETY: the `fops`, `name` and `minor` fields of `this.mdev` have
+        // been properly initialized above (the other fields are zeroed and will be
+        // initialized by `misc_register`).
         let ret = unsafe { bindings::misc_register(&mut this.mdev) };
         if ret < 0 {
             // INVARIANT: `registered` is set back to `false` and the `open_data` is destructued.

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_SIZE < 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_SIZE < 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

@@ -12,7 +12,12 @@ use core::fmt;
 use crate::bindings;
 use crate::c_types::{c_char, c_void};
 
-// Called from `vsprintf` with format specifier `%pA`.
+/// Called from `vsprintf` with format specifier `%pA`.
+///
+/// # Safety
+///
+/// The region between `buf` and `end` must be valid for writes.
+/// `ptr` must point to a valid instance of `fmt::Arguments`.
 #[no_mangle]
 unsafe fn rust_fmt_argument(buf: *mut c_char, end: *mut c_char, ptr: *const c_void) -> *mut c_char {
     use fmt::Write;
@@ -33,7 +38,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 +58,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 +138,7 @@ pub unsafe fn call_printk(
     args: fmt::Arguments<'_>,
 ) {
     // `_printk` does not seem to fail in any path.
+    // SAFETY: this is safe by the safety contract.
     unsafe {
         bindings::_printk(
             format_string.as_ptr() as _,