From mboxrd@z Thu Jan  1 00:00:00 1970
From: Oleg Nesterov <oleg@redhat.com>
Subject: Re: [PATCH 03/12] pid: get ppid pid_t of task in init_pid_ns safely
Date: Tue, 27 Aug 2013 19:21:55 +0200
Message-ID: <20130827172155.GC29147@redhat.com>
References: <cover.1377032086.git.rgb@redhat.com> <a411b87ba4c4af251166ee5877cb8c646cf7826c.1377032086.git.rgb@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Return-path: <linux-security-module-owner@vger.kernel.org>
Content-Disposition: inline
In-Reply-To: <a411b87ba4c4af251166ee5877cb8c646cf7826c.1377032086.git.rgb@redhat.com>
Sender: linux-security-module-owner@vger.kernel.org
To: Richard Guy Briggs <rgb@redhat.com>
Cc: linux-audit@redhat.com, linux-kernel@vger.kernel.org, linux-security-module@vger.kernel.org, Eric Paris <eparis@redhat.com>, Al Viro <viro@zeniv.linux.org.uk>, Ingo Molnar <mingo@redhat.com>, Peter Zijlstra <peterz@infradead.org>, "Serge E. Hallyn" <serge@hallyn.com>, John Johansen <john.johansen@canonical.com>, James Morris <james.l.morris@oracle.com>, Andrew Morton <akpm@linux-foundation.org>, Kentaro Takeda <takedakn@nttdata.co.jp>, Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Jiri Slaby <jslaby@suse.cz>
List-Id: linux-audit@redhat.com

On 08/20, Richard Guy Briggs wrote:
>
> Added the functions
>         task_ppid()
>         task_ppid_nr_ns()
>         task_ppid_nr_init_ns()
> to safely abstract the lookup of the PPID

but it is not safe.

> +static inline struct pid *task_ppid(struct task_struct *task)
> +{
> +	return task_tgid(rcu_dereference(current->real_parent));
                                         ^^^^^^^
task?

> +static inline pid_t task_ppid_nr_ns(struct task_struct *tsk,
> +					struct pid_namespace *ns)
> +{
> +	pid_t pid;
> +
> +	rcu_read_lock();
> +	pid = pid_nr_ns(task_ppid(current), ns);
                                  ^^^^^^^
again.

> +	rcu_read_unlock();

And why this is safe?

rcu_read_lock() can't help if tsk was already dead _before_ it takes
the rcu lock. ->real_parent can point the already freed/reused/unmapped
memory.

This is safe if, for example, the caller alredy holds rcu_read_lock()
and tsk was found by find_task_by*(), or tsk is current.

Richard, just in case... I am going to vacation, I will be completely
offline till Sep 10.

Oleg.