* [PATCH 1/2][v2] pid: Implement ns_of_pid
@ 2008-12-17 0:02 ` Sukadev Bhattiprolu
0 siblings, 0 replies; 6+ messages in thread
From: Sukadev Bhattiprolu @ 2008-12-17 0:02 UTC (permalink / raw)
To: Andrew Morton
Cc: Eric W. Biederman, Oleg Nesterov, bastian, Pavel Emelyanov,
roland, Containers, linux-kernel
Resending patch with correct "From", "Signed-off-by" and a note
in function header of ns_of_pid().
---
From: Eric Biederman <ebiederm@xmission.com>
Subject: [PATCH] pid: Implement ns_of_pid
A current problem with the pid namespace is that it is
easy to do pid related work after exit_task_namespaces which
drops the nsproxy pointer.
However if we are doing pid namespace related work we are
always operating on some struct pid which retains the pid_namespace
pointer of the pid namespace it was allocated in.
So provide ns_of_pid which allows us to find the pid
namespace a pid was allocated in.
Using this we have the needed infrastructure to do pid
namespace related work at anytime we have a struct pid,
removing the chance of accidentally having a NULL
pointer dereference when accessing current->nsproxy.
Changelog[v2]:
- Add a note in function header of ns_of_pid() re: NULL pid.
Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
---
include/linux/pid.h | 18 ++++++++++++++++++
1 files changed, 18 insertions(+), 0 deletions(-)
diff --git a/include/linux/pid.h b/include/linux/pid.h
index d7e98ff..da6f174 100644
--- a/include/linux/pid.h
+++ b/include/linux/pid.h
@@ -122,6 +122,24 @@ int next_pidmap(struct pid_namespace *pid_ns, int last);
extern struct pid *alloc_pid(struct pid_namespace *ns);
extern void free_pid(struct pid *pid);
+/*
+ * ns_of_pid() returns the pid namespace in which the specified pid was
+ * allocated.
+ *
+ * NOTE:
+ * ns_of_pid() is expected to be called for a process (task) that has
+ * an attached 'struct pid' (see attach_pid(), detach_pid()) i.e @pid
+ * is expected to be non-NULL. If @pid is NULL, caller should handle
+ * the resulting NULL pid-ns.
+ */
+static inline struct pid_namespace *ns_of_pid(struct pid *pid)
+{
+ struct pid_namespace *ns = NULL;
+ if (pid)
+ ns = pid->numbers[pid->level].ns;
+ return ns;
+}
+
/*
* the helpers to get the pid's id seen from different namespaces
*
--
1.5.2.5
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 1/2][v2] pid: Implement ns_of_pid
@ 2008-12-17 0:02 ` Sukadev Bhattiprolu
0 siblings, 0 replies; 6+ messages in thread
From: Sukadev Bhattiprolu @ 2008-12-17 0:02 UTC (permalink / raw)
To: Andrew Morton
Cc: Eric W. Biederman, Oleg Nesterov, bastian, Pavel Emelyanov,
roland, Containers, linux-kernel
Resending patch with correct "From", "Signed-off-by" and a note
in function header of ns_of_pid().
---
From: Eric Biederman <ebiederm@xmission.com>
Subject: [PATCH] pid: Implement ns_of_pid
A current problem with the pid namespace is that it is
easy to do pid related work after exit_task_namespaces which
drops the nsproxy pointer.
However if we are doing pid namespace related work we are
always operating on some struct pid which retains the pid_namespace
pointer of the pid namespace it was allocated in.
So provide ns_of_pid which allows us to find the pid
namespace a pid was allocated in.
Using this we have the needed infrastructure to do pid
namespace related work at anytime we have a struct pid,
removing the chance of accidentally having a NULL
pointer dereference when accessing current->nsproxy.
Changelog[v2]:
- Add a note in function header of ns_of_pid() re: NULL pid.
Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
---
include/linux/pid.h | 18 ++++++++++++++++++
1 files changed, 18 insertions(+), 0 deletions(-)
diff --git a/include/linux/pid.h b/include/linux/pid.h
index d7e98ff..da6f174 100644
--- a/include/linux/pid.h
+++ b/include/linux/pid.h
@@ -122,6 +122,24 @@ int next_pidmap(struct pid_namespace *pid_ns, int last);
extern struct pid *alloc_pid(struct pid_namespace *ns);
extern void free_pid(struct pid *pid);
+/*
+ * ns_of_pid() returns the pid namespace in which the specified pid was
+ * allocated.
+ *
+ * NOTE:
+ * ns_of_pid() is expected to be called for a process (task) that has
+ * an attached 'struct pid' (see attach_pid(), detach_pid()) i.e @pid
+ * is expected to be non-NULL. If @pid is NULL, caller should handle
+ * the resulting NULL pid-ns.
+ */
+static inline struct pid_namespace *ns_of_pid(struct pid *pid)
+{
+ struct pid_namespace *ns = NULL;
+ if (pid)
+ ns = pid->numbers[pid->level].ns;
+ return ns;
+}
+
/*
* the helpers to get the pid's id seen from different namespaces
*
--
1.5.2.5
^ permalink raw reply related [flat|nested] 6+ messages in thread[parent not found: <20081217000257.GA810-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>]
* [PATCH 2/2] pid: generalize task_active_pid_ns
2008-12-17 0:02 ` Sukadev Bhattiprolu
@ 2008-12-17 0:05 ` Sukadev Bhattiprolu
-1 siblings, 0 replies; 6+ messages in thread
From: Sukadev Bhattiprolu @ 2008-12-17 0:05 UTC (permalink / raw)
To: Andrew Morton
Cc: bastian-yyjItF7Rl6lg9hUCZPvPmw,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Eric W. Biederman,
Containers, Oleg Nesterov, roland-H+wXaHxf7aLQT0dZR+AlfA,
Pavel Emelyanov
Resending patch with correct "From", "Signed-off-by".
---
From: Eric Biederman <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
Date: Fri, 12 Dec 2008 11:49:27 -0800
Subject: [PATCH] pid: Generalize task_active_pid_ns
Currently task_active_pid_ns is not safe to call after a
task becomes a zombie and exit_task_namespaces is called,
as nsproxy becomes NULL. By reading the pid namespace from
the pid of the task we can trivially solve this problem at
the cost of one extra memory read in what should be the
same cacheline as we read the namespace from.
When moving things around I have made task_active_pid_ns
out of line because keeping it in pid_namespace.h would
require adding includes of pid.h and sched.h that I
don't think we want.
This change does make task_active_pid_ns unsafe to call during
copy_process until we attach a pid on the task_struct which
seems to be a reasonable trade off.
Signed-off-by: Eric W. Biederman <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
include/linux/pid_namespace.h | 6 +-----
kernel/fork.c | 4 ++--
kernel/pid.c | 6 ++++++
3 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/include/linux/pid_namespace.h b/include/linux/pid_namespace.h
index d82fe82..38d1032 100644
--- a/include/linux/pid_namespace.h
+++ b/include/linux/pid_namespace.h
@@ -79,11 +79,7 @@ static inline void zap_pid_ns_processes(struct pid_namespace *ns)
}
#endif /* CONFIG_PID_NS */
-static inline struct pid_namespace *task_active_pid_ns(struct task_struct *tsk)
-{
- return tsk->nsproxy->pid_ns;
-}
-
+extern struct pid_namespace *task_active_pid_ns(struct task_struct *tsk);
void pidhash_init(void);
void pidmap_init(void);
diff --git a/kernel/fork.c b/kernel/fork.c
index dba2d3f..9fa1d91 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1100,12 +1100,12 @@ static struct task_struct *copy_process(unsigned long clone_flags,
if (pid != &init_struct_pid) {
retval = -ENOMEM;
- pid = alloc_pid(task_active_pid_ns(p));
+ pid = alloc_pid(p->nsproxy->pid_ns);
if (!pid)
goto bad_fork_cleanup_io;
if (clone_flags & CLONE_NEWPID) {
- retval = pid_ns_prepare_proc(task_active_pid_ns(p));
+ retval = pid_ns_prepare_proc(p->nsproxy->pid_ns);
if (retval < 0)
goto bad_fork_free_pid;
}
diff --git a/kernel/pid.c b/kernel/pid.c
index 064e76a..c5513fe 100644
--- a/kernel/pid.c
+++ b/kernel/pid.c
@@ -474,6 +474,12 @@ pid_t task_session_nr_ns(struct task_struct *tsk, struct pid_namespace *ns)
}
EXPORT_SYMBOL(task_session_nr_ns);
+struct pid_namespace *task_active_pid_ns(struct task_struct *tsk)
+{
+ return ns_of_pid(task_pid(tsk));
+}
+EXPORT_SYMBOL_GPL(task_active_pid_ns);
+
/*
* Used by proc to find the first pid that is greater then or equal to nr.
*
--
1.5.2.5
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/2] pid: generalize task_active_pid_ns
@ 2008-12-17 0:05 ` Sukadev Bhattiprolu
0 siblings, 0 replies; 6+ messages in thread
From: Sukadev Bhattiprolu @ 2008-12-17 0:05 UTC (permalink / raw)
To: Andrew Morton
Cc: Eric W. Biederman, Oleg Nesterov, bastian, Pavel Emelyanov,
roland, Containers, linux-kernel
Resending patch with correct "From", "Signed-off-by".
---
From: Eric Biederman <ebiederm@xmission.com>
Date: Fri, 12 Dec 2008 11:49:27 -0800
Subject: [PATCH] pid: Generalize task_active_pid_ns
Currently task_active_pid_ns is not safe to call after a
task becomes a zombie and exit_task_namespaces is called,
as nsproxy becomes NULL. By reading the pid namespace from
the pid of the task we can trivially solve this problem at
the cost of one extra memory read in what should be the
same cacheline as we read the namespace from.
When moving things around I have made task_active_pid_ns
out of line because keeping it in pid_namespace.h would
require adding includes of pid.h and sched.h that I
don't think we want.
This change does make task_active_pid_ns unsafe to call during
copy_process until we attach a pid on the task_struct which
seems to be a reasonable trade off.
Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
---
include/linux/pid_namespace.h | 6 +-----
kernel/fork.c | 4 ++--
kernel/pid.c | 6 ++++++
3 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/include/linux/pid_namespace.h b/include/linux/pid_namespace.h
index d82fe82..38d1032 100644
--- a/include/linux/pid_namespace.h
+++ b/include/linux/pid_namespace.h
@@ -79,11 +79,7 @@ static inline void zap_pid_ns_processes(struct pid_namespace *ns)
}
#endif /* CONFIG_PID_NS */
-static inline struct pid_namespace *task_active_pid_ns(struct task_struct *tsk)
-{
- return tsk->nsproxy->pid_ns;
-}
-
+extern struct pid_namespace *task_active_pid_ns(struct task_struct *tsk);
void pidhash_init(void);
void pidmap_init(void);
diff --git a/kernel/fork.c b/kernel/fork.c
index dba2d3f..9fa1d91 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1100,12 +1100,12 @@ static struct task_struct *copy_process(unsigned long clone_flags,
if (pid != &init_struct_pid) {
retval = -ENOMEM;
- pid = alloc_pid(task_active_pid_ns(p));
+ pid = alloc_pid(p->nsproxy->pid_ns);
if (!pid)
goto bad_fork_cleanup_io;
if (clone_flags & CLONE_NEWPID) {
- retval = pid_ns_prepare_proc(task_active_pid_ns(p));
+ retval = pid_ns_prepare_proc(p->nsproxy->pid_ns);
if (retval < 0)
goto bad_fork_free_pid;
}
diff --git a/kernel/pid.c b/kernel/pid.c
index 064e76a..c5513fe 100644
--- a/kernel/pid.c
+++ b/kernel/pid.c
@@ -474,6 +474,12 @@ pid_t task_session_nr_ns(struct task_struct *tsk, struct pid_namespace *ns)
}
EXPORT_SYMBOL(task_session_nr_ns);
+struct pid_namespace *task_active_pid_ns(struct task_struct *tsk)
+{
+ return ns_of_pid(task_pid(tsk));
+}
+EXPORT_SYMBOL_GPL(task_active_pid_ns);
+
/*
* Used by proc to find the first pid that is greater then or equal to nr.
*
--
1.5.2.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
[parent not found: <20081213023144.GA3951@us.ibm.com>]
* [PATCH 2/2] pid: Generalize task_active_pid_ns
[not found] <20081213023144.GA3951@us.ibm.com>
@ 2008-12-13 2:36 ` Sukadev Bhattiprolu
2008-12-15 19:49 ` Serge E. Hallyn
0 siblings, 1 reply; 6+ messages in thread
From: Sukadev Bhattiprolu @ 2008-12-13 2:36 UTC (permalink / raw)
To: Andrew Morton
Cc: Eric W. Biederman, Oleg Nesterov, roland, bastian,
Pavel Emelyanov, sukadev, Containers, linux-kernel
From: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Date: Fri, 12 Dec 2008 11:49:27 -0800
Subject: [PATCH 2/2] pid: Generalize task_active_pid_ns
Currently task_active_pid_ns is not safe to call after a
task becomes a zombie and exit_task_namespaces is called,
as nsproxy becomes NULL. By reading the pid namespace from
the pid of the task we can trivially solve this problem at
the cost of one extra memory read in what should be the
same cacheline as we read the namespace from.
When moving things around I have made task_active_pid_ns
out of line because keeping it in pid_namespace.h would
require adding includes of pid.h and sched.h that I
don't think we want.
This change does make task_active_pid_ns unsafe to call during
copy_process until we attach a pid on the task_struct which
seems to be a reasonable trade off.
Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
include/linux/pid_namespace.h | 6 +-----
kernel/fork.c | 4 ++--
kernel/pid.c | 6 ++++++
3 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/include/linux/pid_namespace.h b/include/linux/pid_namespace.h
index d82fe82..38d1032 100644
--- a/include/linux/pid_namespace.h
+++ b/include/linux/pid_namespace.h
@@ -79,11 +79,7 @@ static inline void zap_pid_ns_processes(struct pid_namespace *ns)
}
#endif /* CONFIG_PID_NS */
-static inline struct pid_namespace *task_active_pid_ns(struct task_struct *tsk)
-{
- return tsk->nsproxy->pid_ns;
-}
-
+extern struct pid_namespace *task_active_pid_ns(struct task_struct *tsk);
void pidhash_init(void);
void pidmap_init(void);
diff --git a/kernel/fork.c b/kernel/fork.c
index dba2d3f..9fa1d91 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1100,12 +1100,12 @@ static struct task_struct *copy_process(unsigned long clone_flags,
if (pid != &init_struct_pid) {
retval = -ENOMEM;
- pid = alloc_pid(task_active_pid_ns(p));
+ pid = alloc_pid(p->nsproxy->pid_ns);
if (!pid)
goto bad_fork_cleanup_io;
if (clone_flags & CLONE_NEWPID) {
- retval = pid_ns_prepare_proc(task_active_pid_ns(p));
+ retval = pid_ns_prepare_proc(p->nsproxy->pid_ns);
if (retval < 0)
goto bad_fork_free_pid;
}
diff --git a/kernel/pid.c b/kernel/pid.c
index 064e76a..c5513fe 100644
--- a/kernel/pid.c
+++ b/kernel/pid.c
@@ -474,6 +474,12 @@ pid_t task_session_nr_ns(struct task_struct *tsk, struct pid_namespace *ns)
}
EXPORT_SYMBOL(task_session_nr_ns);
+struct pid_namespace *task_active_pid_ns(struct task_struct *tsk)
+{
+ return ns_of_pid(task_pid(tsk));
+}
+EXPORT_SYMBOL_GPL(task_active_pid_ns);
+
/*
* Used by proc to find the first pid that is greater then or equal to nr.
*
--
1.5.2.5
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 2/2] pid: Generalize task_active_pid_ns
2008-12-13 2:36 ` [PATCH 2/2] pid: Generalize task_active_pid_ns Sukadev Bhattiprolu
@ 2008-12-15 19:49 ` Serge E. Hallyn
0 siblings, 0 replies; 6+ messages in thread
From: Serge E. Hallyn @ 2008-12-15 19:49 UTC (permalink / raw)
To: Sukadev Bhattiprolu
Cc: Andrew Morton, bastian, linux-kernel, Eric W. Biederman,
Containers, Oleg Nesterov, roland, Pavel Emelyanov
Quoting Sukadev Bhattiprolu (sukadev@linux.vnet.ibm.com):
> From: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
> Date: Fri, 12 Dec 2008 11:49:27 -0800
> Subject: [PATCH 2/2] pid: Generalize task_active_pid_ns
>
> Currently task_active_pid_ns is not safe to call after a
> task becomes a zombie and exit_task_namespaces is called,
> as nsproxy becomes NULL. By reading the pid namespace from
> the pid of the task we can trivially solve this problem at
> the cost of one extra memory read in what should be the
> same cacheline as we read the namespace from.
>
> When moving things around I have made task_active_pid_ns
> out of line because keeping it in pid_namespace.h would
> require adding includes of pid.h and sched.h that I
> don't think we want.
>
> This change does make task_active_pid_ns unsafe to call during
> copy_process until we attach a pid on the task_struct which
> seems to be a reasonable trade off.
Yeah that's not really much of a tradeoff... the text for those
lines gets shorter! :)
Acked-by: Serge Hallyn <serue@us.ibm.com>
(to all three)
thanks,
-serge
> Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
> ---
> include/linux/pid_namespace.h | 6 +-----
> kernel/fork.c | 4 ++--
> kernel/pid.c | 6 ++++++
> 3 files changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/include/linux/pid_namespace.h b/include/linux/pid_namespace.h
> index d82fe82..38d1032 100644
> --- a/include/linux/pid_namespace.h
> +++ b/include/linux/pid_namespace.h
> @@ -79,11 +79,7 @@ static inline void zap_pid_ns_processes(struct pid_namespace *ns)
> }
> #endif /* CONFIG_PID_NS */
>
> -static inline struct pid_namespace *task_active_pid_ns(struct task_struct *tsk)
> -{
> - return tsk->nsproxy->pid_ns;
> -}
> -
> +extern struct pid_namespace *task_active_pid_ns(struct task_struct *tsk);
> void pidhash_init(void);
> void pidmap_init(void);
>
> diff --git a/kernel/fork.c b/kernel/fork.c
> index dba2d3f..9fa1d91 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -1100,12 +1100,12 @@ static struct task_struct *copy_process(unsigned long clone_flags,
>
> if (pid != &init_struct_pid) {
> retval = -ENOMEM;
> - pid = alloc_pid(task_active_pid_ns(p));
> + pid = alloc_pid(p->nsproxy->pid_ns);
> if (!pid)
> goto bad_fork_cleanup_io;
>
> if (clone_flags & CLONE_NEWPID) {
> - retval = pid_ns_prepare_proc(task_active_pid_ns(p));
> + retval = pid_ns_prepare_proc(p->nsproxy->pid_ns);
> if (retval < 0)
> goto bad_fork_free_pid;
> }
> diff --git a/kernel/pid.c b/kernel/pid.c
> index 064e76a..c5513fe 100644
> --- a/kernel/pid.c
> +++ b/kernel/pid.c
> @@ -474,6 +474,12 @@ pid_t task_session_nr_ns(struct task_struct *tsk, struct pid_namespace *ns)
> }
> EXPORT_SYMBOL(task_session_nr_ns);
>
> +struct pid_namespace *task_active_pid_ns(struct task_struct *tsk)
> +{
> + return ns_of_pid(task_pid(tsk));
> +}
> +EXPORT_SYMBOL_GPL(task_active_pid_ns);
> +
> /*
> * Used by proc to find the first pid that is greater then or equal to nr.
> *
> --
> 1.5.2.5
>
> _______________________________________________
> Containers mailing list
> Containers@lists.linux-foundation.org
> https://lists.linux-foundation.org/mailman/listinfo/containers
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2008-12-17 0:11 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-12-17 0:02 [PATCH 1/2][v2] pid: Implement ns_of_pid Sukadev Bhattiprolu
2008-12-17 0:02 ` Sukadev Bhattiprolu
[not found] ` <20081217000257.GA810-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2008-12-17 0:05 ` [PATCH 2/2] pid: generalize task_active_pid_ns Sukadev Bhattiprolu
2008-12-17 0:05 ` Sukadev Bhattiprolu
[not found] <20081213023144.GA3951@us.ibm.com>
2008-12-13 2:36 ` [PATCH 2/2] pid: Generalize task_active_pid_ns Sukadev Bhattiprolu
2008-12-15 19:49 ` Serge E. Hallyn
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.