Skip to content

Commit fe95f58

Browse files
Darksonnbrauner
authored andcommitted
rust: task: adjust safety comments in Task methods
The `Task` struct has several safety comments that aren't so great. For example, the reason that it's okay to read the `pid` is that the field is immutable, so there is no data race, which is not what the safety comment says. Thus, improve the safety comments. Also add an `as_ptr` helper. This makes it easier to read the various accessors on Task, as `self.0` may be confusing syntax for new Rust users. Signed-off-by: Alice Ryhl <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Christian Brauner <[email protected]>
1 parent 22018a5 commit fe95f58

File tree

1 file changed

+24
-19
lines changed

1 file changed

+24
-19
lines changed

rust/kernel/task.rs

Lines changed: 24 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -145,11 +145,17 @@ impl Task {
145145
}
146146
}
147147

148+
/// Returns a raw pointer to the task.
149+
#[inline]
150+
pub fn as_ptr(&self) -> *mut bindings::task_struct {
151+
self.0.get()
152+
}
153+
148154
/// Returns the group leader of the given task.
149155
pub fn group_leader(&self) -> &Task {
150-
// SAFETY: By the type invariant, we know that `self.0` is a valid task. Valid tasks always
151-
// have a valid `group_leader`.
152-
let ptr = unsafe { *ptr::addr_of!((*self.0.get()).group_leader) };
156+
// SAFETY: The group leader of a task never changes after initialization, so reading this
157+
// field is not a data race.
158+
let ptr = unsafe { *ptr::addr_of!((*self.as_ptr()).group_leader) };
153159

154160
// SAFETY: The lifetime of the returned task reference is tied to the lifetime of `self`,
155161
// and given that a task has a reference to its group leader, we know it must be valid for
@@ -159,50 +165,49 @@ impl Task {
159165

160166
/// Returns the PID of the given task.
161167
pub fn pid(&self) -> Pid {
162-
// SAFETY: By the type invariant, we know that `self.0` is a valid task. Valid tasks always
163-
// have a valid pid.
164-
unsafe { *ptr::addr_of!((*self.0.get()).pid) }
168+
// SAFETY: The pid of a task never changes after initialization, so reading this field is
169+
// not a data race.
170+
unsafe { *ptr::addr_of!((*self.as_ptr()).pid) }
165171
}
166172

167173
/// Returns the UID of the given task.
168174
pub fn uid(&self) -> Kuid {
169-
// SAFETY: By the type invariant, we know that `self.0` is valid.
170-
Kuid::from_raw(unsafe { bindings::task_uid(self.0.get()) })
175+
// SAFETY: It's always safe to call `task_uid` on a valid task.
176+
Kuid::from_raw(unsafe { bindings::task_uid(self.as_ptr()) })
171177
}
172178

173179
/// Returns the effective UID of the given task.
174180
pub fn euid(&self) -> Kuid {
175-
// SAFETY: By the type invariant, we know that `self.0` is valid.
176-
Kuid::from_raw(unsafe { bindings::task_euid(self.0.get()) })
181+
// SAFETY: It's always safe to call `task_euid` on a valid task.
182+
Kuid::from_raw(unsafe { bindings::task_euid(self.as_ptr()) })
177183
}
178184

179185
/// Determines whether the given task has pending signals.
180186
pub fn signal_pending(&self) -> bool {
181-
// SAFETY: By the type invariant, we know that `self.0` is valid.
182-
unsafe { bindings::signal_pending(self.0.get()) != 0 }
187+
// SAFETY: It's always safe to call `signal_pending` on a valid task.
188+
unsafe { bindings::signal_pending(self.as_ptr()) != 0 }
183189
}
184190

185191
/// Returns the given task's pid in the current pid namespace.
186192
pub fn pid_in_current_ns(&self) -> Pid {
187-
// SAFETY: We know that `self.0.get()` is valid by the type invariant, and passing a null
188-
// pointer as the namespace is correct for using the current namespace.
189-
unsafe { bindings::task_tgid_nr_ns(self.0.get(), ptr::null_mut()) }
193+
// SAFETY: It's valid to pass a null pointer as the namespace (defaults to current
194+
// namespace). The task pointer is also valid.
195+
unsafe { bindings::task_tgid_nr_ns(self.as_ptr(), ptr::null_mut()) }
190196
}
191197

192198
/// Wakes up the task.
193199
pub fn wake_up(&self) {
194-
// SAFETY: By the type invariant, we know that `self.0.get()` is non-null and valid.
195-
// And `wake_up_process` is safe to be called for any valid task, even if the task is
200+
// SAFETY: It's always safe to call `signal_pending` on a valid task, even if the task
196201
// running.
197-
unsafe { bindings::wake_up_process(self.0.get()) };
202+
unsafe { bindings::wake_up_process(self.as_ptr()) };
198203
}
199204
}
200205

201206
// SAFETY: The type invariants guarantee that `Task` is always refcounted.
202207
unsafe impl crate::types::AlwaysRefCounted for Task {
203208
fn inc_ref(&self) {
204209
// SAFETY: The existence of a shared reference means that the refcount is nonzero.
205-
unsafe { bindings::get_task_struct(self.0.get()) };
210+
unsafe { bindings::get_task_struct(self.as_ptr()) };
206211
}
207212

208213
unsafe fn dec_ref(obj: ptr::NonNull<Self>) {

0 commit comments

Comments
 (0)