linux-kernel - Re: [PATCH] procfs: fix missing RCU protection when reading real_parent in do_task

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20260128031059.2762637-1-alexjlzheng@tencent.com>
Date: Wed, 28 Jan 2026 11:10:57 +0800
From: Jinliang Zheng <alexjlzheng@...il.com>
To: oleg@...hat.com
Cc: akpm@...ux-foundation.org,
	alexjlzheng@...il.com,
	alexjlzheng@...cent.com,
	david@...nel.org,
	linux-fsdevel@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	lorenzo.stoakes@...cle.com,
	mingo@...nel.org,
	ruippan@...cent.com,
	usamaarif642@...il.com
Subject: Re: [PATCH] procfs: fix missing RCU protection when reading real_parent in do_task_stat()

On Tue, 27 Jan 2026 18:25:25 +0100, oleg@...hat.com wrote:
> On 02/27, alexjlzheng@...il.com wrote:
> >
> > From: Jinliang Zheng <alexjlzheng@...cent.com>
> >
> > When reading /proc/[pid]/stat, do_task_stat() accesses task->real_parent
> > without proper RCU protection, which leads:
> 
> Thanks for the patch...
> 
> >   cpu 0                               cpu 1
> >   -----                               -----
> >   do_task_stat
> >     var = task->real_parent
> >                                       release_task
> >                                         call_rcu(delayed_put_task_struct)
> >     task_tgid_nr_ns(var)
> >       rcu_read_lock   <--- Too late!
> 
> Almost off-topic, but I can't resist. This looks confusing to me.
> It is not "Too late", this rcu_read_lock() protects another thing.
> Nevermind.

Yes, and would "Too late to protect task->real_parent!" be better?

> 
> I think that the changelog could be more clear. It should probably
> mention that forget_original_parent() doesn't take child->signal->siglock
> and thus we have a race... I dunno.
> 
> > --- a/fs/proc/array.c
> > +++ b/fs/proc/array.c
> > @@ -528,7 +528,9 @@ static int do_task_stat(struct seq_file *m, struct pid_namespace *ns,
> >  		}
> >
> >  		sid = task_session_nr_ns(task, ns);
> > -		ppid = task_tgid_nr_ns(task->real_parent, ns);
> > +		rcu_read_lock();
> > +		ppid = task_tgid_nr_ns(rcu_dereference(task->real_parent), ns);
> > +		rcu_read_unlock();
> 
> But this can't really help. If task->real_parent has already exited and
> it was reaped, then it is actually "Too late!" for rcu_read_lock().

I think this is acceptable, because we tolerate obtaining a stale value as
long as it doesn’t lead to a Use-After-Free (UAF) bug. This is similar to
the comments in the syscall getppid().

With the protection of rcu_read_lock()/rcu_read_unlock() for loading
task->real_parent, we can guarantee that the task_struct of real_parent
itself will not be freed.

Or, do I miss something?

Thanks,
Jinliang Zheng. :)

> 
> Please use task_ppid_nr_ns() which does the necessary pid_alive() check.



> 
> Oleg.