(Note: This is not a bugfix, it just cleans up an incorrect assumption.)

Task::group_leader() assumes that task::group_leader remains constant until
the task refcount drops to zero.

However, Linux has a special quirk where, when execve() is called by a
thread other than the thread group leader (the main thread), de_thread()
swaps the current thread's identity with the thread group leader's,
making the current thread the new thread group leader.
This means task::group_leader can't be assumed to be immutable for
non-current tasks.

For reference, you can see that accessing the ->group_leader of some random
task requires extra caution in the prlimit64() syscall, which grabs the
tasklist_lock and has a comment explaining that this is done to prevent
races with de_thread().

Signed-off-by: Jann Horn <jannh@google.com>
---
 rust/kernel/task.rs | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/rust/kernel/task.rs b/rust/kernel/task.rs
index 49fad6de0674..91ad88cdfd3b 100644
--- a/rust/kernel/task.rs
+++ b/rust/kernel/task.rs
@@ -103,7 +103,7 @@ macro_rules! current {
 unsafe impl Send for Task {}
 
 // SAFETY: It's OK to access `Task` through shared references from other threads because we're
-// either accessing properties that don't change (e.g., `pid`, `group_leader`) or that are properly
+// either accessing properties that don't change or that are properly
 // synchronised by C code (e.g., `signal_pending`).
 unsafe impl Sync for Task {}
 
@@ -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 the current task never changes in syscall
+        // context (except in the implementation of execve()).
+        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() }
+    }
 }
 
 // SAFETY: The type invariants guarantee that `Task` is always refcounted.

-- 
2.53.0.273.g2a3d683680-goog

On Thu, Feb 12, 2026 at 11:12 PM Jann Horn <jannh@google.com> wrote:
>
> (Note: This is not a bugfix, it just cleans up an incorrect assumption.)
>
> Task::group_leader() assumes that task::group_leader remains constant until
> the task refcount drops to zero.
>
> However, Linux has a special quirk where, when execve() is called by a
> thread other than the thread group leader (the main thread), de_thread()
> swaps the current thread's identity with the thread group leader's,
> making the current thread the new thread group leader.
> This means task::group_leader can't be assumed to be immutable for
> non-current tasks.
>
> For reference, you can see that accessing the ->group_leader of some random
> task requires extra caution in the prlimit64() syscall, which grabs the
> tasklist_lock and has a comment explaining that this is done to prevent
> races with de_thread().
>
> Signed-off-by: Jann Horn <jannh@google.com>

This has already been fixed:
https://lore.kernel.org/all/20260107-task-group-leader-v2-1-8fbf816f2a2f@google.com/

On Fri, Feb 13, 2026 at 8:52 AM Alice Ryhl <aliceryhl@google.com> wrote:
> On Thu, Feb 12, 2026 at 11:12 PM Jann Horn <jannh@google.com> wrote:
> >
> > (Note: This is not a bugfix, it just cleans up an incorrect assumption.)
> >
> > Task::group_leader() assumes that task::group_leader remains constant until
> > the task refcount drops to zero.
> >
> > However, Linux has a special quirk where, when execve() is called by a
> > thread other than the thread group leader (the main thread), de_thread()
> > swaps the current thread's identity with the thread group leader's,
> > making the current thread the new thread group leader.
> > This means task::group_leader can't be assumed to be immutable for
> > non-current tasks.
> >
> > For reference, you can see that accessing the ->group_leader of some random
> > task requires extra caution in the prlimit64() syscall, which grabs the
> > tasklist_lock and has a comment explaining that this is done to prevent
> > races with de_thread().
> >
> > Signed-off-by: Jann Horn <jannh@google.com>
>
> This has already been fixed:
> https://lore.kernel.org/all/20260107-task-group-leader-v2-1-8fbf816f2a2f@google.com/

Ah, and I didn't notice it because it was in the mm tree while I was
looking at the rust tree. Sorry for the duplicate patch.

On Fri, Feb 13, 2026 at 2:38 PM Jann Horn <jannh@google.com> wrote:
>
> On Fri, Feb 13, 2026 at 8:52 AM Alice Ryhl <aliceryhl@google.com> wrote:
> > On Thu, Feb 12, 2026 at 11:12 PM Jann Horn <jannh@google.com> wrote:
> > >
> > > (Note: This is not a bugfix, it just cleans up an incorrect assumption.)
> > >
> > > Task::group_leader() assumes that task::group_leader remains constant until
> > > the task refcount drops to zero.
> > >
> > > However, Linux has a special quirk where, when execve() is called by a
> > > thread other than the thread group leader (the main thread), de_thread()
> > > swaps the current thread's identity with the thread group leader's,
> > > making the current thread the new thread group leader.
> > > This means task::group_leader can't be assumed to be immutable for
> > > non-current tasks.
> > >
> > > For reference, you can see that accessing the ->group_leader of some random
> > > task requires extra caution in the prlimit64() syscall, which grabs the
> > > tasklist_lock and has a comment explaining that this is done to prevent
> > > races with de_thread().
> > >
> > > Signed-off-by: Jann Horn <jannh@google.com>
> >
> > This has already been fixed:
> > https://lore.kernel.org/all/20260107-task-group-leader-v2-1-8fbf816f2a2f@google.com/
>
> Ah, and I didn't notice it because it was in the mm tree while I was
> looking at the rust tree. Sorry for the duplicate patch.

No worries! I did not get around to fixing the pid() issue, which was
also pointed out previously, so thanks for the patch on that one!

Alice