| From bafde4c2cc2b23d6e7500ff50ac75605e1f2c5d1 Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Wed, 7 Jan 2026 08:28:46 +0000 |
| Subject: rust: task: restrict Task::group_leader() to current |
| MIME-Version: 1.0 |
| Content-Type: text/plain; charset=UTF-8 |
| Content-Transfer-Encoding: 8bit |
| |
| From: Alice Ryhl <aliceryhl@google.com> |
| |
| [ Upstream commit 105ddfb2d2b3acec7a7d9695463df48733d91e6c ] |
| |
| 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 <aliceryhl@google.com> |
| Fixes: 313c4281bc9d ("rust: add basic `Task`") |
| Reported-by: Oleg Nesterov <oleg@redhat.com> |
| Closes: https://lore.kernel.org/all/aTLnV-5jlgfk1aRK@redhat.com/ |
| Reviewed-by: Boqun Feng <boqun.feng@gmail.com> |
| Reviewed-by: Gary Guo <gary@garyguo.net> |
| Cc: Andreas Hindborg <a.hindborg@kernel.org> |
| Cc: Benno Lossin <lossin@kernel.org> |
| Cc: "Björn Roy Baron" <bjorn3_gh@protonmail.com> |
| Cc: Björn Roy Baron <bjorn3_gh@protonmail.com> |
| Cc: Christian Brauner <brauner@kernel.org> |
| Cc: Danilo Krummrich <dakr@kernel.org> |
| Cc: FUJITA Tomonori <fujita.tomonori@gmail.com> |
| Cc: Miguel Ojeda <ojeda@kernel.org> |
| Cc: Panagiotis Foliadis <pfoliadis@posteo.net> |
| Cc: Shankari Anand <shankari.ak0208@gmail.com> |
| Cc: Trevor Gross <tmgross@umich.edu> |
| Signed-off-by: Andrew Morton <akpm@linux-foundation.org> |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| 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 @@ pub fn as_ptr(&self) -> *mut bindings::task_struct { |
| 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 @@ pub fn active_pid_ns(&self) -> Option<&PidNamespace> { |
| // `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.51.0 |
| |