From 105ddfb2d2b3acec7a7d9695463df48733d91e6c Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Wed, 7 Jan 2026 08:28:46 +0000 Subject: [PATCH] rust: task: restrict Task::group_leader() to current MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit The Task::group_leader() method currently allows you to access the group_leader() of any task, for example one you hold a refcount to. But this is not safe in general since the group leader could change when a task exits. See for example commit a15f37a40145c ("kernel/sys.c: fix the racy usage of task_lock(tsk->group_leader) in sys_prlimit64() paths"). All existing users of Task::group_leader() call this method on current, which is guaranteed running, so there's not an actual issue in Rust code today. But to prevent code in the future from making this mistake, restrict Task::group_leader() so that it can only be called on current. There are some other cases where accessing task->group_leader is okay. For example it can be safe if you hold tasklist_lock or rcu_read_lock(). However, only supporting current->group_leader is sufficient for all in-tree Rust users of group_leader right now. Safe Rust functionality for accessing it under rcu or while holding tasklist_lock may be added in the future if required by any future Rust module. This patch is a bugfix in that it prevents users of this API from writing incorrect code. It doesn't change behavior of correct code. Link: https://lkml.kernel.org/r/20260107-task-group-leader-v2-1-8fbf816f2a2f@google.com Signed-off-by: Alice Ryhl Fixes: 313c4281bc9d ("rust: add basic `Task`") Reported-by: Oleg Nesterov Closes: https://lore.kernel.org/all/aTLnV-5jlgfk1aRK@redhat.com/ Reviewed-by: Boqun Feng Reviewed-by: Gary Guo Cc: Andreas Hindborg Cc: Benno Lossin Cc: "Björn Roy Baron" Cc: Björn Roy Baron Cc: Christian Brauner Cc: Danilo Krummrich Cc: FUJITA Tomonori Cc: Miguel Ojeda Cc: Panagiotis Foliadis Cc: Shankari Anand Cc: Trevor Gross Signed-off-by: Andrew Morton --- rust/kernel/task.rs | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/rust/kernel/task.rs b/rust/kernel/task.rs index 49fad6de06740..cc907fb531bce 100644 --- a/rust/kernel/task.rs +++ b/rust/kernel/task.rs @@ -204,18 +204,6 @@ impl Task { self.0.get() } - /// Returns the group leader of the given task. - pub fn group_leader(&self) -> &Task { - // SAFETY: The group leader of a task never changes after initialization, so reading this - // field is not a data race. - 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`, - // and given that a task has a reference to its group leader, we know it must be valid for - // the lifetime of the returned task reference. - unsafe { &*ptr.cast() } - } - /// Returns the PID of the given task. pub fn pid(&self) -> Pid { // SAFETY: The pid of a task never changes after initialization, so reading this field is @@ -345,6 +333,18 @@ impl CurrentTask { // `release_task()` call. Some(unsafe { PidNamespace::from_ptr(active_ns) }) } + + /// Returns the group leader of the current task. + pub fn group_leader(&self) -> &Task { + // SAFETY: The group leader of a task never changes while the task is running, and `self` + // is the current task, which is guaranteed running. + let ptr = unsafe { (*self.as_ptr()).group_leader }; + + // SAFETY: `current->group_leader` stays valid for at least the duration in which `current` + // is running, and the signature of this function ensures that the returned `&Task` can + // only be used while `current` is still valid, thus still running. + unsafe { &*ptr.cast() } + } } // SAFETY: The type invariants guarantee that `Task` is always refcounted. -- 2.47.3