Skip to content

Commit

Permalink
rust: kernel: add missing safety comments
Browse files Browse the repository at this point in the history
Contributes to Rust-for-Linux#351

Added `// SAFETY` docs to `fn proc_handler`
`rust/kernel/sysctl.rs:97:1`
`rust/kernel/sysctl.rs:113:5`
`rust/kernel/sysctl.rs:115:9`
`rust/kernel/sysctl.rs:120:5`
`rust/kernel/sysctl.rs:124:5`
`rust/kernel/sysctl.rs:136:5`
`rust/kernel/sysctl.rs:138:5`

Added `// SAFETY` docs to `fn init_lock`
`rust/kernel/sync/mutex.rs:85:9`

Added `// SAFETY` docs to `fn init_lock`
`rust/kernel/sync/spinlock.rs:144:9`

Added `// SAFETY` docs to `fn init`
`rust/kernel/sync/condvar.rs:135:9`

Added `// SAFETY` docs to `fn read`
`rust/kernel/pages.rs:96:9`

Added `// SAFETY` docs to `fn write`
`rust/kernel/pages.rs:125:9`

Added `// SAFETY` docs to `fn rust_fmt_argument`
`rust/kernel/print.rs:17:1`
`rust/kernel/print.rs:62:5`

Added `// SAFETY` docs to `fn set_param`
`rust/kernel/module_param.rs:63:5`
`rust/kernel/module_param.rs:74:13`
`rust/kernel/module_param.rs:80:17`
`rust/kernel/module_param.rs:83:17`

Added `// SAFETY` docs to `fn get_param`
`rust/kernel/module_param.rs:104:9`
`rust/kernel/module_param.rs:108:9`

Added `// SAFETY` docs to `fn free`
`rust/kernel/module_param.rs:124:9`

Signed-off-by: Dee-Jay Logozzo <git@djl.id.au>
  • Loading branch information
DjLogozzo committed Nov 29, 2021
1 parent 540942e commit d8666ba
Show file tree
Hide file tree
Showing 7 changed files with 75 additions and 2 deletions.
18 changes: 17 additions & 1 deletion rust/kernel/module_param.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,19 +63,29 @@ pub trait ModuleParam: core::fmt::Display + core::marker::Sized {
/// # Safety
///
/// If `val` is non-null then it must point to a valid null-terminated
/// string. The `arg` field of `param` must be an instance of `Self`.
/// string that will not be mutated for the lifetime of `param.arg`. The
/// `arg` field of `param` must be an instance of `Self`.
unsafe extern "C" fn set_param(
val: *const crate::c_types::c_char,
param: *const crate::bindings::kernel_param,
) -> crate::c_types::c_int {
let arg = if val.is_null() {
None
} else {
// SAFETY: `val` is non-null due to the check above, and is a valid
// null-terminated string by the safety requirements of this
// function. `val` will not be mutated for the lifetime of
// `param.arg` due to the safety requirements of this function.
Some(unsafe { CStr::from_char_ptr(val).as_bytes() })
};
match Self::try_from_param_arg(arg) {
Some(new_value) => {
// SAFETY: `param.arg` is a valid `Self` by the safety
// requirements of this function.
let old_value = unsafe { (*param).__bindgen_anon_1.arg as *mut Self };
// SAFETY: `old_value` is a valid `Self` by the safety
// requirements of this function. `new_value` is a valid `Self`
// due to the check above.
let _ = unsafe { core::ptr::replace(old_value, new_value) };
0
}
Expand All @@ -95,8 +105,12 @@ pub trait ModuleParam: core::fmt::Display + core::marker::Sized {
buf: *mut crate::c_types::c_char,
param: *const crate::bindings::kernel_param,
) -> crate::c_types::c_int {
// SAFETY: `buf` is valid and writable for atleast `kernel::PAGE_SIZE`
// by the safety requirements of this function.
let slice = unsafe { core::slice::from_raw_parts_mut(buf as *mut u8, crate::PAGE_SIZE) };
let mut buf = crate::buffer::Buffer::new(slice);
// SAFETY: `param.arg` is a valid `Self` by the safety requirements of
// this function.
match unsafe { write!(buf, "{}\0", *((*param).__bindgen_anon_1.arg as *mut Self)) } {
Err(_) => crate::error::Error::EINVAL.to_kernel_errno(),
Ok(()) => buf.bytes_written() as crate::c_types::c_int,
Expand All @@ -111,6 +125,8 @@ pub trait ModuleParam: core::fmt::Display + core::marker::Sized {
///
/// The `arg` field of `param` must be an instance of `Self`.
unsafe extern "C" fn free(arg: *mut crate::c_types::c_void) {
// SAFETY: `arg` is a valid `Self` by the safety requirements of this
// function.
unsafe { core::ptr::drop_in_place(arg as *mut Self) };
}
}
Expand Down
16 changes: 16 additions & 0 deletions rust/kernel/pages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,14 @@ impl<const ORDER: u32> Pages<ORDER> {
}

let mapping = self.kmap(0).ok_or(Error::EINVAL)?;
// SAFETY:
// (*mut u8).add(): `dest.add(offset)` is guarenteed to not overflow an
// `isize` due to the check above, assuming `PAGE_SIZE` is less than or
// equal to `isize::MAX`.
// ptr::copy(): The safety contract of this function guarentees that
// the `dest` buffer is valid for atleast `len` bytes. The invariant
// guarentee of this struct, and the checks guarentees that the
// 'mapping.ptr' buffer is valid for atleast `len` bytes.
unsafe { ptr::copy((mapping.ptr as *mut u8).add(offset), dest, len) };
Ok(())
}
Expand All @@ -113,6 +121,14 @@ impl<const ORDER: u32> Pages<ORDER> {
}

let mapping = self.kmap(0).ok_or(Error::EINVAL)?;
// SAFETY:
// (*mut u8).add(): `dest.add(offset)` is guarenteed to not overflow an
// `isize` due to the check above, assuming `PAGE_SIZE` is less than or
// equal to `isize::MAX`.
// ptr::copy(): The safety contract of this function guarentees that
// the `dest` buffer is valid for at least `len` bytes. The invariant
// guarentee of this struct, and the checks guarentees that the
// 'mapping.ptr' buffer is valid for at least `len` bytes.
unsafe { ptr::copy(src, (mapping.ptr as *mut u8).add(offset), len) };
Ok(())
}
Expand Down
11 changes: 10 additions & 1 deletion rust/kernel/print.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,15 @@ 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 caller must ensure the following guarentees are met.
///
/// * `buf` is non-null and is valid until `end`.
/// * `end` is non-null and is greater than `buf`.
/// * `ptr` is non-null and points to a valid [`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;
Expand Down Expand Up @@ -53,6 +61,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: `ptr` is valid by the safety requirements of this function.
let _ = w.write_fmt(unsafe { *(ptr as *const fmt::Arguments<'_>) });
w.buf as _
}
Expand Down
5 changes: 5 additions & 0 deletions rust/kernel/sync/condvar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,11 @@ impl NeedsLockClass for CondVar {
key: *mut bindings::lock_class_key,
_: *mut bindings::lock_class_key,
) {
// SAFETY:
// `wait_list` is a valid uninitialised C `wait_list`.
// `name` is a valid NUL-Terminated `CStr`.
// `key` The safety requirements of `init` require that it is
// valid and will remain valid for the lifetime for the lock.
unsafe { bindings::__init_waitqueue_head(self.wait_list.get(), name.as_char_ptr(), key) };
}
}
5 changes: 5 additions & 0 deletions rust/kernel/sync/mutex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,11 @@ impl<T> CreatableLock for Mutex<T> {
name: &'static CStr,
key: *mut bindings::lock_class_key,
) {
// SAFETY:
// `mutex` is a valid uninitialised C `mutex`.
// `name` is a valid NUL-Terminated `CStr`.
// `key` The safety requirements of `init_lock` require that it is
// valid and will remain valid for the lifetime for the lock.
unsafe { bindings::__mutex_init(self.mutex.get(), name.as_char_ptr(), key) };
}
}
Expand Down
5 changes: 5 additions & 0 deletions rust/kernel/sync/spinlock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,11 @@ impl<T> CreatableLock for SpinLock<T> {
name: &'static CStr,
key: *mut bindings::lock_class_key,
) {
// SAFETY:
// `spin_lock` is a valid uninitialised C `spin_lock`.
// `name` is a valid `NUL`-Terminated `CStr`.
// `key` The safety requirements of `init_lock` require that it is
// valid and will remain valid for the lifetime for the lock.
unsafe { bindings::__spin_lock_init(self.spin_lock.get(), name.as_char_ptr(), key) };
}
}
Expand Down
17 changes: 17 additions & 0 deletions rust/kernel/sysctl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,14 @@ pub struct Sysctl<T: SysctlStorage> {
// `T: Sync`. Any new methods must adhere to this requirement.
unsafe impl<T: SysctlStorage> Sync for Sysctl<T> {}

/// # Safety
///
/// The caller must ensure the following guarentees are met
///
/// * `ctl` points to a valid `ctl_table`.
/// * `buffer` is non-null and is valid for at least 'len' bytes.
/// * `len` points to a valid `usize`.
/// * `ppos` points to a valid `loff_t`.
unsafe extern "C" fn proc_handler<T: SysctlStorage>(
ctl: *mut bindings::ctl_table,
write: c_types::c_int,
Expand All @@ -103,12 +111,18 @@ unsafe extern "C" fn proc_handler<T: SysctlStorage>(
) -> c_types::c_int {
// If we are reading from some offset other than the beginning of the file,
// return an empty read to signal EOF.
//
// SAFETY: `ppos` is valid by the safety requirements of this function.
if unsafe { *ppos } != 0 && write == 0 {
// SAFETY: `len` is valid by the safety requirements of this function.
unsafe { *len = 0 };
return 0;
}

// SAFETY: The safety contract for `UserSlicePtr::new()` is upheld by the
// safety requirements of this function.
let data = unsafe { UserSlicePtr::new(buffer, *len) };
// SAFETY: `ctl` is valid by the safety requirements of this function.
let storage = unsafe { &*((*ctl).data as *const T) };
let (bytes_processed, result) = if write != 0 {
let data = match data.read_all() {
Expand All @@ -120,7 +134,10 @@ unsafe extern "C" fn proc_handler<T: SysctlStorage>(
let mut writer = data.writer();
storage.read_value(&mut writer)
};
// SAFETY: `len` is valid by the safety requirements of this function.
unsafe { *len = bytes_processed };
// SAFETY: `ppos` and `len` are valid by the safety requirements of this
// function.
unsafe { *ppos += *len as bindings::loff_t };
match result {
Ok(()) => 0,
Expand Down

0 comments on commit d8666ba

Please sign in to comment.