Re: [PATCH] pid: use READ_ONCE() in pid_alive()

[Oleg Nesterov]

On 10/03, David Laight wrote:
>
> On Fri,  2 Oct 2026 01:21:41 +0000
> Babanpreet Singh <bbnpreetsingh@xxxxxxxxx> wrote:
>
> > KCSAN reports pid_alive() reading task->thread_pid while it gets cleared
> > under tasklist_lock. The check only cares about NULL, so READ_ONCE() and
> > WRITE_ONCE() are enough.
>
> I just looked at change_pid() - isn't it completely broken?

No, but...

> __change_pid() uses hlist_del_rcu() to remove the item from a list.
> IIUC this leaves the 'next' pointer valid to allow for concurrent readers.
> I thought that had to stay valid until the end of the rcu period.
> But the following attach_pid() adds the item to another list.

Yep. That is why do_each_pid_task() needs tasklist_lock.

This is the known fact, let me quote the part of my old email
https://lore.kernel.org/all/20200512150936.GA28621@xxxxxxxxxx/

	> Currently the tasklist_lock is shared mainly in order to observe
	> the list atomically for the PRIO_PGRP and PRIO_USER cases, as
	> the actual lookups are already rcu-safe,

	not really...

	do_each_pid_task(PIDTYPE_PGID) can race with change_pid(PIDTYPE_PGID)
	which moves the task from one hlist to another. Yes, it is safe in
	that task_struct can't go away. But still this is not right because
	do_each_pid_task() can scan the wrong (2nd) hlist.

Somehow I thought this was documented, but it isn't. And this is not obvious.
I think this deserves a comment above do_each_pid_task(), will send the patch.

Oleg.