OpenVZ Forum - RDF feed
type: channel
items: [RFC][PATCH 0/16] Enable cloning of pid namespace
items: [RFC][PATCH 01/16] Define/use task_active_pid_ns() wrapper
items: [RFC][PATCH 02/16] Rename pid_nr function
items: [RFC][PATCH 03/16] Rename child_reaper function
items: [RFC][PATCH 04/16] Use pid_to_nr() in process info functions
items: [RFC][PATCH 05/16] Use task_pid() to find leader's pid
items: [RFC][PATCH 06/16] Define is_global_init()
items: [RFC][PATCH 07/16] Move alloc_pid call to copy_process
items: [RFC][PATCH 08/16] Define/use pid->upid_list list.
items: [RFC][PATCH 09/16] Use pid ns from pid->upid_list
items: [RFC][PATCH 10/16] Define CLONE_NEWPID flag
items: [RFC][PATCH 11/16] Enable cloning pid namespace
items: [RFC][PATCH 12/16] Terminate processes in a ns when reaper is exiting.
items: [RFC][PATCH 13/16] Remove proc_mnt's use for killing inodes
items: [RFC][PATCH 14/16] Introduce proc_mnt for pid_ns
items: [RFC][PATCH 15/16] Enable signaling child reaper from parent ns.
items: [RFC][PATCH 16/16] Move inline functions to sched.h
items: Re: [RFC][PATCH 04/16] Use pid_to_nr() in process info functions
items: Re: [RFC][PATCH 06/16] Define is_global_init()
items: Re: [RFC][PATCH 07/16] Move alloc_pid call to copy_process
items: Re: [RFC][PATCH 08/16] Define/use pid->upid_list list.
items: Re: [RFC][PATCH 14/16] Introduce proc_mnt for pid_ns
items: Re: [RFC][PATCH 0/16] Enable cloning of pid namespace
items: Re: [RFC][PATCH 11/16] Enable cloning pid namespace
items: Re: [RFC][PATCH 15/16] Enable signaling child reaper from parent ns.

[RFC][PATCH 0/16] Enable cloning of pid namespace
type: item
description: Dave, Serge, Here is my current pid namespace patchset. There are still a couple of issues I am investigating, but appreciate any feedback. Suka --- From: Sukadev Bhattiprolu <sukadev@us.ibm.com> Subject: [RFC][PATCH 0/16] Enable clone pid namespace A pid namespace is a "view" of a particular set of tasks on the system. They work in a similar way to filesystem namespaces. A file (or a process) can be accessed in multiple namespaces, but it may have a different name in each. In a filesystem, this name might be /etc/passwd in one namespace, but /chroot/etc/passwd in another. For processes, a process may have pid 1234 in one namespace, but be pid 1 in another. This allows new pid namespaces to have basically arbitrary pids, and not have to worry about what pids exist in other namespaces. This is essential for checkpoint/restart where a restarted process's pid might collide with an existing process on the system's pid. In this particular implementation, pid namespaces have a parent-child relationship, just like processes. A process in a pid namespace may see all of the processes in the same namespace, as well as all of the processes in all of the namespaces which are children of its namespace. Processes may not, however, see others which are in their parent's namespace, but not in their own. The same goes for sibling namespaces. Imagine it like a directory structure that has several processes chroot'd into it: / |-- dir0 | |-- subdir0 | `-- subdir1 `-- dir1 |-- subdir2 `-- subdir3 A process in the '/' dir can see all of the directories. However, one chrooted into dir0 can only see subdirs 0 and 1. It can not see dir1, or subdirs 2 and 3. Pid namespaces work in the same way. Following are the patches in this set. They apply to the 2.6.21-mm2 kernel. [PATCH 1/16] Define/use task_active_pid_ns() wrapper [PATCH 2/16] Rename pid_nr function [PATCH 3/16] Rename child_reaper function. [PATCH 4/16] Use pid_to_nr() in process info functions [PATCH 5/16] Use task_pid() to find le…
creator: Sukadev Bhattiprolu
date: 2007-05-24T01:03:03-00:00

[RFC][PATCH 01/16] Define/use task_active_pid_ns() wrapper
type: item
description: Subject: Define/use task_active_pid_ns() wrapper From: Sukadev Bhattiprolu <sukadev@us.ibm.com> A process is visible in one or more pid namespaces, as it is known by some pid_t in every ancestor pid namespace. Every time the process forks, the child process also gets a pid_t in every ancestor pid namespace. While a process is visible in >=1 pid namespaces, it can see pid_t's in only one pid namespace. We call this pid namespace it's "active pid namespace", and it is always the youngest pid namespace in which the process is known. This patch defines/uses a wrapper to find the active pid namespace of a process. The implementation of the wrapper will be changed in subsequent patches when support for multiple pid namespaces are added. Changelog: 2.6.21-rc6-mm1: - Rename task_pid_ns() to task_active_pid_ns() to reflect that a process can have multiple pid namespaces. Signed-off-by: Sukadev Bhattiprolu <sukadev@us.ibm.com> --- fs/exec.c | 2 +- fs/proc/proc_misc.c | 3 ++- include/linux/pid_namespace.h | 7 ++++++- kernel/exit.c | 5 +++-- kernel/pid.c | 4 ++-- 5 files changed, 14 insertions(+), 7 deletions(-) Index: lx26-21-mm2/include/linux/pid_namespace.h =================================================================== --- lx26-21-mm2.orig/include/linux/pid_namespace.h 2007-05-22 16:59:29.000000000 -0700 +++ lx26-21-mm2/include/linux/pid_namespace.h 2007-05-22 17:49:52.000000000 -0700 @@ -37,9 +37,14 @@ static inline void put_pid_ns(struct pid kref_put(&ns->kref, free_pid_ns); } +static inline struct pid_namespace *task_active_pid_ns(struct task_struct *tsk) +{ + return tsk->nsproxy->pid_ns; +} + static inline struct task_struct *child_reaper(struct task_struct *tsk) { - return init_pid_ns.child_reaper; + return task_active_pid_ns(tsk)->child_reaper; } #endif /* _LINUX_PID_NS_H */ Index: lx26-21-mm2/fs/exec.c =================================================================== --- lx26-21-mm2.orig/fs/exec.c 2007-05-22 16:59:29.000000000 -0700 +++ lx26-21-mm2/fs/exec.c 20…
creator: Sukadev Bhattiprolu
date: 2007-05-24T01:08:20-00:00

[RFC][PATCH 02/16] Rename pid_nr function
type: item
description: Subject: Rename pid_nr function From: Sukadev Bhattiprolu <sukadev@us.ibm.com> Rename pid_nr() function to pid_to_nr() which is more descriptive and will hopefully cause less confusion with new structure/functions being added to support multiple pid namespaces. Signed-off-by: Sukadev Bhattiprolu <sukadev@us.ibm.com> --- drivers/char/n_r3964.c | 7 ++++--- drivers/char/tty_io.c | 4 ++-- fs/autofs/root.c | 2 +- fs/fcntl.c | 2 +- fs/proc/array.c | 2 +- fs/smbfs/proc.c | 2 +- include/linux/pid.h | 2 +- ipc/mqueue.c | 2 +- kernel/fork.c | 2 +- kernel/sysctl.c | 2 +- 10 files changed, 14 insertions(+), 13 deletions(-) Index: lx26-21-mm2/include/linux/pid.h =================================================================== --- lx26-21-mm2.orig/include/linux/pid.h 2007-05-22 16:59:29.000000000 -0700 +++ lx26-21-mm2/include/linux/pid.h 2007-05-22 16:59:40.000000000 -0700 @@ -98,7 +98,7 @@ extern struct pid *find_ge_pid(int nr); extern struct pid *alloc_pid(void); extern void FASTCALL(free_pid(struct pid *pid)); -static inline pid_t pid_nr(struct pid *pid) +static inline pid_t pid_to_nr(struct pid *pid) { pid_t nr = 0; if (pid) Index: lx26-21-mm2/drivers/char/n_r3964.c =================================================================== --- lx26-21-mm2.orig/drivers/char/n_r3964.c 2007-05-22 16:59:29.000000000 -0700 +++ lx26-21-mm2/drivers/char/n_r3964.c 2007-05-22 16:59:40.000000000 -0700 @@ -771,7 +771,7 @@ static int enable_signals(struct r3964_i if (pClient->pid == pid) { TRACE_PS("removing client %d from client list", - pid_nr(pid)); + pid_to_nr(pid)); *ppClient = pClient->next; while (pClient->msg_count) { pMsg = remove_msg(pInfo, pClient); @@ -801,7 +801,8 @@ static int enable_signals(struct r3964_i if (pClient == NULL) return -ENOMEM; - TRACE_PS("add client %d to client list", pid_nr(pid)); + TRACE_PS("add client %d to client list", + pid_to_nr(pid)); spin_lock_init(&pClient->lock); pClient->sig_flags = arg; pClient->pid = get_pid(pid); @@ -933,7 +934,7 @@ static void r…
creator: Sukadev Bhattiprolu
date: 2007-05-24T01:08:52-00:00

[RFC][PATCH 03/16] Rename child_reaper function
type: item
description: Subject: Rename child_reaper function. From: Sukadev Bhattiprolu <sukadev@us.ibm.com> Rename the child_reaper() function to task_child_reaper() to be inline with other task_* functions and to distinguish the function from struct pid_namspace.child_reaper. Signed-off-by: Sukadev Bhattiprolu <sukadev@us.ibm.com> --- fs/exec.c | 2 +- include/linux/pid_namespace.h | 2 +- kernel/exit.c | 4 ++-- kernel/signal.c | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) Index: lx26-21-mm2/fs/exec.c =================================================================== --- lx26-21-mm2.orig/fs/exec.c 2007-05-22 16:59:34.000000000 -0700 +++ lx26-21-mm2/fs/exec.c 2007-05-22 16:59:42.000000000 -0700 @@ -628,7 +628,7 @@ static int de_thread(struct task_struct * Reparenting needs write_lock on tasklist_lock, * so it is safe to do it under read_lock. */ - if (unlikely(tsk->group_leader == child_reaper(tsk))) + if (unlikely(tsk->group_leader == task_child_reaper(tsk))) task_active_pid_ns(tsk)->child_reaper = tsk; zap_other_threads(tsk); Index: lx26-21-mm2/kernel/exit.c =================================================================== --- lx26-21-mm2.orig/kernel/exit.c 2007-05-22 16:59:34.000000000 -0700 +++ lx26-21-mm2/kernel/exit.c 2007-05-22 16:59:42.000000000 -0700 @@ -687,7 +687,7 @@ forget_original_parent(struct task_struc do { reaper = next_thread(reaper); if (reaper == father) { - reaper = child_reaper(father); + reaper = task_child_reaper(father); break; } } while (reaper->exit_state); @@ -875,7 +875,7 @@ fastcall NORET_TYPE void do_exit(long co panic("Aiee, killing interrupt handler!"); if (unlikely(!tsk->pid)) panic("Attempted to kill the idle task!"); - if (unlikely(tsk == child_reaper(tsk))) { + if (unlikely(tsk == task_child_reaper(tsk))) { if (task_active_pid_ns(tsk) != &init_pid_ns) task_active_pid_ns(tsk)->child_reaper = init_pid_ns.child_reaper; Index: lx26-21-mm2/kernel/signal.c =================================================================== --- lx26-21-mm2.orig/k…
creator: Sukadev Bhattiprolu
date: 2007-05-24T01:09:28-00:00

[RFC][PATCH 04/16] Use pid_to_nr() in process info functions
type: item
description: Subject: Use pid_to_nr() in process info functions From: Sukadev Bhattiprolu <sukadev@us.ibm.com> Use pid_to_nr() function in getppid(), getpgid() and getsid() functions so they return the correct pid_t for processes in multiple pid namespaces. Note: We don't need pid_to_nr() in getpid() because the process always "sees itself" as being in its active pid namespace. Using pid_to_nr() in getpid() would unnecessarily hurt its performance. Signed-off-by: Sukadev Bhattiprolu <sukadev@us.ibm.com> --- include/linux/sched.h | 24 ++++++++++++++++++++++++ kernel/sys.c | 10 +++++----- kernel/timer.c | 2 +- 3 files changed, 30 insertions(+), 6 deletions(-) Index: lx26-21-mm2/include/linux/sched.h =================================================================== --- lx26-21-mm2.orig/include/linux/sched.h 2007-05-22 16:58:38.000000000 -0700 +++ lx26-21-mm2/include/linux/sched.h 2007-05-22 16:59:44.000000000 -0700 @@ -1124,6 +1124,30 @@ static inline struct pid *task_tgid(stru return task->group_leader->pids[PIDTYPE_PID].pid; } +/* NOTE: Caller must hold rcu_readlock() */ +static inline struct pid *task_parent_tgid(struct task_struct *task) +{ + return task_tgid(rcu_dereference(task->real_parent)); +} + +/* NOTE: Caller must hold rcu_readlock() */ +static inline struct pid *task_parent_pid(struct task_struct *task) +{ + return task_pid(rcu_dereference(task->real_parent)); +} + +/* NOTE: Caller must hold rcu_readlock() */ +static inline struct pid *task_tracer_tgid(struct task_struct *task) +{ + return task_tgid(rcu_dereference(task->parent)); +} + +/* NOTE: Caller must hold rcu_readlock() */ +static inline struct pid *task_tracer_pid(struct task_struct *task) +{ + return task_pid(rcu_dereference(task->parent)); +} + static inline struct pid *task_pgrp(struct task_struct *task) { return task->group_leader->pids[PIDTYPE_PGID].pid; Index: lx26-21-mm2/kernel/timer.c =================================================================== --- lx26-21-mm2.orig/kernel/timer.c 2007-05-22 16:…
creator: Sukadev Bhattiprolu
date: 2007-05-24T01:09:57-00:00

[RFC][PATCH 05/16] Use task_pid() to find leader's pid
type: item
description: Subject: Use task_pid() to find leader's pid From: Sukadev Bhattiprolu <sukadev@us.ibm.com> Use task_pid() to get leader's pid since find_pid() cannot be used after detach_pid(). See comments in the code below for more details. Signed-off-by: Sukadev Bhattiprolu <sukadev@us.ibm.com> --- fs/exec.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) Index: lx26-21-mm2/fs/exec.c =================================================================== --- lx26-21-mm2.orig/fs/exec.c 2007-05-22 16:59:42.000000000 -0700 +++ lx26-21-mm2/fs/exec.c 2007-05-22 16:59:45.000000000 -0700 @@ -707,10 +707,17 @@ static int de_thread(struct task_struct * The old leader becomes a thread of the this thread group. * Note: The old leader also uses this pid until release_task * is called. Odd but simple and correct. + * Note: With multiple pid namespaces, active pid namespace of + * a process is stored in its struct pid. The detach_pid + * below frees the struct pid, so we will have no notion + * of an active pid namespace until we complete the + * subsequent attach_pid(). Which means - calls like + * find_pid()/pid_to_nr() return NULL and cannot be used + * between the detach_pid() and attach_pid() calls. */ detach_pid(tsk, PIDTYPE_PID); tsk->pid = leader->pid; - attach_pid(tsk, PIDTYPE_PID, find_pid(tsk->pid)); + attach_pid(tsk, PIDTYPE_PID, task_pid(leader)); transfer_pid(leader, tsk, PIDTYPE_PGID); transfer_pid(leader, tsk, PIDTYPE_SID); list_replace_rcu(&leader->tasks, &tsk->tasks); _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Sukadev Bhattiprolu
date: 2007-05-24T01:10:32-00:00

[RFC][PATCH 06/16] Define is_global_init()
type: item
description: Subject: Define is_global_init() From: Serge E. Hallyn <serue@us.ibm.com> is_init() is an ambiguous name for the pid==1 check. Split it into is_global_init() and is_container_init(). A container init has it's tsk->pid == 1. A global init also has it's tsk->pid == 1, and it's active pid namespace is the init_pid_ns. Changelog: 2.6.21-mm2-pidns2: - [Sukadev Bhattiprolu] Changed is_container_init() calls in {powerpc, ppc,avr32}/traps.c for the _exception() call to is_global_init(). This way, we kill only the container if the container's init has a bug rather than force a kernel panic. Signed-off-by: Serge E. Hallyn <serue@us.ibm.com> Signed-off-by: Sukadev Bhattiprolu <sukadev@us.ibm.com> --- arch/alpha/mm/fault.c | 2 +- arch/arm/mm/fault.c | 2 +- arch/arm26/mm/fault.c | 2 +- arch/avr32/kernel/traps.c | 2 +- arch/avr32/mm/fault.c | 6 +++--- arch/i386/lib/usercopy.c | 2 +- arch/i386/mm/fault.c | 2 +- arch/ia64/mm/fault.c | 2 +- arch/m32r/mm/fault.c | 2 +- arch/m68k/mm/fault.c | 2 +- arch/mips/mm/fault.c | 2 +- arch/powerpc/kernel/traps.c | 2 +- arch/powerpc/mm/fault.c | 2 +- arch/powerpc/platforms/pseries/ras.c | 2 +- arch/ppc/kernel/traps.c | 2 +- arch/ppc/mm/fault.c | 2 +- arch/s390/lib/uaccess_pt.c | 2 +- arch/s390/mm/fault.c | 2 +- arch/sh/mm/fault.c | 2 +- arch/sh64/mm/fault.c | 6 +++--- arch/um/kernel/trap.c | 2 +- arch/x86_64/mm/fault.c | 4 ++-- arch/xtensa/mm/fault.c | 2 +- drivers/char/sysrq.c | 2 +- include/linux/sched.h | 13 +++---------- kernel/capability.c | 3 ++- kernel/exit.c | 2 +- kernel/kexec.c | 2 +- kernel/pid.c | 21 +++++++++++++++++++++ kernel/sysctl.c | 2 +- mm/oom_kill.c | 4 ++-- security/commoncap.c | 2 +- 32 files changed, 61 insertions(+), 46 deletions(-) Index: lx26-21-mm2/arch/alpha/mm/fault.c =================================================================== --- lx26-21-mm2.orig/arch/alpha/mm/fault.c 2007-05-22 16:58:38.000000000 -0700 +++ lx26-21-mm2/arch/alpha/mm/fault.c 2007-05-22 16:59:46.000000000 -0700 @@ -192,7 +192,7 @@ do_page_fau…
creator: Sukadev Bhattiprolu
date: 2007-05-24T01:11:00-00:00

[RFC][PATCH 07/16] Move alloc_pid call to copy_process
type: item
description: Subject: Move alloc_pid call to copy_process From: Sukadev Bhattiprolu <sukadev@us.ibm.com> Move alloc_pid() into copy_process(). This will keep all pid and pid namespace code together and simplify error handling when we support multiple pid namespaces. Changelog: - [Eric Biederman] Move the check of copy_process_type to alloc_pid()/ free_pid() and to avoid clutter in copy_process(). Signed-off-by: Sukadev Bhattiprolu <sukadev@us.ibm.com> --- include/linux/pid.h | 7 ++++++- kernel/fork.c | 21 +++++++++++++-------- kernel/pid.c | 10 +++++++++- 3 files changed, 28 insertions(+), 10 deletions(-) Index: lx26-21-mm2/include/linux/pid.h =================================================================== --- lx26-21-mm2.orig/include/linux/pid.h 2007-05-22 16:59:40.000000000 -0700 +++ lx26-21-mm2/include/linux/pid.h 2007-05-22 17:06:48.000000000 -0700 @@ -3,6 +3,11 @@ #include <linux/rcupdate.h> +enum copy_process_type { + COPY_NON_IDLE_PROCESS, + COPY_IDLE_PROCESS, +}; + enum pid_type { PIDTYPE_PID, @@ -95,7 +100,7 @@ extern struct pid *FASTCALL(find_pid(int extern struct pid *find_get_pid(int nr); extern struct pid *find_ge_pid(int nr); -extern struct pid *alloc_pid(void); +extern struct pid *alloc_pid(enum copy_process_type); extern void FASTCALL(free_pid(struct pid *pid)); static inline pid_t pid_to_nr(struct pid *pid) Index: lx26-21-mm2/kernel/fork.c =================================================================== --- lx26-21-mm2.orig/kernel/fork.c 2007-05-22 16:59:41.000000000 -0700 +++ lx26-21-mm2/kernel/fork.c 2007-05-22 17:06:48.000000000 -0700 @@ -961,10 +961,11 @@ static struct task_struct *copy_process( unsigned long stack_size, int __user *parent_tidptr, int __user *child_tidptr, - struct pid *pid) + enum copy_process_type copy_src) { int retval; struct task_struct *p = NULL; + struct pid *pid; if ((clone_flags & (CLONE_NEWNS|CLONE_FS)) == (CLONE_NEWNS|CLONE_FS)) return ERR_PTR(-EINVAL); @@ -1025,6 +1026,10 @@ static struct task_struct *copy_process( if (p->…
creator: Sukadev Bhattiprolu
date: 2007-05-24T01:11:24-00:00

[RFC][PATCH 08/16] Define/use pid->upid_list list.
type: item
description: Subject: Define/use pid->upid_list list. From: Sukadev Bhattiprolu <sukadev@us.ibm.com> With multiple pid namespaces, a process would be known by several pid_t values, one in each pid namespace. To represent this, we introduce a 'struct upid' which associates a single pid_t value with a single pid namespace. We then replace the pid->nr field in 'struct pid' with a list of struct upid' entries (referred to as the pid->upid_list list). This list represents the multiple pid_t values of the process, one in each namespace. The struct upid also replaces 'struct pid' in the pid_hash table to enable us to find processes given a pid_t from any namespace (i.e we find 'struct upid' for a given pid_t and from the 'struct upid', we find the 'struct pid' of the process) We finally reimplement find_pid() and pid_to_nr() to use pid->upid_list and remove unused fields from 'struct pid'. Changelog: 2.6.21-mm2-pidns3: - 'struct upid' used to be called 'struct pid_nr' and a list of these were hanging off of 'struct pid'. So, we renamed 'struct pid_nr' and now hold them in a statically sized array in 'struct pid' since the number of 'struct upid's for a process is known at process- creation time. 2.6.21-rc3-mm2: - [Eric Biederman] Combine all logical changes into one patch - [Eric Biederman] Implement __pid_nr(pid_ns, pid) for use in procfs. (now called pid_to_nr_in_ns()). - [Serge Hallyn]: Remove (!pid_nr) check in free_pid_nr() Signed-off-by: Cedric Le Goater <clg@fr.ibm.com> Signed-off-by: Sukadev Bhattiprolu <sukadev@us.ibm.com> --- fs/proc/array.c | 30 ++++-- fs/proc/base.c | 9 +- include/linux/init_task.h | 13 ++ include/linux/pid.h | 35 +++++-- include/linux/pid_namespace.h | 12 ++ kernel/fork.c | 2 kernel/pid.c | 187 +++++++++++++++++++++++++++++++++++------- 7 files changed, 232 insertions(+), 56 deletions(-) Index: lx26-21-mm2/include/linux/pid.h =================================================================== --- lx26-21-mm2.orig/include/linux/pid.h 2007-05-22 17:06:48.000…
creator: Sukadev Bhattiprolu
date: 2007-05-24T01:11:43-00:00

[RFC][PATCH 09/16] Use pid ns from pid->upid_list
type: item
description: Subject: Use pid ns from pid->upid_list From: Sukadev Bhattiprolu <sukadev@us.ibm.com> We need to decouple pid namespace from nsproxy to allow getting and releasing pid namespace independently from other namespaces. This is required since a process's reference to its pid namespace must exist even after its references to other namespaces are dropped during process exit. With multiple pid namespaces, a process can have different pid_t values in different pid namespaces and 'struct upid' and the pid->upid_list list provide this association of pid_t value with pid namespace for a process. Use pid->upid_list list to find the active pid namespace of a process and remove nsproxy->pid_namespace. (Review note: My description of active pid namespace is probably long-winded. Appreciate any comments in explaining it better :-) TODO: - Include pid_namespace in pid_hash() so processes with same pid_t in different namespaces are on different hash lists. Changelog: 2.6.21-mm2-pidns3: - 'struct upid' used to be called 'struct pid_nr' and a list of these were hanging off of 'struct pid'. So, we renamed 'struct pid_nr' and now hold them in a statically sized array in 'struct pid' since the number of 'struct upid's for a process is known at process- creation time. [2.6.21-rc3-mm2] - Drop support for unshare() of pid namespace which simplifies cloning of pid namespace and reorganize several functions. Signed-off-by: Sukadev Bhattiprolu <sukadev@us.ibm.com> --- include/linux/init_task.h | 1 - include/linux/nsproxy.h | 2 -- include/linux/pid_namespace.h | 4 ++-- kernel/nsproxy.c | 10 ---------- kernel/pid.c | 23 ++++++++++++++++------- 5 files changed, 18 insertions(+), 22 deletions(-) Index: lx26-21-mm2/include/linux/init_task.h =================================================================== --- lx26-21-mm2.orig/include/linux/init_task.h 2007-05-22 16:59:49.000000000 -0700 +++ lx26-21-mm2/include/linux/init_task.h 2007-05-22 16:59:50.000000000 -0700 @@ -72,7 +72,6 @@ extern struct ns…
creator: Sukadev Bhattiprolu
date: 2007-05-24T01:12:36-00:00

[RFC][PATCH 10/16] Define CLONE_NEWPID flag
type: item
description: Subject: Define CLONE_NEWPID flag From: Sukadev Bhattiprolu <sukadev@us.ibm.com> This was discussed on containers and we thought it would be useful to reserve this flag. TODO: Merge this with the next patch ? Define CLONE_NEWPID flag that will be used to clone pid namespaces. Signed-off-by: Sukadev Bhattiprolu <sukadev@us.ibm.com> --- include/linux/sched.h | 1 + 1 file changed, 1 insertion(+) Index: lx26-21-mm2/include/linux/sched.h =================================================================== --- lx26-21-mm2.orig/include/linux/sched.h 2007-05-22 16:59:46.000000000 -0700 +++ lx26-21-mm2/include/linux/sched.h 2007-05-22 16:59:51.000000000 -0700 @@ -26,6 +26,7 @@ #define CLONE_STOPPED 0x02000000 /* Start in stopped state */ #define CLONE_NEWUTS 0x04000000 /* New utsname group? */ #define CLONE_NEWIPC 0x08000000 /* New ipcs */ +#define CLONE_NEWPID 0x10000000 /* New pid namespace */ /* * Scheduling policies _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Sukadev Bhattiprolu
date: 2007-05-24T01:13:03-00:00

[RFC][PATCH 11/16] Enable cloning pid namespace
type: item
description: Subject: Enable cloning pid namespace From: Sukadev Bhattiprolu <sukadev@us.ibm.com> When clone() is invoked with CLONE_NEWPID, create a new pid namespace and then create a new struct pid for the new process. Allocate pid_t's for the new process in the new pid namespace and all ancestor pid namespaces. Make the newly cloned process the session and process group leader. Since the active pid namespace is special and expected to be the first entry in pid->upid_list, preserve the order of pid namespaces when cloning without CLONE_NEWPID. TODO (partial list:) - Identify clone flags that should not be specified with CLONE_NEWPID and return -EINVAL from copy_process(), if they are specified. (eg: CLONE_THREAD|CLONE_NEWPID ?) - Add a privilege check for CLONE_NEWPID Changelog: 2.6.21-mm2-pidns3: - 'struct upid' used to be called 'struct pid_nr' and a list of these were hanging off of 'struct pid'. So, we renamed 'struct pid_nr' and now hold them in a statically sized array in 'struct pid' since the number of 'struct upid's for a process is known at process- creation time 2.6.21-mm2: - [Serge Hallyn] Terminate other processes in pid ns when reaper is exiting. Signed-off-by: Sukadev Bhattiprolu <sukadev@us.ibm.com> --- include/linux/pid.h | 3 include/linux/pid_namespace.h | 5 - init/Kconfig | 9 + kernel/exit.c | 14 ++ kernel/fork.c | 17 ++- kernel/pid.c | 209 ++++++++++++++++++++++++++++++++++++++---- 6 files changed, 227 insertions(+), 30 deletions(-) Index: lx26-21-mm2/kernel/pid.c =================================================================== --- lx26-21-mm2.orig/kernel/pid.c 2007-05-22 16:59:50.000000000 -0700 +++ lx26-21-mm2/kernel/pid.c 2007-05-22 16:59:52.000000000 -0700 @@ -32,6 +32,7 @@ #define pid_hashfn(nr) hash_long((unsigned long)nr, pidhash_shift) static struct hlist_head *pid_hash; static int pidhash_shift; +static struct kmem_cache *pid1_cachep; static struct kmem_cache *pid_cachep; struct upid init_struct_upid = INIT_STRUCT_UPID; struct pid init_struct_p…
creator: Sukadev Bhattiprolu
date: 2007-05-24T01:13:22-00:00

[RFC][PATCH 12/16] Terminate processes in a ns when reaper is exiting.
type: item
description: Subject: Terminate processes in a ns when reaper is exiting. From: Sukadev Bhattiprolu <sukadev@us.ibm.com> This should actually be considered a part of the previous patch which enables cloning of pid namespace. Its been separated out for easier review. Terminate all processes in a namespace when the reaper of the namespace is exiting. We do this by walking the pidmap of the namespace and sending SIGKILL to all processes. TODO: - Consider maintaining a per-pid namespace tasklist. Use that list to terminate processes in the namespace more efficiently. Such a tasklist may also be useful to freeze or checkpoint an application. Signed-off-by: Sukadev Bhattiprolu <sukadev@us.ibm.com> --- include/linux/pid.h | 1 + include/linux/pid_namespace.h | 1 + kernel/exit.c | 5 +++-- kernel/fork.c | 19 +++++++++++++++++++ kernel/pid.c | 42 ++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 66 insertions(+), 2 deletions(-) Index: lx26-21-mm2/include/linux/pid.h =================================================================== --- lx26-21-mm2.orig/include/linux/pid.h 2007-05-22 16:59:52.000000000 -0700 +++ lx26-21-mm2/include/linux/pid.h 2007-05-22 16:59:53.000000000 -0700 @@ -124,6 +124,7 @@ extern void FASTCALL(free_pid(struct pid extern pid_t pid_to_nr_in_ns(struct pid_namespace *ns, struct pid *pid); extern pid_t pid_to_nr(struct pid *pid); +extern void zap_pid_ns_processes(struct pid_namespace *pid_ns); #define do_each_pid_task(pid, type, task) \ do { \ Index: lx26-21-mm2/include/linux/pid_namespace.h =================================================================== --- lx26-21-mm2.orig/include/linux/pid_namespace.h 2007-05-22 16:59:52.000000000 -0700 +++ lx26-21-mm2/include/linux/pid_namespace.h 2007-05-22 16:59:53.000000000 -0700 @@ -32,6 +32,7 @@ struct pid_namespace { struct pidmap pidmap[PIDMAP_ENTRIES]; int last_pid; struct task_struct *child_reaper; + atomic_t terminating; }; extern struct pid_namespace init_pid_ns; Index: lx26-21-mm2/kernel/exit.c ===========…
creator: Sukadev Bhattiprolu
date: 2007-05-24T01:13:54-00:00

[RFC][PATCH 13/16] Remove proc_mnt's use for killing inodes
type: item
description: Subject: Remove proc_mnt's use for killing inodes From: Dave Hansen <hansendc@us.ibm.com> We use proc_mnt as a shortcut to find a superblock on which to go killing /proc inodes. This will break if we ever have more than one /proc superblock. So, use the superblock list to go find each of the /proc sb's and kill inodes on each superblock. This does introduce an extra lock grab from what was there before, but the list should be only 1 long 99% of the time, and we don't exactly remove proc entries in hot paths. Note that this *isn't* the path that we use to get rid of the actual /proc pid entries. Those are a different beast. Signed-off-by: Dave Hansen <haveblue@us.ibm.com> --- fs/proc/generic.c | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) Index: lx26-21-mm2/fs/proc/generic.c =================================================================== --- lx26-21-mm2.orig/fs/proc/generic.c 2007-05-22 16:51:19.000000000 -0700 +++ lx26-21-mm2/fs/proc/generic.c 2007-05-22 16:59:55.000000000 -0700 @@ -554,13 +554,10 @@ static int proc_register(struct proc_dir return 0; } -/* - * Kill an inode that got unregistered.. - */ -static void proc_kill_inodes(struct proc_dir_entry *de) +static void proc_kill_inodes_sb(struct proc_dir_entry *de, + struct super_block *sb) { struct list_head *p; - struct super_block *sb = proc_mnt->mnt_sb; /* * Actually it's a partial revoke(). @@ -584,6 +581,27 @@ static void proc_kill_inodes(struct proc file_list_unlock(); } +/* + * Kill an inode that got unregistered.. + */ +static void proc_kill_inodes(struct proc_dir_entry *de) +{ + struct list_head *l; + struct file_system_type *procfs; + + procfs = get_fs_type("proc"); + if (!procfs) + return; + + spin_lock(&sb_lock); + list_for_each(l, &procfs->fs_supers) { + struct super_block *sb; + sb = list_entry(l, struct super_block, s_instances); + proc_kill_inodes_sb(de, sb); + } + spin_unlock(&sb_lock); +} + static struct proc_dir_entry *proc_create(struct proc_dir_entry *…
creator: Sukadev Bhattiprolu
date: 2007-05-24T01:14:17-00:00

[RFC][PATCH 14/16] Introduce proc_mnt for pid_ns
type: item
description: Subject: Introduce proc_mnt for pid_ns From: Dave Hansen <hansendc@us.ibm.com> The following patch completes the removal of the global proc_mnt. It fetches the mnt on which to do dentry invalidations from the pid_namespace in which the task appears. For now, there is only one pid namespace in mainline so this is straightforward. In the -lxc tree we'll have to do something more complex. The proc_flush_task() code takes a task, and needs to be able to find the corresponding proc superblocks on which that tasks's /proc/<pid> directories could appear. We can tell in which pid namespaces a task appears, so I put a pointer from the pid namespace to the corresponding proc_mnt. /proc currently has some special code to make sure that the root directory gets set up correctly. It proc_mnt variable in order to find its way to the root inode. Signed-off-by: Dave Hansen <haveblue@us.ibm.com> Signed-off-by: Sukadev Bhattiprolu <sukadev@us.ibm.com> --- fs/proc/base.c | 32 ++++++++++++++++++++++++- fs/proc/inode.c | 11 +++++++- fs/proc/root.c | 52 ++++++++++++++++++++++++++++-------------- include/linux/pid_namespace.h | 1 include/linux/proc_fs.h | 1 5 files changed, 75 insertions(+), 22 deletions(-) Index: lx26-21-mm2/fs/proc/base.c =================================================================== --- lx26-21-mm2.orig/fs/proc/base.c 2007-05-22 16:59:49.000000000 -0700 +++ lx26-21-mm2/fs/proc/base.c 2007-05-22 16:59:56.000000000 -0700 @@ -2005,9 +2005,11 @@ static const struct inode_operations pro }; /** - * proc_flush_task - Remove dcache entries for @task from the /proc dcache. + * proc_flush_task_from_pid_ns - Remove dcache entries for @task + * from the /proc dcache. * * @task: task that should be flushed. + * @pid_ns: pid_namespace in which that task appears * * Looks in the dcache for * /proc/@pid @@ -2025,11 +2027,22 @@ static const struct inode_operations pro * that no dcache entries will exist at process exit time it * just makes it very unlikely that any will persist. */…
creator: Sukadev Bhattiprolu
date: 2007-05-24T01:14:42-00:00

[RFC][PATCH 15/16] Enable signaling child reaper from parent ns.
type: item
description: Subject: Enable signaling child reaper from parent ns. From: Sukadev Bhattiprolu <sukadev@us.ibm.com> The reaper of a child namespace must receive signals from its parent pid namespace but not receive any signals from its own namespace. This is a very early draft :-) and following tests seem to pass - Successfully kill child reaper from parent namespace (init_pid_ns) - Fail to kill child reaper from within its namespace (non init_pid_ns) - kill -1 1 from init_pid_ns seemed to work (rescanned inittab) TODO: - Test async io and SIGIO delivery. - Allow any legitimate signals that the child reaper can receive from within its namespace? (we block all signals now) - Sending SIGKILL to the child reaper of a namespace terminates the namespace But if the namespace remounted /proc from user space, /proc would remain mounted even after reaper and other process in the namespace go away. Signed-off-by: Sukadev Bhattiprolu <sukadev@us.ibm.com> --- kernel/signal.c | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) Index: lx26-21-mm2/kernel/signal.c =================================================================== --- lx26-21-mm2.orig/kernel/signal.c 2007-05-22 16:59:42.000000000 -0700 +++ lx26-21-mm2/kernel/signal.c 2007-05-22 16:59:57.000000000 -0700 @@ -507,6 +507,20 @@ static int check_kill_permission(int sig && !capable(CAP_KILL)) return error; + /* + * If t is the reaper of its namespace and someone from that + * namespace is trying to send a signal. + * + * Note: If some one from parent namespace is sending a signal, + * task_child_reaper() != t and we allow the signal. + * + * In the child namespace, does this block even legitimate signals + * like the ones telinit sends to /sbin/init ? + * + */ + if ((!is_global_init(t)) && (t == task_child_reaper(t))) + return -EPERM; + error = security_task_kill(t, info, sig, 0); if (!error) audit_signal_info(sig, t); /* Let audit system see the signal */ @@ -1910,7 +1924,13 @@ relock: /* * Init of a pid space …
creator: Sukadev Bhattiprolu
date: 2007-05-24T01:15:16-00:00

[RFC][PATCH 16/16] Move inline functions to sched.h
type: item
description: Subject: Move inline functions to sched.h From: Sukadev Bhattiprolu <sukadev@us.ibm.com> task_active_pid_ns() and pid_to_nr() cannot both be inline since they are currently defined in different files (pid_namespace.h and pid.h) These files depend on each other and sched.h and to resolve the circular dependency some functions have to be extern even if they all should really be inline. This patch moves all these functions to sched.h and makes them all inline. TODO: Fold these changes into the appropriate patches in the patchset. Any concern with adding these to sched.h ? Signed-off-by: Sukadev Bhattiprolu <sukadev@us.ibm.com> --- include/linux/pid.h | 2 include/linux/pid_namespace.h | 8 --- include/linux/sched.h | 92 ++++++++++++++++++++++++++++++++++++++++++ kernel/pid.c | 86 --------------------------------------- 4 files changed, 92 insertions(+), 96 deletions(-) Index: lx26-21-mm2/kernel/pid.c =================================================================== --- lx26-21-mm2.orig/kernel/pid.c 2007-05-23 13:25:56.000000000 -0700 +++ lx26-21-mm2/kernel/pid.c 2007-05-23 13:25:56.000000000 -0700 @@ -246,49 +246,6 @@ static int init_upid(struct upid *upid, return 0; } -static struct upid *pid_active_upid(struct pid *pid) -{ - return &pid->upid_list[0]; -} - -/* - * Return the active pid namespace of the process @pid. - * - * Note: - * To avoid having to use an extra pointer in struct pid to keep track - * of active pid namespace, dup_struct_pid() maintains the order of - * entries in 'pid->upid_list' such that the youngest (or the 'active') - * pid namespace is the first entry and oldest (init_pid_ns) is the last - * entry in the list. - */ -struct pid_namespace *pid_active_pid_ns(struct pid *pid) -{ - return pid_active_upid(pid)->pid_ns; -} -EXPORT_SYMBOL_GPL(pid_active_pid_ns); - -/* - * Return the parent pid_namespace of the active pid namespace of @tsk. - * - * Note: - * Refer to function header of pid_active_pid_ns() for information on - * the order of entries in…
creator: Sukadev Bhattiprolu
date: 2007-05-24T01:15:36-00:00

Re: [RFC][PATCH 04/16] Use pid_to_nr() in process info functions
type: item
description: > =================================================================== > --- lx26-21-mm2.orig/kernel/timer.c 2007-05-22 16:58:38.000000000 -0700 > +++ lx26-21-mm2/kernel/timer.c 2007-05-22 16:59:44.000000000 -0700 > @@ -945,7 +945,7 @@ asmlinkage long sys_getppid(void) > int pid; > > rcu_read_lock(); > - pid = rcu_dereference(current->real_parent)->tgid; > + pid = pid_to_nr(task_parent_tgid(current)); This breaks fsys_getppid() call in ia64... > rcu_read_unlock(); > > return pid; _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: xemul
date: 2007-05-24T08:22:25-00:00

Re: [RFC][PATCH 06/16] Define is_global_init()
type: item
description: > Index: lx26-21-mm2/kernel/pid.c > =================================================================== > --- lx26-21-mm2.orig/kernel/pid.c 2007-05-22 16:59:34.000000000 -0700 > +++ lx26-21-mm2/kernel/pid.c 2007-05-22 16:59:46.000000000 -0700 > @@ -71,6 +71,27 @@ struct pid_namespace init_pid_ns = { > .child_reaper = &init_task > }; > > + > +/** > + * is_global_init - check if a task structure is init > + * @tsk: Task structure to be checked. > + * > + * Check if a task structure is the first user space task the kernel created. > + */ > +int is_global_init(struct task_struct *tsk) > +{ > + return (task_active_pid_ns(tsk) == &init_pid_ns && tsk->pid == 1); This can OOPS if you pass arbitrary task to this call... tsk->nsproxy can already be NULL. > +} > + > +/* > + * is_container_init: > + * check whether in the task is init in it's own pid namespace. > + */ > +int is_container_init(struct task_struct *tsk) > +{ > + return tsk->pid == 1; > +} > + > /* > * Note: disable interrupts while the pidmap_lock is held as an > * interrupt might come in and do read_lock(&tasklist_lock). > _______________________________________________ > Containers mailing list > Containers@lists.linux-foundation.org > https://lists.linux-foundation.org/mailman/listinfo/containers > _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: xemul
date: 2007-05-24T08:28:59-00:00

Re: [RFC][PATCH 06/16] Define is_global_init()
type: item
description: > Index: lx26-21-mm2/kernel/pid.c > =================================================================== > --- lx26-21-mm2.orig/kernel/pid.c 2007-05-22 16:59:34.000000000 -0700 > +++ lx26-21-mm2/kernel/pid.c 2007-05-22 16:59:46.000000000 -0700 > @@ -71,6 +71,27 @@ struct pid_namespace init_pid_ns = { > .child_reaper = &init_task > }; > > + > +/** > + * is_global_init - check if a task structure is init > + * @tsk: Task structure to be checked. > + * > + * Check if a task structure is the first user space task the kernel created. > + */ > +int is_global_init(struct task_struct *tsk) > +{ > + return (task_active_pid_ns(tsk) == &init_pid_ns && tsk->pid == 1); This can OOPS if you pass arbitrary task to this call... tsk->nsproxy can already be NULL. > +} > + > +/* > + * is_container_init: > + * check whether in the task is init in it's own pid namespace. > + */ > +int is_container_init(struct task_struct *tsk) > +{ > + return tsk->pid == 1; > +} > + > /* > * Note: disable interrupts while the pidmap_lock is held as an > * interrupt might come in and do read_lock(&tasklist_lock). > _______________________________________________ > Containers mailing list > Containers@lists.linux-foundation.org > https://lists.linux-foundation.org/mailman/listinfo/containers > _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: xemul
date: 2007-05-24T08:29:10-00:00

Re: [RFC][PATCH 07/16] Move alloc_pid call to copy_process
type: item
description: > Index: lx26-21-mm2/kernel/pid.c > =================================================================== > --- lx26-21-mm2.orig/kernel/pid.c 2007-05-22 16:59:46.000000000 -0700 > +++ lx26-21-mm2/kernel/pid.c 2007-05-22 17:06:48.000000000 -0700 > @@ -216,6 +216,10 @@ fastcall void free_pid(struct pid *pid) > /* We can be called with write_lock_irq(&tasklist_lock) held */ > unsigned long flags; > > + /* check this here to keep copy_process() cleaner */ > + if (unlikely(pid == &init_struct_pid)) > + return; > + This looks ugly to me. That's the same as if we put if (ns == &init_pid_ns) return; in put_pid_ns() call. Such small struts of their own do not introduce any noticeable effect, but when we have them in many places (and on fast patsh like alloc_pid()) the performance hurts... > spin_lock_irqsave(&pidmap_lock, flags); > hlist_del_rcu(&pid->pid_chain); > spin_unlock_irqrestore(&pidmap_lock, flags); > @@ -224,12 +228,16 @@ fastcall void free_pid(struct pid *pid) > call_rcu(&pid->rcu, delayed_put_pid); > } > > -struct pid *alloc_pid(void) > +struct pid *alloc_pid(enum copy_process_type copy_src) > { > struct pid *pid; > enum pid_type type; > int nr = -1; > > + /* check this here to keep copy_process() cleaner */ > + if (unlikely(copy_src == COPY_IDLE_PROCESS)) > + return &init_struct_pid; > + > pid = kmem_cache_alloc(pid_cachep, GFP_KERNEL); > if (!pid) > goto out; > _______________________________________________ > Containers mailing list > Containers@lists.linux-foundation.org > https://lists.linux-foundation.org/mailman/listinfo/containers > _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: xemul
date: 2007-05-24T08:35:48-00:00

Re: [RFC][PATCH 08/16] Define/use pid->upid_list list.
type: item
description: > +/* > + * Return the pid_t by which the process @pid is known in the pid > + * namespace @ns. > + * > + * Return 0 if: > + * - @pid is NULL (eg: procfs calls this for task_pgrp(init_task) > + * which is NULL). > + * > + * - process does not have pid_t in the namespace @ns (eg: parent > + * process of a child reaper does not exist in the child namespace. > + * A getppid() call by the child reaper results in 0). > + */ > +pid_t pid_to_nr_in_ns(struct pid_namespace *ns, struct pid *pid) > +{ > + int i; > + struct upid *upid; > + > + if (!pid) > + return 0; > + > + upid = &pid->upid_list[0]; > + for (i = 0; i < pid->num_upids; i++, upid++) { > + if (upid->pid_ns == ns) > + return upid->nr; > + } This will make users of the kernel who do not need the pid namespaces suffer from performance loss. Why not introduce a CONFIG_PID_NS option? > + return 0; > +} > +EXPORT_SYMBOL_GPL(pid_to_nr_in_ns); > @@ -2182,6 +2185,8 @@ int proc_pid_readdir(struct file * filp, > struct task_struct *reaper = get_proc_task(filp->f_path.dentry->d_inode); > struct task_struct *task; > int tgid; > + /* TODO get pid_ns from proc mnt rather than current */ > + struct pid_namespace *ns = task_active_pid_ns(current); IMHO this is not a TODO, but MUSTDO. When you have one /proc mount accessed from different pid namespaces you may be in situation when one pidnr (like 123) represents different tasks in these namespaces and thus the inode staying behind the dentry with d_name "123" must reference two tasks. When we scale the model to N namespace inode must reference N tasks. But I don't see it in the patches... > > if (!reaper) > goto out_no_task; _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: xemul
date: 2007-05-24T08:57:33-00:00

Re: [RFC][PATCH 06/16] Define is_global_init()
type: item
description: Pavel Emelianov [xemul@sw.ru] wrote: | > Index: lx26-21-mm2/kernel/pid.c | > =================================================================== | > --- lx26-21-mm2.orig/kernel/pid.c 2007-05-22 16:59:34.000000000 -0700 | > +++ lx26-21-mm2/kernel/pid.c 2007-05-22 16:59:46.000000000 -0700 | > @@ -71,6 +71,27 @@ struct pid_namespace init_pid_ns = { | > .child_reaper = &init_task | > }; | > | > + | > +/** | > + * is_global_init - check if a task structure is init | > + * @tsk: Task structure to be checked. | > + * | > + * Check if a task structure is the first user space task the kernel created. | > + */ | > +int is_global_init(struct task_struct *tsk) | > +{ | > + return (task_active_pid_ns(tsk) == &init_pid_ns && tsk->pid == 1); | | This can OOPS if you pass arbitrary task to this call... | tsk->nsproxy can already be NULL. Hmm. You are right. btw, this could be a bisect issue. Patch 9 of uses pid_ns from pid->upid_list and removes nsproxy->pid_ns. | | > +} | > + | > +/* | > + * is_container_init: | > + * check whether in the task is init in it's own pid namespace. | > + */ | > +int is_container_init(struct task_struct *tsk) | > +{ | > + return tsk->pid == 1; | > +} | > + | > /* | > * Note: disable interrupts while the pidmap_lock is held as an | > * interrupt might come in and do read_lock(&tasklist_lock). | > _______________________________________________ | > Containers mailing list | > Containers@lists.linux-foundation.org | > https://lists.linux-foundation.org/mailman/listinfo/containers | > | > _______________________________________________ | > Devel mailing list | > Devel@openvz.org | > https://openvz.org/mailman/listinfo/devel | > _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Sukadev Bhattiprolu
date: 2007-05-24T09:10:52-00:00

Re: [RFC][PATCH 07/16] Move alloc_pid call to copy_process
type: item
description: Pavel Emelianov [xemul@sw.ru] wrote: | > Index: lx26-21-mm2/kernel/pid.c | > =================================================================== | > --- lx26-21-mm2.orig/kernel/pid.c 2007-05-22 16:59:46.000000000 -0700 | > +++ lx26-21-mm2/kernel/pid.c 2007-05-22 17:06:48.000000000 -0700 | > @@ -216,6 +216,10 @@ fastcall void free_pid(struct pid *pid) | > /* We can be called with write_lock_irq(&tasklist_lock) held */ | > unsigned long flags; | > | > + /* check this here to keep copy_process() cleaner */ | > + if (unlikely(pid == &init_struct_pid)) | > + return; | > + | | This looks ugly to me. I agree about the ugly part :-) but we need to distinguish between idle thread and normal thread at some point in do_fork(). | That's the same as if we put | if (ns == &init_pid_ns) | return; | in put_pid_ns() call. | | Such small struts of their own do not introduce any noticeable | effect, but when we have them in many places (and on fast patsh | like alloc_pid()) the performance hurts... I agree and we have been trying to keep the impact as low as possible. | | | > spin_lock_irqsave(&pidmap_lock, flags); | > hlist_del_rcu(&pid->pid_chain); | > spin_unlock_irqrestore(&pidmap_lock, flags); | > @@ -224,12 +228,16 @@ fastcall void free_pid(struct pid *pid) | > call_rcu(&pid->rcu, delayed_put_pid); | > } | > | > -struct pid *alloc_pid(void) | > +struct pid *alloc_pid(enum copy_process_type copy_src) | > { | > struct pid *pid; | > enum pid_type type; | > int nr = -1; | > | > + /* check this here to keep copy_process() cleaner */ | > + if (unlikely(copy_src == COPY_IDLE_PROCESS)) | > + return &init_struct_pid; | > + | > pid = kmem_cache_alloc(pid_cachep, GFP_KERNEL); | > if (!pid) | > goto out; | > _______________________________________________ | > Containers mailing list | > Containers@lists.linux-foundation.org | > https://lists.linux-foundation.org/mailman/listinfo/containers | > | > _______________________________________________ | > Devel mailing list | > Devel@openvz.org | …
creator: Sukadev Bhattiprolu
date: 2007-05-24T09:17:47-00:00

Re: [RFC][PATCH 14/16] Introduce proc_mnt for pid_ns
type: item
description: sukadev@us.ibm.com wrote: > Subject: Introduce proc_mnt for pid_ns > > From: Dave Hansen <hansendc@us.ibm.com> > > The following patch completes the removal of the global proc_mnt. > It fetches the mnt on which to do dentry invalidations from the > pid_namespace in which the task appears. > > For now, there is only one pid namespace in mainline so this is > straightforward. In the -lxc tree we'll have to do something > more complex. The proc_flush_task() code takes a task, and > needs to be able to find the corresponding proc superblocks on > which that tasks's /proc/<pid> directories could appear. We > can tell in which pid namespaces a task appears, so I put a > pointer from the pid namespace to the corresponding proc_mnt. > > /proc currently has some special code to make sure that the root > directory gets set up correctly. It proc_mnt variable in order > to find its way to the root inode. > > Signed-off-by: Dave Hansen <haveblue@us.ibm.com> > Signed-off-by: Sukadev Bhattiprolu <sukadev@us.ibm.com> > --- > > fs/proc/base.c | 32 ++++++++++++++++++++++++- > fs/proc/inode.c | 11 +++++++- > fs/proc/root.c | 52 ++++++++++++++++++++++++++++-------------- > include/linux/pid_namespace.h | 1 > include/linux/proc_fs.h | 1 > 5 files changed, 75 insertions(+), 22 deletions(-) > > Index: lx26-21-mm2/fs/proc/base.c > =================================================================== > --- lx26-21-mm2.orig/fs/proc/base.c 2007-05-22 16:59:49.000000000 -0700 > +++ lx26-21-mm2/fs/proc/base.c 2007-05-22 16:59:56.000000000 -0700 > @@ -2005,9 +2005,11 @@ static const struct inode_operations pro > }; > > /** > - * proc_flush_task - Remove dcache entries for @task from the /proc dcache. > + * proc_flush_task_from_pid_ns - Remove dcache entries for @task > + * from the /proc dcache. > * > * @task: task that should be flushed. > + * @pid_ns: pid_namespace in which that task appears > * > * Looks in the dcache for > * /proc/@pid > @@ -2025,11 +2027,22 @@ static const struct inode_operat…
creator: xemul
date: 2007-05-24T09:23:01-00:00

Re: [RFC][PATCH 06/16] Define is_global_init()
type: item
description: sukadev@us.ibm.com wrote: > Pavel Emelianov [xemul@sw.ru] wrote: > | > Index: lx26-21-mm2/kernel/pid.c > | > =================================================================== > | > --- lx26-21-mm2.orig/kernel/pid.c 2007-05-22 16:59:34.000000000 -0700 > | > +++ lx26-21-mm2/kernel/pid.c 2007-05-22 16:59:46.000000000 -0700 > | > @@ -71,6 +71,27 @@ struct pid_namespace init_pid_ns = { > | > .child_reaper = &init_task > | > }; > | > > | > + > | > +/** > | > + * is_global_init - check if a task structure is init > | > + * @tsk: Task structure to be checked. > | > + * > | > + * Check if a task structure is the first user space task the kernel created. > | > + */ > | > +int is_global_init(struct task_struct *tsk) > | > +{ > | > + return (task_active_pid_ns(tsk) == &init_pid_ns && tsk->pid == 1); > | > | This can OOPS if you pass arbitrary task to this call... > | tsk->nsproxy can already be NULL. > > Hmm. You are right. btw, this could be a bisect issue. Patch 9 of uses > pid_ns from pid->upid_list and removes nsproxy->pid_ns. Yes, but that patch is not good either. task_pid(tsk) may become NULL as well and this will oops. > | > | > +} > | > + > | > +/* > | > + * is_container_init: > | > + * check whether in the task is init in it's own pid namespace. > | > + */ > | > +int is_container_init(struct task_struct *tsk) > | > +{ > | > + return tsk->pid == 1; > | > +} > | > + > | > /* > | > * Note: disable interrupts while the pidmap_lock is held as an > | > * interrupt might come in and do read_lock(&tasklist_lock). > | > _______________________________________________ > | > Containers mailing list > | > Containers@lists.linux-foundation.org > | > https://lists.linux-foundation.org/mailman/listinfo/containers > | > > | > _______________________________________________ > | > Devel mailing list > | > Devel@openvz.org > | > https://openvz.org/mailman/listinfo/devel > | > > _______________________________________________ Containers mailing list Containers@lists.linux-foundation.or…
creator: xemul
date: 2007-05-24T09:24:54-00:00

Re: [RFC][PATCH 07/16] Move alloc_pid call to copy_process
type: item
description: sukadev@us.ibm.com wrote: > Pavel Emelianov [xemul@sw.ru] wrote: > | > Index: lx26-21-mm2/kernel/pid.c > | > =================================================================== > | > --- lx26-21-mm2.orig/kernel/pid.c 2007-05-22 16:59:46.000000000 -0700 > | > +++ lx26-21-mm2/kernel/pid.c 2007-05-22 17:06:48.000000000 -0700 > | > @@ -216,6 +216,10 @@ fastcall void free_pid(struct pid *pid) > | > /* We can be called with write_lock_irq(&tasklist_lock) held */ > | > unsigned long flags; > | > > | > + /* check this here to keep copy_process() cleaner */ > | > + if (unlikely(pid == &init_struct_pid)) > | > + return; > | > + > | > | This looks ugly to me. > > I agree about the ugly part :-) but we need to distinguish > between idle thread and normal thread at some point in do_fork(). Why not keep this as it was - pass the pid from do_fork() or do_fork_idle(). Why is that bad? > | That's the same as if we put > | if (ns == &init_pid_ns) > | return; > | in put_pid_ns() call. > | > | Such small struts of their own do not introduce any noticeable > | effect, but when we have them in many places (and on fast patsh > | like alloc_pid()) the performance hurts... > > I agree and we have been trying to keep the impact as low as possible. In this patches - yes. But when we have many patches with sucn "hooks" this becomes noticeable and hard to debug. > | > | > | > spin_lock_irqsave(&pidmap_lock, flags); > | > hlist_del_rcu(&pid->pid_chain); > | > spin_unlock_irqrestore(&pidmap_lock, flags); > | > @@ -224,12 +228,16 @@ fastcall void free_pid(struct pid *pid) > | > call_rcu(&pid->rcu, delayed_put_pid); > | > } > | > > | > -struct pid *alloc_pid(void) > | > +struct pid *alloc_pid(enum copy_process_type copy_src) > | > { > | > struct pid *pid; > | > enum pid_type type; > | > int nr = -1; > | > > | > + /* check this here to keep copy_process() cleaner */ > | > + if (unlikely(copy_src == COPY_IDLE_PROCESS)) > | > + return &init_struct_pid; > | > + > | > pid = kmem_cache_alloc(pid_cachep, …
creator: xemul
date: 2007-05-24T09:30:09-00:00

Re: [RFC][PATCH 08/16] Define/use pid->upid_list list.
type: item
description: Pavel Emelianov [xemul@sw.ru] wrote: | > +/* | > + * Return the pid_t by which the process @pid is known in the pid | > + * namespace @ns. | > + * | > + * Return 0 if: | > + * - @pid is NULL (eg: procfs calls this for task_pgrp(init_task) | > + * which is NULL). | > + * | > + * - process does not have pid_t in the namespace @ns (eg: parent | > + * process of a child reaper does not exist in the child namespace. | > + * A getppid() call by the child reaper results in 0). | > + */ | > +pid_t pid_to_nr_in_ns(struct pid_namespace *ns, struct pid *pid) | > +{ | > + int i; | > + struct upid *upid; | > + | > + if (!pid) | > + return 0; | > + | > + upid = &pid->upid_list[0]; | > + for (i = 0; i < pid->num_upids; i++, upid++) { | > + if (upid->pid_ns == ns) | > + return upid->nr; | > + } | | This will make users of the kernel who do not need the | pid namespaces suffer from performance loss. Why not | introduce a CONFIG_PID_NS option? CONFIG_PID_NS is in the next patch which actually enables cloning pid namespace. But this code is common and we are trying to optimize. Patch 16 makes this an inline function (plan to fold that diff into this if there are no concerns with adding to sched.h). Also, when number of pid namespaces is 1, the 'upid_list' is allocated adjacent to the 'struct pid' (pls see alloc_struct_pid() for the case num_upids == 1) | | > + return 0; | > +} | > +EXPORT_SYMBOL_GPL(pid_to_nr_in_ns); | > @@ -2182,6 +2185,8 @@ int proc_pid_readdir(struct file * filp, | > struct task_struct *reaper = get_proc_task(filp->f_path.dentry->d_inode); | > struct task_struct *task; | > int tgid; | > + /* TODO get pid_ns from proc mnt rather than current */ | > + struct pid_namespace *ns = task_active_pid_ns(current); | | IMHO this is not a TODO, but MUSTDO. | When you have one /proc mount accessed from different pid | namespaces you may be in situation when one pidnr (like 123) | represents different tasks in these namespaces and thus the | inode staying behind the dentry with …
creator: Sukadev Bhattiprolu
date: 2007-05-24T09:31:20-00:00

Re: [RFC][PATCH 0/16] Enable cloning of pid namespace
type: item
description: I haven't found an appropriate patch, so I ask the question here. Consider we're sending a signal from parent namespace to one of the child namespaces. What pid will be put in siginfo as si_pid? Pavel. _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: xemul
date: 2007-05-24T09:31:50-00:00

Re: [RFC][PATCH 07/16] Move alloc_pid call to copy_process
type: item
description: Pavel Emelianov [xemul@sw.ru] wrote: | sukadev@us.ibm.com wrote: | > Pavel Emelianov [xemul@sw.ru] wrote: | > | > Index: lx26-21-mm2/kernel/pid.c | > | > =================================================================== | > | > --- lx26-21-mm2.orig/kernel/pid.c 2007-05-22 16:59:46.000000000 -0700 | > | > +++ lx26-21-mm2/kernel/pid.c 2007-05-22 17:06:48.000000000 -0700 | > | > @@ -216,6 +216,10 @@ fastcall void free_pid(struct pid *pid) | > | > /* We can be called with write_lock_irq(&tasklist_lock) held */ | > | > unsigned long flags; | > | > | > | > + /* check this here to keep copy_process() cleaner */ | > | > + if (unlikely(pid == &init_struct_pid)) | > | > + return; | > | > + | > | | > | This looks ugly to me. | > | > I agree about the ugly part :-) but we need to distinguish | > between idle thread and normal thread at some point in do_fork(). | | Why not keep this as it was - pass the pid from do_fork() or | do_fork_idle(). Why is that bad? When CLONE_NEWPID flag is specified and we create a new pid ns we need the task_struct that will become the child reaper for the new ns. The task_struct is allocated in copy_process(). We could make a second check for CLONE_NEWPID in copy_process() and then initialize the reaper for the ns, but that could have same or more performance impact ? | | > | That's the same as if we put | > | if (ns == &init_pid_ns) | > | return; | > | in put_pid_ns() call. | > | | > | Such small struts of their own do not introduce any noticeable | > | effect, but when we have them in many places (and on fast patsh | > | like alloc_pid()) the performance hurts... | > | > I agree and we have been trying to keep the impact as low as possible. | | In this patches - yes. But when we have many patches with sucn | "hooks" this becomes noticeable and hard to debug. | | > | | > | | > | > spin_lock_irqsave(&pidmap_lock, flags); | > | > hlist_del_rcu(&pid->pid_chain); | > | > spin_unlock_irqrestore(&pidmap_lock, flags); | > | > @@ -224,12 +228,16 @@ fast…
creator: Sukadev Bhattiprolu
date: 2007-05-24T09:42:41-00:00

Re: [RFC][PATCH 14/16] Introduce proc_mnt for pid_ns
type: item
description: sukadev@us.ibm.com wrote: > Subject: Introduce proc_mnt for pid_ns > > From: Dave Hansen <hansendc@us.ibm.com> > > The following patch completes the removal of the global proc_mnt. > It fetches the mnt on which to do dentry invalidations from the > pid_namespace in which the task appears. > > For now, there is only one pid namespace in mainline so this is > straightforward. In the -lxc tree we'll have to do something > more complex. The proc_flush_task() code takes a task, and > needs to be able to find the corresponding proc superblocks on > which that tasks's /proc/<pid> directories could appear. We > can tell in which pid namespaces a task appears, so I put a > pointer from the pid namespace to the corresponding proc_mnt. > > /proc currently has some special code to make sure that the root > directory gets set up correctly. It proc_mnt variable in order > to find its way to the root inode. > > Signed-off-by: Dave Hansen <haveblue@us.ibm.com> > Signed-off-by: Sukadev Bhattiprolu <sukadev@us.ibm.com> > --- > > fs/proc/base.c | 32 ++++++++++++++++++++++++- > fs/proc/inode.c | 11 +++++++- > fs/proc/root.c | 52 ++++++++++++++++++++++++++++-------------- > include/linux/pid_namespace.h | 1 > include/linux/proc_fs.h | 1 > 5 files changed, 75 insertions(+), 22 deletions(-) > > Index: lx26-21-mm2/fs/proc/base.c > =================================================================== > --- lx26-21-mm2.orig/fs/proc/base.c 2007-05-22 16:59:49.000000000 -0700 > +++ lx26-21-mm2/fs/proc/base.c 2007-05-22 16:59:56.000000000 -0700 > @@ -2005,9 +2005,11 @@ static const struct inode_operations pro > }; > > /** > - * proc_flush_task - Remove dcache entries for @task from the /proc dcache. > + * proc_flush_task_from_pid_ns - Remove dcache entries for @task > + * from the /proc dcache. > * > * @task: task that should be flushed. > + * @pid_ns: pid_namespace in which that task appears > * > * Looks in the dcache for > * /proc/@pid > @@ -2025,11 +2027,22 @@ static const struct inode_operat…
creator: xemul
date: 2007-05-24T10:15:33-00:00

Re: [RFC][PATCH 0/16] Enable cloning of pid namespace
type: item
description: Pavel Emelianov wrote: > I haven't found an appropriate patch, so I ask the > question here. > > Consider we're sending a signal from parent namespace > to one of the child namespaces. What pid will be put > in siginfo as si_pid? Would 0 be an issue ? it seems reasonable to pretend that the signal was sent from the kernel and also set si_code to SI_KERNEL. C. _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Cedric Le Goater
date: 2007-05-24T10:19:46-00:00

Re: [RFC][PATCH 11/16] Enable cloning pid namespace
type: item
description: Quoting sukadev@us.ibm.com (sukadev@us.ibm.com): > > Subject: Enable cloning pid namespace > > From: Sukadev Bhattiprolu <sukadev@us.ibm.com> > > > When clone() is invoked with CLONE_NEWPID, create a new pid namespace > and then create a new struct pid for the new process. Allocate pid_t's > for the new process in the new pid namespace and all ancestor pid > namespaces. Make the newly cloned process the session and process group > leader. > > Since the active pid namespace is special and expected to be the first > entry in pid->upid_list, preserve the order of pid namespaces when > cloning without CLONE_NEWPID. > > TODO (partial list:) > > - Identify clone flags that should not be specified with CLONE_NEWPID > and return -EINVAL from copy_process(), if they are specified. (eg: > CLONE_THREAD|CLONE_NEWPID ?) > - Add a privilege check for CLONE_NEWPID > > Changelog: > > 2.6.21-mm2-pidns3: > - 'struct upid' used to be called 'struct pid_nr' and a list of these > were hanging off of 'struct pid'. So, we renamed 'struct pid_nr' > and now hold them in a statically sized array in 'struct pid' since > the number of 'struct upid's for a process is known at process- > creation time > > 2.6.21-mm2: > - [Serge Hallyn] Terminate other processes in pid ns when reaper is > exiting. > Signed-off-by: Sukadev Bhattiprolu <sukadev@us.ibm.com> > --- > include/linux/pid.h | 3 > include/linux/pid_namespace.h | 5 - > init/Kconfig | 9 + > kernel/exit.c | 14 ++ > kernel/fork.c | 17 ++- > kernel/pid.c | 209 ++++++++++++++++++++++++++++++++++++++---- > 6 files changed, 227 insertions(+), 30 deletions(-) > > Index: lx26-21-mm2/kernel/pid.c > =================================================================== > --- lx26-21-mm2.orig/kernel/pid.c 2007-05-22 16:59:50.000000000 -0700 > +++ lx26-21-mm2/kernel/pid.c 2007-05-22 16:59:52.000000000 -0700 > @@ -32,6 +32,7 @@ > #define pid_hashfn(nr) hash_long((unsigned long)nr, pidhash_shift) > static struct hlist_head *pid_hash; > static int pidhash_shi…
creator: serue
date: 2007-05-24T14:59:32-00:00

Re: [RFC][PATCH 06/16] Define is_global_init()
type: item
description: Quoting sukadev@us.ibm.com (sukadev@us.ibm.com): > > Subject: Define is_global_init() > > From: Serge E. Hallyn <serue@us.ibm.com> > > > is_init() is an ambiguous name for the pid==1 check. Split it into > is_global_init() and is_container_init(). > > A container init has it's tsk->pid == 1. > > A global init also has it's tsk->pid == 1, and it's active pid namespace > is the init_pid_ns. > > Changelog: > > 2.6.21-mm2-pidns2: > > - [Sukadev Bhattiprolu] Changed is_container_init() calls in {powerpc, > ppc,avr32}/traps.c for the _exception() call to is_global_init(). > This way, we kill only the container if the container's init has a > bug rather than force a kernel panic. > > Signed-off-by: Serge E. Hallyn <serue@us.ibm.com> > Signed-off-by: Sukadev Bhattiprolu <sukadev@us.ibm.com> > --- > arch/alpha/mm/fault.c | 2 +- > arch/arm/mm/fault.c | 2 +- > arch/arm26/mm/fault.c | 2 +- > arch/avr32/kernel/traps.c | 2 +- > arch/avr32/mm/fault.c | 6 +++--- > arch/i386/lib/usercopy.c | 2 +- > arch/i386/mm/fault.c | 2 +- > arch/ia64/mm/fault.c | 2 +- > arch/m32r/mm/fault.c | 2 +- > arch/m68k/mm/fault.c | 2 +- > arch/mips/mm/fault.c | 2 +- > arch/powerpc/kernel/traps.c | 2 +- > arch/powerpc/mm/fault.c | 2 +- > arch/powerpc/platforms/pseries/ras.c | 2 +- > arch/ppc/kernel/traps.c | 2 +- > arch/ppc/mm/fault.c | 2 +- > arch/s390/lib/uaccess_pt.c | 2 +- > arch/s390/mm/fault.c | 2 +- > arch/sh/mm/fault.c | 2 +- > arch/sh64/mm/fault.c | 6 +++--- > arch/um/kernel/trap.c | 2 +- > arch/x86_64/mm/fault.c | 4 ++-- > arch/xtensa/mm/fault.c | 2 +- > drivers/char/sysrq.c | 2 +- > include/linux/sched.h | 13 +++---------- > kernel/capability.c | 3 ++- > kernel/exit.c | 2 +- > kernel/kexec.c | 2 +- > kernel/pid.c | 21 +++++++++++++++++++++ > kernel/sysctl.c | 2 +- > mm/oom_kill.c | 4 ++-- > security/commoncap.c | 2 +- > 32 files changed, 61 insertions(+), 46 deletions(-) > > Index: lx26-21-mm2/arch/alpha/mm/fault.c > =================================================================== > --- lx26-…
creator: serue
date: 2007-05-24T15:27:43-00:00

Re: [RFC][PATCH 15/16] Enable signaling child reaper from parent ns.
type: item
description: Quoting sukadev@us.ibm.com (sukadev@us.ibm.com): > > Subject: Enable signaling child reaper from parent ns. > > From: Sukadev Bhattiprolu <sukadev@us.ibm.com> > > The reaper of a child namespace must receive signals from its parent pid > namespace but not receive any signals from its own namespace. > > This is a very early draft :-) and following tests seem to pass > > - Successfully kill child reaper from parent namespace (init_pid_ns) > > - Fail to kill child reaper from within its namespace (non init_pid_ns) > > - kill -1 1 from init_pid_ns seemed to work (rescanned inittab) > > TODO: > - Test async io and SIGIO delivery. > > - Allow any legitimate signals that the child reaper can receive > from within its namespace? (we block all signals now) > > - Sending SIGKILL to the child reaper of a namespace terminates the > namespace But if the namespace remounted /proc from user space, > /proc would remain mounted even after reaper and other process in > the namespace go away. > > Signed-off-by: Sukadev Bhattiprolu <sukadev@us.ibm.com> > --- > kernel/signal.c | 22 +++++++++++++++++++++- > 1 file changed, 21 insertions(+), 1 deletion(-) > > Index: lx26-21-mm2/kernel/signal.c > =================================================================== > --- lx26-21-mm2.orig/kernel/signal.c 2007-05-22 16:59:42.000000000 -0700 > +++ lx26-21-mm2/kernel/signal.c 2007-05-22 16:59:57.000000000 -0700 > @@ -507,6 +507,20 @@ static int check_kill_permission(int sig > && !capable(CAP_KILL)) > return error; > > + /* > + * If t is the reaper of its namespace and someone from that > + * namespace is trying to send a signal. > + * > + * Note: If some one from parent namespace is sending a signal, > + * task_child_reaper() != t and we allow the signal. > + * > + * In the child namespace, does this block even legitimate signals > + * like the ones telinit sends to /sbin/init ? > + * > + */ > + if ((!is_global_init(t)) && (t == task_child_reaper(t))) Couldn't you more clearly achieve what you w…
creator: serue
date: 2007-05-24T15:59:40-00:00

Re: [RFC][PATCH 06/16] Define is_global_init()
type: item
description: On Thu, 2007-05-24 at 13:24 +0400, Pavel Emelianov wrote: > > | > +int is_global_init(struct task_struct *tsk) > > | > +{ > > | > + return (task_active_pid_ns(tsk) == &init_pid_ns && tsk->pid == 1); > > | > > | This can OOPS if you pass arbitrary task to this call... > > | tsk->nsproxy can already be NULL. > > > > Hmm. You are right. btw, this could be a bisect issue. Patch 9 of uses > > pid_ns from pid->upid_list and removes nsproxy->pid_ns. > > Yes, but that patch is not good either. > task_pid(tsk) may become NULL as well and this will oops. Have you reviewed the call paths to make sure this can actually happen in practice? This just seems like another one of those racing-with-task-exit races. Shouldn't be too invasive to solve. -- Dave _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Dave Hansen
date: 2007-05-24T16:37:18-00:00

Re: [RFC][PATCH 14/16] Introduce proc_mnt for pid_ns
type: item
description: On Thu, 2007-05-24 at 14:15 +0400, Pavel Emelianov wrote: > > > s->s_flags |= MS_NODIRATIME | MS_NOSUID | MS_NOEXEC; > > @@ -466,6 +467,7 @@ int proc_fill_super(struct super_block * > > s->s_magic = PROC_SUPER_MAGIC; > > s->s_op = &proc_sops; > > s->s_time_gran = 1; > > + s->s_fs_info = pid_ns; > > One more thing I've just noticed - you don't get the namespace > here so after all the tasks die and namespace is freed we > have a proc mount pointing to freed namespace... Yep, missed reference count. Thanks for catching this! -- Dave _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Dave Hansen
date: 2007-05-24T16:51:23-00:00

Re: [RFC][PATCH 06/16] Define is_global_init()
type: item
description: Dave Hansen wrote: > On Thu, 2007-05-24 at 13:24 +0400, Pavel Emelianov wrote: >>> | > +int is_global_init(struct task_struct *tsk) >>> | > +{ >>> | > + return (task_active_pid_ns(tsk) == &init_pid_ns && tsk->pid == 1); >>> | >>> | This can OOPS if you pass arbitrary task to this call... >>> | tsk->nsproxy can already be NULL. >>> >>> Hmm. You are right. btw, this could be a bisect issue. Patch 9 of uses >>> pid_ns from pid->upid_list and removes nsproxy->pid_ns. >> Yes, but that patch is not good either. >> task_pid(tsk) may become NULL as well and this will oops. > > Have you reviewed the call paths to make sure this can actually happen > in practice? > > This just seems like another one of those racing-with-task-exit races. > Shouldn't be too invasive to solve. It is, but if we make patch that OOPSes the kernel in 0.1% of cases and we do know this - this MUST be fixed. > -- Dave > > _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Pavel Emelianov
date: 2007-05-25T06:39:28-00:00

Re: [RFC][PATCH 07/16] Move alloc_pid call to copy_process
type: item
description: sukadev@us.ibm.com wrote: > Subject: Move alloc_pid call to copy_process > > From: Sukadev Bhattiprolu <sukadev@us.ibm.com> > > Move alloc_pid() into copy_process(). This will keep all pid and pid > namespace code together and simplify error handling when we support > multiple pid namespaces. I haven't found this in patches, so I ask it here: We clone a new task with CLONE_NEWPIDS flag. This task allocates its PIDTYPE_PID pid and this pid happens in booth parent and child namespace. This is OK. Then new task attaches PIDTYPE_SID and PIDTYPE_PGID pids from parent task. But these ones are in parent namespace only. Right? Is that good? > Changelog: > - [Eric Biederman] Move the check of copy_process_type to alloc_pid()/ > free_pid() and to avoid clutter in copy_process(). > > Signed-off-by: Sukadev Bhattiprolu <sukadev@us.ibm.com> > --- > include/linux/pid.h | 7 ++++++- > kernel/fork.c | 21 +++++++++++++-------- > kernel/pid.c | 10 +++++++++- > 3 files changed, 28 insertions(+), 10 deletions(-) > > Index: lx26-21-mm2/include/linux/pid.h > =================================================================== > --- lx26-21-mm2.orig/include/linux/pid.h 2007-05-22 16:59:40.000000000 -0700 > +++ lx26-21-mm2/include/linux/pid.h 2007-05-22 17:06:48.000000000 -0700 > @@ -3,6 +3,11 @@ > > #include <linux/rcupdate.h> > > +enum copy_process_type { > + COPY_NON_IDLE_PROCESS, > + COPY_IDLE_PROCESS, > +}; > + > enum pid_type > { > PIDTYPE_PID, > @@ -95,7 +100,7 @@ extern struct pid *FASTCALL(find_pid(int > extern struct pid *find_get_pid(int nr); > extern struct pid *find_ge_pid(int nr); > > -extern struct pid *alloc_pid(void); > +extern struct pid *alloc_pid(enum copy_process_type); > extern void FASTCALL(free_pid(struct pid *pid)); > > static inline pid_t pid_to_nr(struct pid *pid) > Index: lx26-21-mm2/kernel/fork.c > =================================================================== > --- lx26-21-mm2.orig/kernel/fork.c 2007-05-22 16:59:41.000000000 -0700 > +++ lx26-21-mm2/kernel/f…
creator: Pavel Emelianov
date: 2007-05-25T06:41:58-00:00

Re: [RFC][PATCH 14/16] Introduce proc_mnt for pid_ns
type: item
description: Dave Hansen wrote: > On Thu, 2007-05-24 at 14:15 +0400, Pavel Emelianov wrote: >>> s->s_flags |= MS_NODIRATIME | MS_NOSUID | MS_NOEXEC; >>> @@ -466,6 +467,7 @@ int proc_fill_super(struct super_block * >>> s->s_magic = PROC_SUPER_MAGIC; >>> s->s_op = &proc_sops; >>> s->s_time_gran = 1; >>> + s->s_fs_info = pid_ns; >> One more thing I've just noticed - you don't get the namespace >> here so after all the tasks die and namespace is freed we >> have a proc mount pointing to freed namespace... > > Yep, missed reference count. Thanks for catching this! refcount is not the only badness. As I said you have race in testing superblock's namespace and setting it and one more: do you use one struct proc_dir_entry proc_root for all the superolocks? > -- Dave > > _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Pavel Emelianov
date: 2007-05-25T12:27:54-00:00

Re: [RFC][PATCH 06/16] Define is_global_init()
type: item
description: On Fri, 2007-05-25 at 10:39 +0400, Pavel Emelianov wrote: > > It is, but if we make patch that OOPSes the kernel in 0.1% > of cases and we do know this - this MUST be fixed. Oh, yeah, it certainly needs to be fixed. I just wanted to make sure you weren't seeing some endemic problem with the patches. -- Dave _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Dave Hansen
date: 2007-05-25T16:18:49-00:00

Re: [RFC][PATCH 14/16] Introduce proc_mnt for pid_ns
type: item
description: On Fri, 2007-05-25 at 16:27 +0400, Pavel Emelianov wrote: > > refcount is not the only badness. As I said you have > race in testing superblock's namespace and setting it > and one more: do you use one struct proc_dir_entry > proc_root for all the superolocks? Yep. Do you see some problems with that? -- Dave _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Dave Hansen
date: 2007-05-25T16:21:16-00:00

Re: [RFC][PATCH 15/16] Enable signaling child reaper from parent ns.
type: item
description: Quoting sukadev@us.ibm.com (sukadev@us.ibm.com): > > Subject: Enable signaling child reaper from parent ns. > > From: Sukadev Bhattiprolu <sukadev@us.ibm.com> > > The reaper of a child namespace must receive signals from its parent pid > namespace but not receive any signals from its own namespace. > > This is a very early draft :-) and following tests seem to pass > > - Successfully kill child reaper from parent namespace (init_pid_ns) > > - Fail to kill child reaper from within its namespace (non init_pid_ns) > > - kill -1 1 from init_pid_ns seemed to work (rescanned inittab) > > TODO: > - Test async io and SIGIO delivery. > > - Allow any legitimate signals that the child reaper can receive > from within its namespace? (we block all signals now) > > - Sending SIGKILL to the child reaper of a namespace terminates the > namespace But if the namespace remounted /proc from user space, > /proc would remain mounted even after reaper and other process in > the namespace go away. > > Signed-off-by: Sukadev Bhattiprolu <sukadev@us.ibm.com> > --- > kernel/signal.c | 22 +++++++++++++++++++++- > 1 file changed, 21 insertions(+), 1 deletion(-) > > Index: lx26-21-mm2/kernel/signal.c > =================================================================== > --- lx26-21-mm2.orig/kernel/signal.c 2007-05-22 16:59:42.000000000 -0700 > +++ lx26-21-mm2/kernel/signal.c 2007-05-22 16:59:57.000000000 -0700 > @@ -507,6 +507,20 @@ static int check_kill_permission(int sig > && !capable(CAP_KILL)) > return error; > > + /* > + * If t is the reaper of its namespace and someone from that > + * namespace is trying to send a signal. > + * > + * Note: If some one from parent namespace is sending a signal, > + * task_child_reaper() != t and we allow the signal. > + * > + * In the child namespace, does this block even legitimate signals > + * like the ones telinit sends to /sbin/init ? > + * > + */ > + if ((!is_global_init(t)) && (t == task_child_reaper(t))) > + return -EPERM; Ok, let's just go over th…
creator: serue
date: 2007-05-25T20:13:33-00:00

Re: [RFC][PATCH 06/16] Define is_global_init()
type: item
description: Dave Hansen [hansendc@us.ibm.com] wrote: | On Thu, 2007-05-24 at 13:24 +0400, Pavel Emelianov wrote: | > > | > +int is_global_init(struct task_struct *tsk) | > > | > +{ | > > | > + return (task_active_pid_ns(tsk) == &init_pid_ns && tsk->pid == 1); | > > | | > > | This can OOPS if you pass arbitrary task to this call... | > > | tsk->nsproxy can already be NULL. | > > | > > Hmm. You are right. btw, this could be a bisect issue. Patch 9 of uses | > > pid_ns from pid->upid_list and removes nsproxy->pid_ns. | > | > Yes, but that patch is not good either. | > task_pid(tsk) may become NULL as well and this will oops. | | Have you reviewed the call paths to make sure this can actually happen | in practice? task_pid() can be NULL when we are tearing down the task structure in release_task() and in the tiny window between detach_pid() and attach_pid() in de_thread(). I think task_pid() is safe as long as it is called for 'current'. (we should probably add some comments) I will double check my code, but I think all my calls to task_pid() and hence, to task_active_pid_ns() are safe, except for two cases: a) is_global_init(). There are a few calls to process other than current, but not sure if they are a problem. For instance in current code, unhandled_signal() checks tsk->pid == 1 and proceeds to derefernce tsk->sighand. If task_pid() is NULL because the task was in release_task(), then so is tsk->sighand. b) the temporary check I added in check_kill_permissions(). (I need to address Serge's comment here anyway). To make is_global_init() more efficient and independent of task_pid(), can we steal a bit from task_struct->flags ? Like PF_KSWAPD, and there are unused bits :-) | | This just seems like another one of those racing-with-task-exit races. | Shouldn't be too invasive to solve. A little invasive approach for the release_task() case could be to remove the 'struct pid' from the hash table, but leave it attached to the 'task_struct' till the 'task_struct' itself is freed. Rem…
creator: Sukadev Bhattiprolu
date: 2007-05-25T20:44:45-00:00

Re: [RFC][PATCH 04/16] Use pid_to_nr() in process info functions
type: item
description: Pavel Emelianov [xemul@sw.ru] wrote: | > =================================================================== | > --- lx26-21-mm2.orig/kernel/timer.c 2007-05-22 16:58:38.000000000 -0700 | > +++ lx26-21-mm2/kernel/timer.c 2007-05-22 16:59:44.000000000 -0700 | > @@ -945,7 +945,7 @@ asmlinkage long sys_getppid(void) | > int pid; | > | > rcu_read_lock(); | > - pid = rcu_dereference(current->real_parent)->tgid; | > + pid = pid_to_nr(task_parent_tgid(current)); | | This breaks fsys_getppid() call in ia64... Good catch. I need to figure out how to make the pid_to_nr() call in IA64 assembly and if there are other architectures that implement in assembly. But in general, do you see any problems with the interfaces like task_parent_tgid() ? | | > rcu_read_unlock(); | > | > return pid; _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Sukadev Bhattiprolu
date: 2007-05-25T22:53:18-00:00

Re: [RFC][PATCH 07/16] Move alloc_pid call to copy_process
type: item
description: Pavel Emelianov [xemul@openvz.org] wrote: | sukadev@us.ibm.com wrote: | > Subject: Move alloc_pid call to copy_process | > | > From: Sukadev Bhattiprolu <sukadev@us.ibm.com> | > | > Move alloc_pid() into copy_process(). This will keep all pid and pid | > namespace code together and simplify error handling when we support | > multiple pid namespaces. | | I haven't found this in patches, so I ask it here: | | We clone a new task with CLONE_NEWPIDS flag. This task | allocates its PIDTYPE_PID pid and this pid happens in | booth parent and child namespace. This is OK. | Then new task attaches PIDTYPE_SID and PIDTYPE_PGID pids from | parent task. But these ones are in parent namespace only. | | Right? Is that good? | In this patch, yes, we still attach to the parent process, bc at this point we still support only one namespace. In the patch that actually allows creating multiple namespaces, (Patch #11), I have the following code which makes the process that cloned its pid ns a session and pgrp leader, just like /sbin/init for init_pid_ns. @@ -1255,11 +1254,17 @@ static struct task_struct *copy_process( __ptrace_link(p, current->parent); if (thread_group_leader(p)) { + struct pid *pgrp = task_pgrp(current); + struct pid *session = task_session(current); + + if (clone_flags & CLONE_NEWPID) + pgrp = session = pid; + p->signal->tty = current->signal->tty; - p->signal->pgrp = process_group(current); - set_signal_session(p->signal, process_session(current)); - attach_pid(p, PIDTYPE_PGID, task_pgrp(current)); - attach_pid(p, PIDTYPE_SID, task_session(current)); + p->signal->pgrp = pid_to_nr(pgrp); + set_signal_session(p->signal, pid_to_nr(session)); + attach_pid(p, PIDTYPE_PGID, pgrp); + attach_pid(p, PIDTYPE_SID, session); list_add_tail_rcu(&p->tasks, &init_task.tasks); __get_cpu_var(process_counts)++; | > Changelog: | > - [Eric Biederman] Move the check of copy_process_type to alloc_pid()/ | > free_pid() and to avoid clutter in copy_process(). | > | > Signed-off-by: Sukadev Bh…
creator: Sukadev Bhattiprolu
date: 2007-05-25T23:01:59-00:00

Re: [RFC][PATCH 15/16] Enable signaling child reaper from parent ns.
type: item
description: Serge E. Hallyn [serue@us.ibm.com] wrote: | Quoting sukadev@us.ibm.com (sukadev@us.ibm.com): | > | > Subject: Enable signaling child reaper from parent ns. | > | > From: Sukadev Bhattiprolu <sukadev@us.ibm.com> | > | > The reaper of a child namespace must receive signals from its parent pid | > namespace but not receive any signals from its own namespace. | > | > This is a very early draft :-) and following tests seem to pass | > | > - Successfully kill child reaper from parent namespace (init_pid_ns) | > | > - Fail to kill child reaper from within its namespace (non init_pid_ns) | > | > - kill -1 1 from init_pid_ns seemed to work (rescanned inittab) | > | > TODO: | > - Test async io and SIGIO delivery. | > | > - Allow any legitimate signals that the child reaper can receive | > from within its namespace? (we block all signals now) | > | > - Sending SIGKILL to the child reaper of a namespace terminates the | > namespace But if the namespace remounted /proc from user space, | > /proc would remain mounted even after reaper and other process in | > the namespace go away. | > | > Signed-off-by: Sukadev Bhattiprolu <sukadev@us.ibm.com> | > --- | > kernel/signal.c | 22 +++++++++++++++++++++- | > 1 file changed, 21 insertions(+), 1 deletion(-) | > | > Index: lx26-21-mm2/kernel/signal.c | > =================================================================== | > --- lx26-21-mm2.orig/kernel/signal.c 2007-05-22 16:59:42.000000000 -0700 | > +++ lx26-21-mm2/kernel/signal.c 2007-05-22 16:59:57.000000000 -0700 | > @@ -507,6 +507,20 @@ static int check_kill_permission(int sig | > && !capable(CAP_KILL)) | > return error; | > | > + /* | > + * If t is the reaper of its namespace and someone from that | > + * namespace is trying to send a signal. | > + * | > + * Note: If some one from parent namespace is sending a signal, | > + * task_child_reaper() != t and we allow the signal. | > + * | > + * In the child namespace, does this block even legitimate signals | > + * like the ones telini…
creator: Sukadev Bhattiprolu
date: 2007-05-25T23:11:46-00:00

Re: [RFC][PATCH 15/16] Enable signaling child reaper from parent ns.
type: item
description: Serge E. Hallyn [serue@us.ibm.com] wrote: | Quoting sukadev@us.ibm.com (sukadev@us.ibm.com): | > | > Subject: Enable signaling child reaper from parent ns. | > | > From: Sukadev Bhattiprolu <sukadev@us.ibm.com> | > | > The reaper of a child namespace must receive signals from its parent pid | > namespace but not receive any signals from its own namespace. | > | > This is a very early draft :-) and following tests seem to pass | > | > - Successfully kill child reaper from parent namespace (init_pid_ns) | > | > - Fail to kill child reaper from within its namespace (non init_pid_ns) | > | > - kill -1 1 from init_pid_ns seemed to work (rescanned inittab) | > | > TODO: | > - Test async io and SIGIO delivery. | > | > - Allow any legitimate signals that the child reaper can receive | > from within its namespace? (we block all signals now) | > | > - Sending SIGKILL to the child reaper of a namespace terminates the | > namespace But if the namespace remounted /proc from user space, | > /proc would remain mounted even after reaper and other process in | > the namespace go away. | > | > Signed-off-by: Sukadev Bhattiprolu <sukadev@us.ibm.com> | > --- | > kernel/signal.c | 22 +++++++++++++++++++++- | > 1 file changed, 21 insertions(+), 1 deletion(-) | > | > Index: lx26-21-mm2/kernel/signal.c | > =================================================================== | > --- lx26-21-mm2.orig/kernel/signal.c 2007-05-22 16:59:42.000000000 -0700 | > +++ lx26-21-mm2/kernel/signal.c 2007-05-22 16:59:57.000000000 -0700 | > @@ -507,6 +507,20 @@ static int check_kill_permission(int sig | > && !capable(CAP_KILL)) | > return error; | > | > + /* | > + * If t is the reaper of its namespace and someone from that | > + * namespace is trying to send a signal. | > + * | > + * Note: If some one from parent namespace is sending a signal, | > + * task_child_reaper() != t and we allow the signal. | > + * | > + * In the child namespace, does this block even legitimate signals | > + * like the ones telini…
creator: Sukadev Bhattiprolu
date: 2007-05-25T23:18:11-00:00

Re: [RFC][PATCH 0/16] Enable cloning of pid namespace
type: item
description: sukadev@us.ibm.com wrote: > Dave, Serge, > > Here is my current pid namespace patchset. There are still a couple of > issues I am investigating, but appreciate any feedback. > > Suka > I tried to compile your patches with CONFIG_PID_NS=n and got this: CC kernel/configs.o kernel/pid.c:361: warning: function declaration isn't a prototype kernel/pid.c: In function `dup_struct_pid': kernel/pid.c:489: warning: initialization makes pointer from integer without a cast Thanks, Pavel. _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Pavel Emelianov
date: 2007-05-29T10:29:06-00:00

Re: [RFC][PATCH 0/16] Enable cloning of pid namespace
type: item
description: On Tue, 2007-05-29 at 14:29 +0400, Pavel Emelianov wrote: > sukadev@us.ibm.com wrote: > > Dave, Serge, > > > > Here is my current pid namespace patchset. There are still a couple of > > issues I am investigating, but appreciate any feedback. > > > > Suka > > > > I tried to compile your patches with CONFIG_PID_NS=n and got this: > > CC kernel/configs.o > kernel/pid.c:361: warning: function declaration isn't a prototype > kernel/pid.c: In function `dup_struct_pid': > kernel/pid.c:489: warning: initialization makes pointer from integer without a cast Could you send along your actual .config? -- Dave _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Dave Hansen
date: 2007-05-29T15:48:09-00:00

Re: [RFC][PATCH 15/16] Enable signaling child reaper from parent ns.
type: item
description: On Fri, 2007-05-25 at 15:13 -0500, Serge E. Hallyn wrote: > > First note on that, (current == child_reaper(current)) is correct if we > want to treat a container init as init no matter who sends the signal, > however I contend that if a signal is sent from an ancester namespace, > then we want to treat the task like any other task. > > Could I get some confirmation or rebuttal from Suka, Eric, Dave, or > Pavel? I agree with this. Signals should only be special when you're sending them "up the chain" to _your_ init process. We shouldn't do anything at all special to them when one process in a set of peers just happens to be the init for a child namespace. -- Dave _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Dave Hansen
date: 2007-05-29T16:47:19-00:00

Re: [RFC][PATCH 06/16] Define is_global_init()
type: item
description: On Fri, 2007-05-25 at 13:44 -0700, sukadev@us.ibm.com wrote: > Dave Hansen [hansendc@us.ibm.com] wrote: > | On Thu, 2007-05-24 at 13:24 +0400, Pavel Emelianov wrote: > | > > | > +int is_global_init(struct task_struct *tsk) > | > > | > +{ > | > > | > + return (task_active_pid_ns(tsk) == &init_pid_ns && tsk->pid == 1); > | > > | > | > > | This can OOPS if you pass arbitrary task to this call... > | > > | tsk->nsproxy can already be NULL. > | > > > | > > Hmm. You are right. btw, this could be a bisect issue. Patch 9 of uses > | > > pid_ns from pid->upid_list and removes nsproxy->pid_ns. > | > > | > Yes, but that patch is not good either. > | > task_pid(tsk) may become NULL as well and this will oops. > | > | Have you reviewed the call paths to make sure this can actually happen > | in practice? > > task_pid() can be NULL when we are tearing down the task structure in > release_task() and in the tiny window between detach_pid() and attach_pid() > in de_thread(). > > I think task_pid() is safe as long as it is called for 'current'. (we should > probably add some comments) If we only call it for "current", then perhaps we should just change the function so that it doesn't take any arguments. That way nobody can screw it up. > I will double check my code, but I think all my calls to task_pid() and hence, > to task_active_pid_ns() are safe, except for two cases: > > a) is_global_init(). There are a few calls to process other than > current, but not sure if they are a problem. > > For instance in current code, unhandled_signal() checks > tsk->pid == 1 and proceeds to derefernce tsk->sighand. > > If task_pid() is NULL because the task was in release_task(), > then so is tsk->sighand. Really? Are there barriers or locks to make this happen? Can you be sure that compiler or cpu re-ordered code will keep this true? > b) the temporary check I added in check_kill_permissions(). > (I need to address Serge's comment here anyway). > > To make is_global_init() more efficient and indep…
creator: Dave Hansen
date: 2007-05-29T16:54:32-00:00

Re: [RFC][PATCH 0/16] Enable cloning of pid namespace
type: item
description: On Tue, 2007-05-29 at 08:48 -0700, Dave Hansen wrote: > On Tue, 2007-05-29 at 14:29 +0400, Pavel Emelianov wrote: > > sukadev@us.ibm.com wrote: > > > Dave, Serge, > > > > > > Here is my current pid namespace patchset. There are still a couple of > > > issues I am investigating, but appreciate any feedback. > > > > > > Suka > > > > > > > I tried to compile your patches with CONFIG_PID_NS=n and got this: > > > > CC kernel/configs.o > > kernel/pid.c:361: warning: function declaration isn't a prototype > > kernel/pid.c: In function `dup_struct_pid': > > kernel/pid.c:489: warning: initialization makes pointer from integer without a cast > > Could you send along your actual .config? Not needed :( Index: linux-2.6.21/kernel/pid.c =================================================================== --- linux-2.6.21.orig/kernel/pid.c 2007-05-25 09:06:30.000000000 -0700 +++ linux-2.6.21/kernel/pid.c 2007-05-29 10:07:39.000000000 -0700 @@ -357,7 +357,7 @@ #else -static int alloc_pid_ns() +static struct pid_namespace *alloc_pid_ns(void) { static int warned; @@ -365,7 +365,7 @@ printk(KERN_INFO "WARNING: CLONE_NEWPID disabled\n"); warned = 1; } - return 0; + return NULL; } void zap_pid_ns_processes(struct pid_namespace *pid_ns) _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Badari Pulavarty
date: 2007-05-29T17:12:33-00:00

Re: [RFC][PATCH 06/16] Define is_global_init()
type: item
description: Dave Hansen [hansendc@us.ibm.com] wrote: | On Fri, 2007-05-25 at 13:44 -0700, sukadev@us.ibm.com wrote: | > Dave Hansen [hansendc@us.ibm.com] wrote: | > | On Thu, 2007-05-24 at 13:24 +0400, Pavel Emelianov wrote: | > | > > | > +int is_global_init(struct task_struct *tsk) | > | > > | > +{ | > | > > | > + return (task_active_pid_ns(tsk) == &init_pid_ns && tsk->pid == 1); | > | > > | | > | > > | This can OOPS if you pass arbitrary task to this call... | > | > > | tsk->nsproxy can already be NULL. | > | > > | > | > > Hmm. You are right. btw, this could be a bisect issue. Patch 9 of uses | > | > > pid_ns from pid->upid_list and removes nsproxy->pid_ns. | > | > | > | > Yes, but that patch is not good either. | > | > task_pid(tsk) may become NULL as well and this will oops. | > | | > | Have you reviewed the call paths to make sure this can actually happen | > | in practice? | > | > task_pid() can be NULL when we are tearing down the task structure in | > release_task() and in the tiny window between detach_pid() and attach_pid() | > in de_thread(). | > | > I think task_pid() is safe as long as it is called for 'current'. (we should | > probably add some comments) | | If we only call it for "current", then perhaps we should just change the | function so that it doesn't take any arguments. That way nobody can | screw it up. Well, if the caller can confirm that the tsk passed in to task_pid() is not exiting, then it is ok to use. We have one such usage in do_fork() where we use the task struct we just initialized. The task has not yet been woken up. | | > I will double check my code, but I think all my calls to task_pid() and hence, | > to task_active_pid_ns() are safe, except for two cases: | > | > a) is_global_init(). There are a few calls to process other than | > current, but not sure if they are a problem. | > | > For instance in current code, unhandled_signal() checks | > tsk->pid == 1 and proceeds to derefernce tsk->sighand. | > | > If task_pid() is NULL because the ta…
creator: Sukadev Bhattiprolu
date: 2007-05-30T00:29:27-00:00

Re: [RFC][PATCH 14/16] Introduce proc_mnt for pid_ns
type: item
description: sukadev@us.ibm.com wrote: > Subject: Introduce proc_mnt for pid_ns > > From: Dave Hansen <hansendc@us.ibm.com> > > The following patch completes the removal of the global proc_mnt. > It fetches the mnt on which to do dentry invalidations from the > pid_namespace in which the task appears. > > For now, there is only one pid namespace in mainline so this is > straightforward. In the -lxc tree we'll have to do something > more complex. The proc_flush_task() code takes a task, and > needs to be able to find the corresponding proc superblocks on > which that tasks's /proc/<pid> directories could appear. We > can tell in which pid namespaces a task appears, so I put a > pointer from the pid namespace to the corresponding proc_mnt. > > /proc currently has some special code to make sure that the root > directory gets set up correctly. It proc_mnt variable in order > to find its way to the root inode. > > Signed-off-by: Dave Hansen <haveblue@us.ibm.com> > Signed-off-by: Sukadev Bhattiprolu <sukadev@us.ibm.com> > --- > > fs/proc/base.c | 32 ++++++++++++++++++++++++- > fs/proc/inode.c | 11 +++++++- > fs/proc/root.c | 52 ++++++++++++++++++++++++++++-------------- > include/linux/pid_namespace.h | 1 > include/linux/proc_fs.h | 1 > 5 files changed, 75 insertions(+), 22 deletions(-) > [snip] > @@ -2071,6 +2084,21 @@ out: > return; > } > > +void proc_flush_task(struct task_struct *task) > +{ > + int i; > + struct pid *pid; > + struct upid* upid; > + > + pid = task_pid(task); > + if (!pid) > + return; The code below will never be called as task flushes all his pids in __unhash_process() that happens before this. Or did I miss smth? > + upid = &pid->upid_list[0]; > + for (i = 0; i < pid->num_upids; i++, upid++) > + proc_flush_task_from_pid_ns(task, upid->pid_ns); > +} > + > static struct dentry *proc_pid_instantiate(struct inode *dir, > struct dentry * dentry, > struct task_struct *task, const void *ptr) _______________________________________________ Containers mailing list Containe…
creator: Pavel Emelianov
date: 2007-05-31T11:48:21-00:00

Re: [RFC][PATCH 14/16] Introduce proc_mnt for pid_ns
type: item
description: On Thu, 2007-05-31 at 15:48 +0400, Pavel Emelianov wrote: > > +void proc_flush_task(struct task_struct *task) > > +{ > > + int i; > > + struct pid *pid; > > + struct upid* upid; > > + > > + pid = task_pid(task); > > + if (!pid) > > + return; > > The code below will never be called as task flushes all his pids > in __unhash_process() that happens before this. Or did I miss smth? Nope, that's a very nice catch. Suka is working on a patch to fix this right now. We just need to keep the list of pids accessible a little longer, but I think Suka ran into a chicken-and-egg problem while trying to solve this. Suka, care to post your workaround? -- Dave _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Dave Hansen
date: 2007-05-31T16:40:04-00:00

Re: [RFC][PATCH 14/16] Introduce proc_mnt for pid_ns
type: item
description: Dave Hansen [hansendc@us.ibm.com] wrote: | On Thu, 2007-05-31 at 15:48 +0400, Pavel Emelianov wrote: | > > +void proc_flush_task(struct task_struct *task) | > > +{ | > > + int i; | > > + struct pid *pid; | > > + struct upid* upid; | > > + | > > + pid = task_pid(task); | > > + if (!pid) | > > + return; | > | > The code below will never be called as task flushes all his pids | > in __unhash_process() that happens before this. Or did I miss smth? | | Nope, that's a very nice catch. Suka is working on a patch to fix this | right now. We just need to keep the list of pids accessible a little | longer, but I think Suka ran into a chicken-and-egg problem while trying | to solve this. Suka, care to post your workaround? Yes. the chicken and egg problem is in the order of the __unhash_process() and proc_flush_task(). With __unhash_process() first, proc_flush_task() has no way of knowing the pid namespaces the process belongs to. With proc_flush_task() first, we find the process but have its dentry removed from proc. This crashes when I run a test in a tight loop. A quick/dirty fix is to save the pid namespace list before __unhash_process() and use that in proc_flush_task(). That would make the exit() path quite expensive. I am still investigating the crash and looking for a better option. | | -- Dave _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Sukadev Bhattiprolu
date: 2007-05-31T19:50:07-00:00