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 <aliceryhl@google.com>
Link: https://lore.kernel.org/r/20241015-task-safety-cmnts-v1-1-46ee92c82768@google.com
Signed-off-by: Christian Brauner <brauner@kernel.org>
This commit is contained in:
Alice Ryhl 2024-10-15 14:02:12 +00:00 committed by Christian Brauner
parent 22018a5a54
commit fe95f58320
No known key found for this signature in database
GPG Key ID: 91C61BC06578DCA2

View File

@ -145,11 +145,17 @@ impl Task {
} }
} }
/// Returns a raw pointer to the task.
#[inline]
pub fn as_ptr(&self) -> *mut bindings::task_struct {
self.0.get()
}
/// Returns the group leader of the given task. /// Returns the group leader of the given task.
pub fn group_leader(&self) -> &Task { pub fn group_leader(&self) -> &Task {
// SAFETY: By the type invariant, we know that `self.0` is a valid task. Valid tasks always // SAFETY: The group leader of a task never changes after initialization, so reading this
// have a valid `group_leader`. // field is not a data race.
let ptr = unsafe { *ptr::addr_of!((*self.0.get()).group_leader) }; let ptr = unsafe { *ptr::addr_of!((*self.as_ptr()).group_leader) };
// SAFETY: The lifetime of the returned task reference is tied to the lifetime of `self`, // SAFETY: The lifetime of the returned task reference is tied to the lifetime of `self`,
// and given that a task has a reference to its group leader, we know it must be valid for // and given that a task has a reference to its group leader, we know it must be valid for
@ -159,42 +165,41 @@ impl Task {
/// Returns the PID of the given task. /// Returns the PID of the given task.
pub fn pid(&self) -> Pid { pub fn pid(&self) -> Pid {
// SAFETY: By the type invariant, we know that `self.0` is a valid task. Valid tasks always // SAFETY: The pid of a task never changes after initialization, so reading this field is
// have a valid pid. // not a data race.
unsafe { *ptr::addr_of!((*self.0.get()).pid) } unsafe { *ptr::addr_of!((*self.as_ptr()).pid) }
} }
/// Returns the UID of the given task. /// Returns the UID of the given task.
pub fn uid(&self) -> Kuid { pub fn uid(&self) -> Kuid {
// SAFETY: By the type invariant, we know that `self.0` is valid. // SAFETY: It's always safe to call `task_uid` on a valid task.
Kuid::from_raw(unsafe { bindings::task_uid(self.0.get()) }) Kuid::from_raw(unsafe { bindings::task_uid(self.as_ptr()) })
} }
/// Returns the effective UID of the given task. /// Returns the effective UID of the given task.
pub fn euid(&self) -> Kuid { pub fn euid(&self) -> Kuid {
// SAFETY: By the type invariant, we know that `self.0` is valid. // SAFETY: It's always safe to call `task_euid` on a valid task.
Kuid::from_raw(unsafe { bindings::task_euid(self.0.get()) }) Kuid::from_raw(unsafe { bindings::task_euid(self.as_ptr()) })
} }
/// Determines whether the given task has pending signals. /// Determines whether the given task has pending signals.
pub fn signal_pending(&self) -> bool { pub fn signal_pending(&self) -> bool {
// SAFETY: By the type invariant, we know that `self.0` is valid. // SAFETY: It's always safe to call `signal_pending` on a valid task.
unsafe { bindings::signal_pending(self.0.get()) != 0 } unsafe { bindings::signal_pending(self.as_ptr()) != 0 }
} }
/// Returns the given task's pid in the current pid namespace. /// Returns the given task's pid in the current pid namespace.
pub fn pid_in_current_ns(&self) -> Pid { pub fn pid_in_current_ns(&self) -> Pid {
// SAFETY: We know that `self.0.get()` is valid by the type invariant, and passing a null // SAFETY: It's valid to pass a null pointer as the namespace (defaults to current
// pointer as the namespace is correct for using the current namespace. // namespace). The task pointer is also valid.
unsafe { bindings::task_tgid_nr_ns(self.0.get(), ptr::null_mut()) } unsafe { bindings::task_tgid_nr_ns(self.as_ptr(), ptr::null_mut()) }
} }
/// Wakes up the task. /// Wakes up the task.
pub fn wake_up(&self) { pub fn wake_up(&self) {
// SAFETY: By the type invariant, we know that `self.0.get()` is non-null and valid. // SAFETY: It's always safe to call `signal_pending` on a valid task, even if the task
// And `wake_up_process` is safe to be called for any valid task, even if the task is
// running. // running.
unsafe { bindings::wake_up_process(self.0.get()) }; unsafe { bindings::wake_up_process(self.as_ptr()) };
} }
} }
@ -202,7 +207,7 @@ impl Task {
unsafe impl crate::types::AlwaysRefCounted for Task { unsafe impl crate::types::AlwaysRefCounted for Task {
fn inc_ref(&self) { fn inc_ref(&self) {
// SAFETY: The existence of a shared reference means that the refcount is nonzero. // SAFETY: The existence of a shared reference means that the refcount is nonzero.
unsafe { bindings::get_task_struct(self.0.get()) }; unsafe { bindings::get_task_struct(self.as_ptr()) };
} }
unsafe fn dec_ref(obj: ptr::NonNull<Self>) { unsafe fn dec_ref(obj: ptr::NonNull<Self>) {