Skip to content

Commit 8a073d5

Browse files
committed
rust: kernel: add missing safety comments
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 <[email protected]>
1 parent 540942e commit 8a073d5

File tree

7 files changed

+75
-1
lines changed

7 files changed

+75
-1
lines changed

rust/kernel/module_param.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,11 +71,17 @@ pub trait ModuleParam: core::fmt::Display + core::marker::Sized {
7171
let arg = if val.is_null() {
7272
None
7373
} else {
74+
// SAFETY: `val` is a valid CStr by the safety requirements of
75+
// this function
7476
Some(unsafe { CStr::from_char_ptr(val).as_bytes() })
7577
};
7678
match Self::try_from_param_arg(arg) {
7779
Some(new_value) => {
80+
// SAFETY: `param.arg` is a valid `Self` by the safety
81+
// requirements of this function
7882
let old_value = unsafe { (*param).__bindgen_anon_1.arg as *mut Self };
83+
// SAFETY: `old_value` is a valid `Self` by the safety
84+
// requirements of this function
7985
let _ = unsafe { core::ptr::replace(old_value, new_value) };
8086
0
8187
}
@@ -95,8 +101,12 @@ pub trait ModuleParam: core::fmt::Display + core::marker::Sized {
95101
buf: *mut crate::c_types::c_char,
96102
param: *const crate::bindings::kernel_param,
97103
) -> crate::c_types::c_int {
104+
// SAFETY: `buf` is valid and writable for atleast `kernel::PAGE_SIZE`
105+
// by the safety requirements of this function
98106
let slice = unsafe { core::slice::from_raw_parts_mut(buf as *mut u8, crate::PAGE_SIZE) };
99107
let mut buf = crate::buffer::Buffer::new(slice);
108+
// SAFETY: `param.arg` is a valid `Self` by the safety requirements of
109+
// this function
100110
match unsafe { write!(buf, "{}\0", *((*param).__bindgen_anon_1.arg as *mut Self)) } {
101111
Err(_) => crate::error::Error::EINVAL.to_kernel_errno(),
102112
Ok(()) => buf.bytes_written() as crate::c_types::c_int,
@@ -111,6 +121,8 @@ pub trait ModuleParam: core::fmt::Display + core::marker::Sized {
111121
///
112122
/// The `arg` field of `param` must be an instance of `Self`.
113123
unsafe extern "C" fn free(arg: *mut crate::c_types::c_void) {
124+
// SAFETY: `arg` is a valid `Self` by the safety requirements of this
125+
// function
114126
unsafe { core::ptr::drop_in_place(arg as *mut Self) };
115127
}
116128
}

rust/kernel/pages.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,15 @@ impl<const ORDER: u32> Pages<ORDER> {
9393
}
9494

9595
let mapping = self.kmap(0).ok_or(Error::EINVAL)?;
96+
// SAFETY:
97+
// (*mut u8).add(): `dest.add(offset)` is guarenteed to not overflow an
98+
// `isize` due to the check above, assuming `PAGE_SIZE`
99+
// is less than or equal to `isize::MAX`
100+
// ptr::copy(): The safety contract of this function guarentees that
101+
// the `dest` buffer is valid for atleast `len` bytes
102+
// The invariant guarentee of this struct, and the checks
103+
// guarentees that the 'mapping.ptr' buffer is valid fir
104+
// atleast `len` bytes
96105
unsafe { ptr::copy((mapping.ptr as *mut u8).add(offset), dest, len) };
97106
Ok(())
98107
}
@@ -113,6 +122,15 @@ impl<const ORDER: u32> Pages<ORDER> {
113122
}
114123

115124
let mapping = self.kmap(0).ok_or(Error::EINVAL)?;
125+
// SAFETY:
126+
// (*mut u8).add(): `dest.add(offset)` is guarenteed to not overflow an
127+
// `isize` due to the check above, assuming `PAGE_SIZE`
128+
// is less than or equal to `isize::MAX`
129+
// ptr::copy(): The safety contract of this function guarentees that
130+
// the `dest` buffer is valid for atleast `len` bytes
131+
// The invariant guarentee of this struct, and the checks
132+
// guarentees that the 'mapping.ptr' buffer is valid fir
133+
// atleast `len` bytes
116134
unsafe { ptr::copy(src, (mapping.ptr as *mut u8).add(offset), len) };
117135
Ok(())
118136
}

rust/kernel/print.rs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,15 @@ use core::fmt;
1212
use crate::bindings;
1313
use crate::c_types::{c_char, c_void};
1414

15-
// Called from `vsprintf` with format specifier `%pA`.
15+
/// Called from `vsprintf` with format specifier `%pA`.
16+
///
17+
/// # Safety
18+
///
19+
/// The caller must ensure the following guarentees are met
20+
///
21+
/// * `buf` is non-null and is valid until `end`
22+
/// * `end` is non-null and is greater than `buf`
23+
/// * `ptr` is non-null and points to a valid [`fmt::Arguments`]
1624
#[no_mangle]
1725
unsafe fn rust_fmt_argument(buf: *mut c_char, end: *mut c_char, ptr: *const c_void) -> *mut c_char {
1826
use fmt::Write;
@@ -53,6 +61,7 @@ unsafe fn rust_fmt_argument(buf: *mut c_char, end: *mut c_char, ptr: *const c_vo
5361
buf: buf as _,
5462
end: end as _,
5563
};
64+
// SAFETY: `ptr` is valid by the safety requirements of this function
5665
let _ = w.write_fmt(unsafe { *(ptr as *const fmt::Arguments<'_>) });
5766
w.buf as _
5867
}

rust/kernel/sync/condvar.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,13 @@ impl NeedsLockClass for CondVar {
132132
key: *mut bindings::lock_class_key,
133133
_: *mut bindings::lock_class_key,
134134
) {
135+
// SAFETY:
136+
// `wait_list` is a valid uninitialised C `wait_list`
137+
// `name` is a valid NUL-Terminated `CStr`
138+
// `key` The safety requirements of `init` require that it is
139+
// valid and will remain valid for the lifetime for the lock
140+
// `_` The safety requirements of `init` require that it is
141+
// valid and will remain valid for the lifetime for the lock
135142
unsafe { bindings::__init_waitqueue_head(self.wait_list.get(), name.as_char_ptr(), key) };
136143
}
137144
}

rust/kernel/sync/mutex.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,11 @@ impl<T> CreatableLock for Mutex<T> {
8282
name: &'static CStr,
8383
key: *mut bindings::lock_class_key,
8484
) {
85+
// SAFETY:
86+
// `mutex` is a valid uninitialised C `mutex`
87+
// `name` is a valid NUL-Terminated `CStr`
88+
// `key` The safety requirements of `init_lock` require that it is
89+
// valid and will remain valid for the lifetime for the lock
8590
unsafe { bindings::__mutex_init(self.mutex.get(), name.as_char_ptr(), key) };
8691
}
8792
}

rust/kernel/sync/spinlock.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,11 @@ impl<T> CreatableLock for SpinLock<T> {
141141
name: &'static CStr,
142142
key: *mut bindings::lock_class_key,
143143
) {
144+
// SAFETY:
145+
// `spin_lock` is a valid uninitialised C `spin_lock`
146+
// `name` is a valid `NUL`-Terminated `CStr`
147+
// `key` The safety requirements of `init_lock` require that it is
148+
// valid and will remain valid for the lifetime for the lock
144149
unsafe { bindings::__spin_lock_init(self.spin_lock.get(), name.as_char_ptr(), key) };
145150
}
146151
}

rust/kernel/sysctl.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,14 @@ pub struct Sysctl<T: SysctlStorage> {
9494
// `T: Sync`. Any new methods must adhere to this requirement.
9595
unsafe impl<T: SysctlStorage> Sync for Sysctl<T> {}
9696

97+
/// # Safety
98+
///
99+
/// The caller must ensure the following guarentees are met
100+
///
101+
/// * `ctl` points to a valid `ctl_table`
102+
/// * `buffer` is non-null and is valid for atleast 'len' bytes
103+
/// * `len` points to a valid `usize`
104+
/// * `ppos` points to a valid `loff_t`
97105
unsafe extern "C" fn proc_handler<T: SysctlStorage>(
98106
ctl: *mut bindings::ctl_table,
99107
write: c_types::c_int,
@@ -103,12 +111,19 @@ unsafe extern "C" fn proc_handler<T: SysctlStorage>(
103111
) -> c_types::c_int {
104112
// If we are reading from some offset other than the beginning of the file,
105113
// return an empty read to signal EOF.
114+
//
115+
// SAFETY: `ppos` is valid by the safety requirements of this function
106116
if unsafe { *ppos } != 0 && write == 0 {
117+
// SAFETY: `len` is valid by the safety requirements of this function
107118
unsafe { *len = 0 };
108119
return 0;
109120
}
110121

122+
// SAFETY:
123+
// The safety contract for `UserSlicePtr::new()` is upheld by the safety
124+
// requirements of this function
111125
let data = unsafe { UserSlicePtr::new(buffer, *len) };
126+
// SAFETY: `ctl` is valid by the safety requirements of this function
112127
let storage = unsafe { &*((*ctl).data as *const T) };
113128
let (bytes_processed, result) = if write != 0 {
114129
let data = match data.read_all() {
@@ -120,7 +135,10 @@ unsafe extern "C" fn proc_handler<T: SysctlStorage>(
120135
let mut writer = data.writer();
121136
storage.read_value(&mut writer)
122137
};
138+
// SAFETY: `len` is valid by the safety requirements of this function
123139
unsafe { *len = bytes_processed };
140+
// SAFETY:
141+
// `ppos` and `len` are valid by the safety requirements of this function
124142
unsafe { *ppos += *len as bindings::loff_t };
125143
match result {
126144
Ok(()) => 0,

0 commit comments

Comments
 (0)