Re: [PATCH] procfs: fix missing RCU protection when reading real_parent in do_task_stat()

[Oleg Nesterov]

On 01/27, alexjlzheng@xxxxxxxxx wrote:
>
> From: Jinliang Zheng <alexjlzheng@xxxxxxxxxxx>
>
> 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.

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().

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

Oleg.