diff --git a/rust/kernel/miscdev.rs b/rust/kernel/miscdev.rs index cf0cabe95dd1e1..bc6b5903ab09b2 100644 --- a/rust/kernel/miscdev.rs +++ b/rust/kernel/miscdev.rs @@ -147,7 +147,7 @@ impl Registration { 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 Registration { 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. diff --git a/rust/kernel/pages.rs b/rust/kernel/pages.rs index de8358629fdd95..f5847a81fc5b8d 100644 --- a/rust/kernel/pages.rs +++ b/rust/kernel/pages.rs @@ -82,7 +82,7 @@ impl Pages { /// /// # 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,7 +93,15 @@ impl Pages { } 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(()) } @@ -101,7 +109,7 @@ impl Pages { /// /// # 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 Pages { } 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(()) } diff --git a/rust/kernel/print.rs b/rust/kernel/print.rs index 328d893f87aafe..1eb53cb3d621d3 100644 --- a/rust/kernel/print.rs +++ b/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 _,