[PATCH 7/7] pid: get pid_t ppid of task in init_pid_ns

[PATCH 7/7] pid: get pid_t ppid of task in init_pid_ns

[Richard Guy Briggs]

Added the functions task_ppid_nr_ns() and task_ppid_nr() to abstract the lookup
of the PPID (real_parent's pid_t) of a process, including rcu locking, in the
arbitrary and init_pid_ns.
This provides an alternative to sys_getppid(), which is relative to the child
process' pid namespace.

(informed by ebiederman's 6c621b7e)
Cc: stable@vger.kernel.org
Cc: Eric W. Biederman <ebiederm@xmission.com>
Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
 include/linux/sched.h |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 2016d92..cba2486 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1561,6 +1561,24 @@ static inline pid_t task_tgid_vnr(const struct task_struct *tsk)
 }
 
 
+static int pid_alive(const struct task_struct *p);
+static inline pid_t task_ppid_nr_ns(const struct task_struct *tsk, struct pid_namespace *ns)
+{
+	pid_t pid = 0;
+
+	rcu_read_lock();
+	if (pid_alive(tsk))
+		pid = task_tgid_nr_ns(rcu_dereference(tsk->real_parent), ns);
+	rcu_read_unlock();
+
+	return pid;
+}
+
+static inline pid_t task_ppid_nr(const struct task_struct *tsk)
+{
+	return task_ppid_nr_ns(tsk, &init_pid_ns);
+}
+
 static inline pid_t task_pgrp_nr_ns(const struct task_struct *tsk,
 					struct pid_namespace *ns)
 {
-- 
1.7.1

Re: [PATCH 7/7] pid: get pid_t ppid of task in init_pid_ns

[Oleg Nesterov]

On 01/23, Richard Guy Briggs wrote:
>
> Added the functions task_ppid_nr_ns() and task_ppid_nr() to abstract the lookup
> of the PPID (real_parent's pid_t) of a process, including rcu locking, in the
> arbitrary and init_pid_ns.
> This provides an alternative to sys_getppid(), which is relative to the child
> process' pid namespace.

I agree, this makes sense.


imho it would be better to send this patch along with sys_getppid()
conversions, but I won't argue.

> +static int pid_alive(const struct task_struct *p);
> +static inline pid_t task_ppid_nr_ns(const struct task_struct *tsk, struct pid_namespace *ns)
> +{
> +	pid_t pid = 0;
> +
> +	rcu_read_lock();
> +	if (pid_alive(tsk))
> +		pid = task_tgid_nr_ns(rcu_dereference(tsk->real_parent), ns);
> +	rcu_read_unlock();
> +
> +	return pid;
> +}

Perhaps it should be named task_ptgid_nr_ns() or even parent_tgid_nr_ns().
Since it returns tgid, not pid (== tid).

But this is cosmetic, I won't insist.

Oleg.

Re: [PATCH 7/7] pid: get pid_t ppid of task in init_pid_ns

[Richard Guy Briggs]

On 14/02/20, Oleg Nesterov wrote:
> On 01/23, Richard Guy Briggs wrote:
> >
> > Added the functions task_ppid_nr_ns() and task_ppid_nr() to abstract the lookup
> > of the PPID (real_parent's pid_t) of a process, including rcu locking, in the
> > arbitrary and init_pid_ns.
> > This provides an alternative to sys_getppid(), which is relative to the child
> > process' pid namespace.
> 
> I agree, this makes sense.
> 
> 
> imho it would be better to send this patch along with sys_getppid()
> conversions, but I won't argue.

I don't think sys_getppid() should be changed since it is a syscall
whose userspace user would assume the working namespace.  Many of the
kernel internal uses of it are likely wrong though, so fixing that
should probably fall under another patch.  I fixed audit in another
patch.

> > +static int pid_alive(const struct task_struct *p);
> > +static inline pid_t task_ppid_nr_ns(const struct task_struct *tsk, struct pid_namespace *ns)
> > +{
> > +	pid_t pid = 0;
> > +
> > +	rcu_read_lock();
> > +	if (pid_alive(tsk))
> > +		pid = task_tgid_nr_ns(rcu_dereference(tsk->real_parent), ns);
> > +	rcu_read_unlock();
> > +
> > +	return pid;
> > +}
> 
> Perhaps it should be named task_ptgid_nr_ns() or even parent_tgid_nr_ns().
> Since it returns tgid, not pid (== tid).

I like task_ptgid_nr_ns() (or maybe even task_parent_tgid_nr_ns() but
that gets a bit long.

> But this is cosmetic, I won't insist.
> 
> Oleg.

- RGB

--
Richard Guy Briggs <rbriggs@redhat.com>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545

Re: [PATCH 7/7] pid: get pid_t ppid of task in init_pid_ns

[Oleg Nesterov]

On 02/21, Richard Guy Briggs wrote:
>
> On 14/02/20, Oleg Nesterov wrote:
> >
> > imho it would be better to send this patch along with sys_getppid()
> > conversions, but I won't argue.
>
> I don't think sys_getppid() should be changed since it is a syscall
> whose userspace user would assume the working namespace.

Sorry for confusion, I didn't mean we need to change sys_getppid(),

> Many of the
> kernel internal uses of it are likely wrong though, so fixing that
> should probably fall under another patch.  I fixed audit in another
> patch.

Yes, this is what I meant. And this patch should probably go into
that series which fixes the in-kernel users of sys_getppid().

And in fact you already did this? I see another series which resends
this patch as 1/5.

Oleg.

Re: [PATCH 7/7] pid: get pid_t ppid of task in init_pid_ns

[Tony Luck]

On Thu, Jan 23, 2014 at 11:32 AM, Richard Guy Briggs <rgb@redhat.com> wrote:
> Added the functions task_ppid_nr_ns() and task_ppid_nr() to abstract the lookup
> of the PPID (real_parent's pid_t) of a process, including rcu locking, in the
> arbitrary and init_pid_ns.
> This provides an alternative to sys_getppid(), which is relative to the child
> process' pid namespace.
...
> +static int pid_alive(const struct task_struct *p);

This patch (or some successor version of it) showed up
in next-20140317 and the above declaration caused a
bunch of warnings on ia64:

include/linux/sched.h:1718: warning: 'pid_alive' declared inline after
being called

[repeated 1675 times across files that include this]

The ia64 complier is a lot happier if "inline" is added like this:

static inline int pid_alive(const struct task_struct *p);

-Tony

Re: [PATCH 7/7] pid: get pid_t ppid of task in init_pid_ns

[Eric Paris]

On Mon, 2014-03-17 at 13:14 -0700, Tony Luck wrote:
> On Thu, Jan 23, 2014 at 11:32 AM, Richard Guy Briggs <rgb@redhat.com> wrote:
> > Added the functions task_ppid_nr_ns() and task_ppid_nr() to abstract the lookup
> > of the PPID (real_parent's pid_t) of a process, including rcu locking, in the
> > arbitrary and init_pid_ns.
> > This provides an alternative to sys_getppid(), which is relative to the child
> > process' pid namespace.
> ...
> > +static int pid_alive(const struct task_struct *p);
> 
> This patch (or some successor version of it) showed up
> in next-20140317 and the above declaration caused a
> bunch of warnings on ia64:
> 
> include/linux/sched.h:1718: warning: 'pid_alive' declared inline after
> being called
> 
> [repeated 1675 times across files that include this]
> 
> The ia64 complier is a lot happier if "inline" is added like this:
> 
> static inline int pid_alive(const struct task_struct *p);

Fixed for tomorrow.