rust: wrong SAFETY comments in group_leader() and pid() + questions

DISCUSSION Oleg Nesterov 3 participants 11 messages Dec 5, 2025 – Dec 11, 2025 Rust

Share & tools

Oleg Nesterov

author

2025-12-05 14:08 UTC #0 lore raw

From rust/kernel/task.rs:

    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
        // not a data race.
        unsafe { *ptr::addr_of!((*self.as_ptr()).pid) }
    }

The comments look wrong. Unless same_thread_group(current, task) == T, task->group_leader and/or task->pid can change if a non-leader task's sub-thread execs. This also means that in general it is not safe to dereference group_leader, for example this C code is not safe:

	rcu_read_lock();
	task = find_task_by_vpid(vpid);
	if (task)
		get_task_struct(task);
	rcu_read_unlock();

	if (task)
		pid = task->group_leader->pid; // BUG! ->group_leader can be already freed

Now the questions. Sorry! I don't know rust.

1. Can I simply remove these misleading comments? Or SAFETY comment is mandatory?

2. I am working on the patch(es) which move ->group_leader from task_struct to signal_struct, so the 1st change adds the new trivial helper in preparation:

	struct task_struct *task_group_leader(struct task_struct *task)
	{
		return task->group_leader; // will be updated
	}

Now, how can I change group_leader() to use it? I guess I need to add

	struct task_struct *rust_helper_task_group_leader(struct task_struct *task)
	{
		return task_group_leader(task);
	}

into rust/helpers/task.c, but will something like

	pub fn group_leader(&self) -> &Task {
		unsafe { bindings::task_group_leader(self.as_ptr()) }
	}

    work? I'm afraid it won't ;)

Oleg.

Alice Ryhl

reviewer

2025-12-05 14:19 UTC #1 lore raw

Thanks a lot for your email!

+Christian Brauner

Quoted 34 lines · Oleg Nesterov

From rust/kernel/task.rs:

    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
        // not a data race.
        unsafe { *ptr::addr_of!((*self.as_ptr()).pid) }
    }

	rcu_read_lock();
	task = find_task_by_vpid(vpid);
	if (task)
		get_task_struct(task);
	rcu_read_unlock();

	if (task)
		pid = task->group_leader->pid; // BUG! ->group_leader can be already freed

Quoted 3 lines

Now the questions. Sorry! I don't know rust.
1. Can I simply remove these misleading comments? Or SAFETY comment is mandatory?

If the safety comments are wrong, then the code is probably wrong too!

What is the correct way to read the pid or group_leader fields of a struct task_struct?

Quoted 8 lines

2. I am working on the patch(es) which move ->group_leader from task_struct to signal_struct, so the 1st change adds the new trivial helper in preparation:
	struct task_struct *task_group_leader(struct task_struct *task)
	{
		return task->group_leader; // will be updated
	}

Quoted 14 lines

   Now, how can I change group_leader() to use it? I guess I need to add

	struct task_struct *rust_helper_task_group_leader(struct task_struct *task)
	{
		return task_group_leader(task);
	}

   into rust/helpers/task.c, but will something like

	pub fn group_leader(&self) -> &Task {
		unsafe { bindings::task_group_leader(self.as_ptr()) }
	}

    work? I'm afraid it won't ;)

That looks like it should work. The rust_helper_ function is only required if task_group_leader is marked static inline. Otherwise bindings:: will pick up the function straight from the C header. (As long as the relevant header is included in bindings_helper.h)

That still does raise the question of how you correctly call this function if the group leader could be freed at any time.

Alice

Alice Ryhl

reviewer

2025-12-05 14:29 UTC #2 lore raw

Quoted 26 lines · Alice Ryhl

Thanks a lot for your email!
+Christian Brauner
Quoted 21 lines · Oleg Nesterov
2. I am working on the patch(es) which move ->group_leader from task_struct to signal_struct, so the 1st change adds the new trivial helper in preparation:
	struct task_struct *task_group_leader(struct task_struct *task)
	{
		return task->group_leader; // will be updated
	}
   Now, how can I change group_leader() to use it? I guess I need to add

	struct task_struct *rust_helper_task_group_leader(struct task_struct *task)
	{
		return task_group_leader(task);
	}

   into rust/helpers/task.c, but will something like

	pub fn group_leader(&self) -> &Task {
		unsafe { bindings::task_group_leader(self.as_ptr()) }
	}

Quoted 6 lines

Quoted 1 line
work? I'm afraid it won't ;)
That looks like it should work. The rust_helper_ function is only required if task_group_leader is marked static inline. Otherwise bindings:: will pick up the function straight from the C header. (As long as the relevant header is included in bindings_helper.h)

Ah, actually you need to convert from *mut bindings::task_struct to &Task. And you need a safety comment. E.g.:

pub fn group_leader(&self) -> &Task {
    // 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 { &*bindings::task_group_leader(self.as_ptr()).cast::<Task>() }
}

Alice

Oleg Nesterov

author

2025-12-05 17:22 UTC #3 lore raw

Quoted 8 lines · Alice Ryhl

pub fn group_leader(&self) -> &Task {
    // 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 { &*bindings::task_group_leader(self.as_ptr()).cast::<Task>() }
}

Thanks again Alice, but the comment still looks misleading to me... OK, quite possibly this is because I don't understand what does the "lifetime of the returned task reference" actually mean in the rust code. Does it mean "lifetime of task_struct" of "lifetime of the process/thread" ?

Let me provide the artificial example. Suppose we have something like

	struct task_struct *TASK = NULL;

	void stupid_example(void)
	{
		TASK = get_task_struct(current);

		do_exit(0);
	}

and a non-leader task calls stupid_example().

After that the global TASK pointer is still valid, it is safe to dereference it, task_struct itself can't go away.

But! Right after that TASK->group_leader can point to nowhere (to the freed memory) if another thread does do_group_exit() or sys_execve().

So. Perhaps the the comment should say something like

SAFETY: The lifetime of the returned task reference is tied to the lifetime of the THREAD represented by self

Oleg.

Alice Ryhl

reviewer

2025-12-05 18:17 UTC #4 lore raw

Quoted 10 lines · Oleg Nesterov

Quoted 8 lines · Alice Ryhl

pub fn group_leader(&self) -> &Task {
    // 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 { &*bindings::task_group_leader(self.as_ptr()).cast::<Task>() }
}

Quoted 4 lines

Thanks again Alice, but the comment still looks misleading to me... OK, quite possibly this is because I don't understand what does the "lifetime of the returned task reference" actually mean in the rust code. Does it mean "lifetime of task_struct" of "lifetime of the process/thread" ?

To start with, it's likely that this comment is not the right choice for this function, given our discussion. Most likely group_leader() needs to be moved to impl CurrentTask {} and the safety comment needs to explain why being the current task ensures that the returned &Task lives for long enough. I just took the safety comment from the code we have today.

To explain "lifetime of the returned task reference" it may be useful to show you the long form of this function signature:

fn group_leader<'a>(&'a self) -> &'a Task;

By "lifetime of the returned task reference" I am referring to 'a, which is some region of code in the caller. For example, 'a might be:

* The region of code until I put the task I called group_leader() on.
* The region of code between a rcu_read_lock() / rcu_read_unlock() pair.
* The region of code until I release a mutex.
* The region of code inside a scope or function.

So for example, with the code as it is today, this example:

let leader = task.group_leader();
drop(task); // put refcount
do_something(leader); // COMPILE ERROR

Rust will see that task may no longer be valid after the second line, so 'a becomes the region of code until drop(task), and the use of leader after the end of 'a is a compilation error.

Now, the above example is not really valid. It works today, but once we fix group_leader() to require the task to be the current task, then the example might look like this:

	let leader;
	{
		let current = current!();
		leader = current.leader();
		do_something(leader); // OK
	}
	do_something(leader); // COMPILE ERROR

And ok, this example isn't great because the last line is actually OK. However, the way that I designed current!() to work is that it gives you an &'a CurrentTask where 'a is the duration of the current scope. So any use of leader after the end of the current scope is a compile error.

Quoted 11 lines

Let me provide the artificial example. Suppose we have something like
	struct task_struct *TASK = NULL;

	void stupid_example(void)
	{
		TASK = get_task_struct(current);

		do_exit(0);
	}

Quoted 4 lines

and a non-leader task calls stupid_example().
After that the global TASK pointer is still valid, it is safe to dereference it, task_struct itself can't go away.

This would not compile in Rust. The &CurrentTask obtained using current!() can't leave the scope it is created in.

Quoted 9 lines

But! Right after that TASK->group_leader can point to nowhere (to the freed memory) if another thread does do_group_exit() or sys_execve().
So. Perhaps the the comment should say something like
SAFETY: The lifetime of the returned task reference is tied to the lifetime of the THREAD represented by self
?

I think the safety comment should make some reference to the fact that we know it was obtained from the current task. After all, that's why we know it's ok.

impl CurrentTask {
    fn group_leader(&self) -> &Task {
        // SAFETY: This is the current task, so the task must be alive.
        // Therefore the group leader cannot change, and thus it will
        // stay valid as long as self is the current task.
        unsafe { &*bindings::task_group_leader(self.as_ptr()).cast::<Task>() }
    }
}

Alice

Oleg Nesterov

author

2025-12-08 14:30 UTC #5 lore raw

Alice,

Thanks again for your explanations. Not that I fully understand them, though ;)

Quoted 4 lines · Alice Ryhl

To start with, it's likely that this comment is not the right choice for this function, given our discussion. Most likely group_leader() needs to be moved to impl CurrentTask {}

I obviously can't comment this proposal,

Quoted 3 lines

and the safety comment needs
to explain why being the current task ensures that the returned &Task lives for long enough.

This is simple. task->group_leader can't change or go away until this task exits or execs. The "current" task can't exit/exec.

(This also covers the race with mt-exec from current's subthread,
the execing thread will kill all the threads and wait until they all pass release_task(). Only then it will change ->group_leader).

Quoted 8 lines

impl CurrentTask {
    fn group_leader(&self) -> &Task {
        // SAFETY: This is the current task, so the task must be alive.
        // Therefore the group leader cannot change, and thus it will
        // stay valid as long as self is the current task.
        unsafe { &*bindings::task_group_leader(self.as_ptr()).cast::<Task>() }
    }
}

Yes, the comment looks good to me.

But we don't have the task_group_leader() helper yet, so far I only sent the trivial initial preparations, see
https://lore.kernel.org/all/[email protected]/

So if you are going to move Task::group_leader to the CurrentTask block, please use .group_leader directly, like the current code does.

Oleg.

Alice Ryhl

reviewer

2025-12-11 01:20 UTC #6 lore raw

Quoted 35 lines · Oleg Nesterov

Alice,
Thanks again for your explanations. Not that I fully understand them, though ;)
Quoted 4 lines · Alice Ryhl
To start with, it's likely that this comment is not the right choice for this function, given our discussion. Most likely group_leader() needs to be moved to impl CurrentTask {}
I obviously can't comment this proposal,
Quoted 3 lines
and the safety comment needs
to explain why being the current task ensures that the returned &Task lives for long enough.
This is simple. task->group_leader can't change or go away until this task exits or execs. The "current" task can't exit/exec.
(This also covers the race with mt-exec from current's subthread,
the execing thread will kill all the threads and wait until they all pass release_task(). Only then it will change ->group_leader).
Quoted 8 lines
impl CurrentTask {
    fn group_leader(&self) -> &Task {
        // SAFETY: This is the current task, so the task must be alive.
        // Therefore the group leader cannot change, and thus it will
        // stay valid as long as self is the current task.
        unsafe { &*bindings::task_group_leader(self.as_ptr()).cast::<Task>() }
    }
}
Yes, the comment looks good to me.

Quoted 7 lines

But we don't have the task_group_leader() helper yet, so far I only sent the trivial initial preparations, see
https://lore.kernel.org/all/[email protected]/
So if you are going to move Task::group_leader to the CurrentTask block, please use .group_leader directly, like the current code does.

Yeah I'll be moving it soon (after conferences). I'll add a Reported-by tag for you if that's okay :)

Alice

Gary Guo

reviewer

2025-12-08 15:43 UTC #7 lore raw

In reply to Alice Ryhl

Quoted 16 lines · On Fri, 5 Dec 2025 18:17:09 +0000 Alice Ryhl <[email protected]> wrote:

Quoted 10 lines · Oleg Nesterov
Quoted 8 lines · Alice Ryhl
pub fn group_leader(&self) -> &Task {
    // 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 { &*bindings::task_group_leader(self.as_ptr()).cast::<Task>() }
}  
Quoted 4 lines
Thanks again Alice, but the comment still looks misleading to me... OK, quite possibly this is because I don't understand what does the "lifetime of the returned task reference" actually mean in the rust code. Does it mean "lifetime of task_struct" of "lifetime of the process/thread" ?

Quoted 6 lines

To start with, it's likely that this comment is not the right choice for this function, given our discussion. Most likely group_leader() needs to be moved to impl CurrentTask {} and the safety comment needs to explain why being the current task ensures that the returned &Task lives for long enough. I just took the safety comment from the code we have today.

This indeed sounds like the right approach to take.

If Task::pid or Task::group_leader just gives the pid or group leader at the time of invocation and doesn't have any stability guarantee, then the user of the functions will likely be misusing these functions.

It's better to just have them on CurrentTask for now. When an user arises that need to retrieve them for another task, then we can come back and think about a proper solution taking the scenario into account.

Best,
Gary

Alice Ryhl

reviewer

2025-12-11 01:16 UTC #8 lore raw

Quoted 28 lines · Gary Guo

Quoted 16 lines · On Fri, 5 Dec 2025 18:17:09 +0000 Alice Ryhl <[email protected]> wrote:
Quoted 10 lines · Oleg Nesterov
Quoted 8 lines · Alice Ryhl
pub fn group_leader(&self) -> &Task {
    // 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 { &*bindings::task_group_leader(self.as_ptr()).cast::<Task>() }
}  
Quoted 4 lines
Thanks again Alice, but the comment still looks misleading to me... OK, quite possibly this is because I don't understand what does the "lifetime of the returned task reference" actually mean in the rust code. Does it mean "lifetime of task_struct" of "lifetime of the process/thread" ?
Quoted 6 lines
To start with, it's likely that this comment is not the right choice for this function, given our discussion. Most likely group_leader() needs to be moved to impl CurrentTask {} and the safety comment needs to explain why being the current task ensures that the returned &Task lives for long enough. I just took the safety comment from the code we have today.
This indeed sounds like the right approach to take.

Quoted 8 lines

If Task::pid or Task::group_leader just gives the pid or group leader at the time of invocation and doesn't have any stability guarantee, then the user of the functions will likely be misusing these functions.
It's better to just have them on CurrentTask for now. When an user arises that need to retrieve them for another task, then we can come back and think about a proper solution taking the scenario into account.

I believe Binder just uses them for including the pid when it prints to the console. But I'll look into this and figure out the best approach.

Alice

Oleg Nesterov

author

2025-12-05 15:35 UTC #9 lore raw

In reply to Alice Ryhl

Quoted 36 lines · Alice Ryhl

Quoted 34 lines · Oleg Nesterov

From rust/kernel/task.rs:

    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
        // not a data race.
        unsafe { *ptr::addr_of!((*self.as_ptr()).pid) }
    }

	rcu_read_lock();
	task = find_task_by_vpid(vpid);
	if (task)
		get_task_struct(task);
	rcu_read_unlock();

	if (task)
		pid = task->group_leader->pid; // BUG! ->group_leader can be already freed

Quoted 8 lines

Quoted 3 lines
Now the questions. Sorry! I don't know rust.
1. Can I simply remove these misleading comments? Or SAFETY comment is mandatory?
If the safety comments are wrong, then the code is probably wrong too!
What is the correct way to read the pid or group_leader fields of a struct task_struct?

Alice, let me repeat I don't know rust ;) So I'll try to answer in "C".

As long as "struct task_struct *task" pointer is stable and can't go away, it is always safe to _read_ task->pid or even task->group_leader (but see below).

However, task->pid is not "const" in general, so "The pid of a task never changes ..." doesn't look right. See exchange_tids() which swaps left->pid/right->pid.

So this C code

	pid1 = READ_ONCE(task->pid);
	...
	pid2 = READ_ONCE(task->pid);
	...
	BUG_ON(pid1 != pid2);

is only correct if same_thread(current, task) == T. Or tasklist_lock is held, or another lock (say, cred_guard_mutex) which blocks de_thread().

Same for ->group_leader, it is not stable in general. Plus, unlike ->pid it can go away if we race with mt-exec or this task exits. So, just for example, in theory

	char c = task->group_leader->comm[0];

is only safe if same_thread(current, task) == T, or tasklist_lock is held, or it is called under rcu_read_lock() and pid_alive(task) == T. This makes me think that the "The lifetime of the returned task reference is tied to the lifetime of self" comment is not right too.

See for example commit a15f37a40145c ("kernel/sys.c: fix the racy usage of task_lock(tsk->group_leader) in sys_prlimit64() paths")

However, according to "git grep -E '\bgroup_leader\('" the only user of group_leader(task) is drivers/android/binder/process.rs, and task is always "current", so I think the current usage of group_leader() is fine.

Quoted 21 lines

2. I am working on the patch(es) which move ->group_leader from task_struct to signal_struct, so the 1st change adds the new trivial helper in preparation:

	struct task_struct *task_group_leader(struct task_struct *task)
	{
		return task->group_leader; // will be updated
	}

   Now, how can I change group_leader() to use it? I guess I need to add

	struct task_struct *rust_helper_task_group_leader(struct task_struct *task)
	{
		return task_group_leader(task);
	}

   into rust/helpers/task.c, but will something like

	pub fn group_leader(&self) -> &Task {
		unsafe { bindings::task_group_leader(self.as_ptr()) }
	}

Quoted 6 lines

Quoted 1 line
work? I'm afraid it won't ;)
That looks like it should work. The rust_helper_ function is only required if task_group_leader is marked static inline. Otherwise bindings:: will pick up the function straight from the C header. (As long as the relevant header is included in bindings_helper.h)

Thanks Alice! (yes, I see your next email ;)

Oleg.

Alice Ryhl

reviewer

2025-12-05 16:28 UTC #10 lore raw

Quoted 39 lines · Oleg Nesterov

Quoted 38 lines · Alice Ryhl

Quoted 36 lines · Oleg Nesterov

From rust/kernel/task.rs:

    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
        // not a data race.
        unsafe { *ptr::addr_of!((*self.as_ptr()).pid) }
    }

	rcu_read_lock();
	task = find_task_by_vpid(vpid);
	if (task)
		get_task_struct(task);
	rcu_read_unlock();

	if (task)
		pid = task->group_leader->pid; // BUG! ->group_leader can be already freed

Now the questions. Sorry! I don't know rust.

Quoted 8 lines

Quoted 6 lines
Quoted 1 line
1. Can I simply remove these misleading comments? Or SAFETY comment is mandatory?
If the safety comments are wrong, then the code is probably wrong too!
What is the correct way to read the pid or group_leader fields of a struct task_struct?
Alice, let me repeat I don't know rust ;) So I'll try to answer in "C".

That's perfect :)

Quoted 2 lines

As long as "struct task_struct *task" pointer is stable and can't go away, it is always safe to _read_ task->pid or even task->group_leader (but see below).

Right. But if it might change in parallel with our read, then we should ideally use READ_ONCE() or rcu_dereference() or an atomic load or similar.

With regards to "as long as pointer is stable", you may assume that any time you see a &T or &mut T, then the ampersand means a reference, which is always stable. Pointers that may be dangling have to be raw pointers, which use *mut T/*const T syntax.

Quoted 5 lines

However, task->pid is not "const" in general, so "The pid of a task never changes ..." doesn't look right. See exchange_tids() which swaps left->pid/right->pid.
So this C code

Quoted 8 lines

	pid1 = READ_ONCE(task->pid);
	...
	pid2 = READ_ONCE(task->pid);
	...
	BUG_ON(pid1 != pid2);
is only correct if same_thread(current, task) == T. Or tasklist_lock is held, or another lock (say, cred_guard_mutex) which blocks de_thread().

Ok, then we should probably adjust the Rust Task::pid() method to use READ_ONCE() to read the pid. And adjust the safety comment to match.

Quoted 10 lines

Same for ->group_leader, it is not stable in general. Plus, unlike ->pid it can go away if we race with mt-exec or this task exits. So, just for example, in theory
	char c = task->group_leader->comm[0];
is only safe if same_thread(current, task) == T, or tasklist_lock is held, or it is called under rcu_read_lock() and pid_alive(task) == T. This makes me think that the "The lifetime of the returned task reference is tied to the lifetime of self" comment is not right too.

Yeah, the current code encodes the lifetime constraint that the task always outlives its group leader. So for example this compiles:

	let leader = task.group_leader();
	do_stuff(leader); // ok

but this will fail to compile:

let leader = task.group_leader(); drop(task); // put refcount on task do_stuff(leader); // ERROR task is no longer valid

because Rust can see that leader is used in a context where using task would be illegal.

But it sounds like the above is not sufficient, and that we have situation where incorrect code would compile.

Quoted 6 lines

See for example commit a15f37a40145c ("kernel/sys.c: fix the racy usage of task_lock(tsk->group_leader) in sys_prlimit64() paths")
However, according to "git grep -E '\bgroup_leader\('" the only user of group_leader(task) is drivers/android/binder/process.rs, and task is always "current", so I think the current usage of group_leader() is fine.

Yes, the current usage seems fine. However, one of the main goals behind API design in Rust is to ensure that no matter how stupid the code that uses it is, if the final code is safe, then it should not lead to UAF or other memory unsafety.

In this case, we can fix the design by moving Task::group_leader() to the impl CurrentTask {} block. That way, calling group_leader() on a task will only compile if it's called on the current task.

We could also move Task::pid() to the impl CurrentTask block to avoid the READ_ONCE(). Or we could add pid() in both places, in which case task.pid() would use READ_ONCE() only when the &Task is not derived from current.

Alice