Pid namespaces

(Comment is taken from Sukadev's patchset-v3)

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.

This set is based on my patches, I sent before, but it includes some comments and patches that I received from Sukadev. Sukadev, please, add your Acked-by, Signed-off-by or From, to patches you want (everybody is also welcome :) ).

The set is based on 2.6.23-rc1-mm1, which already has some preparation patches
for pid namespaces. After the review and fixing all the comments, this set will be benchmarked and sent to Andrew for inclusion in -mm tree.

Make task release its namespaces after it has reparented all his children to child_reaper, but before it notifies its parent about its death.

The reason to release namespaces after reparenting is that when task exits it may send a signal to its parent (SIGCHLD), but if the parent has already exited its namespaces there will be no way to decide what pid to dever to him - parent can be from different namespace.

The reason to release namespace before notifying the parent it that when task sends a SIGCHLD to parent it can call wait() on this taks and release it. But releasing the mnt namespace implies dropping of all the mounts in the mnt namespace and NFS expects the task to have valid sighand pointer.

---

exit.c | 5 ++++-

1 files changed, 4 insertions(+), 1 deletion(-)

diff -upr linux-2.6.23-rc1-mm1.orig/kernel/exit.c linux-2.6.23-rc1-mm1-7/kernel/exit.c
@@ -788,6 +804,10 @@ static void exit_notify(struct task_stru

	BUG_ON(!list_empty(&tsk->children));
	BUG_ON(!list_empty(&tsk->ptrace_children));

+ write_unlock_irq(&tasklist_lock);
+ exit_task_namespaces(tsk);
+ write_lock_irq(&tasklist_lock);
+
/*
* Check to see if any process groups have become orphaned
* as a result of our exiting, and if they have any stopped
@@ -999,7 +1021,6 @@ fastcall NORET_TYPE void do_exit(long co

	tsk->exit_code = code;
	proc_exit_connector(tsk);
-	exit_task_namespaces(tsk);
	exit_notify(tsk);
#ifdef CONFIG_NUMA
	mpol_free(tsk->mempolicy);

Looks like there are :( Oleg pointed out, that if any thread dies it can re-parent all its children to this one, who already released its namespaces... :(

the was a request from many people including Andrew that CONFIG_XXX is bad approach.

First versions of Pavel's patches had CONFIG_XXX for this.

Thanks,
Kirill

Perhaps, we can do something like the patch below. Roland, what do you think?

We can check PF_EXITING instead of ->exit_state while choosing the new parent. Note that tasklits_lock acts as a barrier, everyone who takes tasklist after us (when forget_original_parent() drops it) must see PF_EXITING.

Oleg.

@@ -686,11 +686,14 @@ reparent_thread(struct task_struct *p, s
  * the child reaper process (ie "init") in our pid
  * space.
  */
-static void
-forget_original_parent(struct task_struct *father, struct list_head *to_release)
+static void forget_original_parent(struct task_struct *father)
 {
 	struct task_struct *p, *reaper = father;
-	struct list_head *_p, *_n;
+	struct list_head *ptrace_dead, *_p, *_n;
+
+	INIT_LIST_HEAD(&ptrace_dead);
+
+	write_lock_irq(&tasklist_lock);
 
 	do {
 		reaper = next_thread(reaper);

@@ -698,7 +701,7 @@ forget_original_parent(struct task_struc
 			reaper = child_reaper(father);
 			break;
 		}
-	} while (reaper->exit_state);
+	} while (reaper->flags & PF_EXITING);
 
 	/*
 	 * There are only two places where our children can be:

@@ -736,13 +739,25 @@ forget_original_parent(struct task_struc
 		 * while it was being traced by us, to be able to see it in wait4.
 		 */
 		if (unlikely(ptrace && p->exit_state == EXIT_ZOMBIE && p->exit_signal == -1))
-			list_add(&p->ptrace_list, to_release);
+			list_add(&p->ptrace_list, &ptrace_dead);
 	}
+
 	list_for_each_safe(_p, _n, &father->ptrace_children) {
 		p = list_entry(_p, struct task_struct, ptrace_list);
 		choose_new_parent(p, reaper);
 		reparent_thread(p, father, 1);
 	}
+
+	write_unlock_irq(&tasklist_lock);
+	BUG_ON(!list_empty(&tsk->children));
+	BUG_ON(!list_empty(&tsk->ptrace_children));
+
+	list_for_each_safe(_p, _n, &ptrace_dead) {
+		list_del_init(_p);
+		t = list_entry(_p, struct task_struct, ptrace_list);
+		release_task(t);
+	}
+
 }
 
 /*

@@ -753,7 +768,6 @@ static void exit_notify(struct task_stru
 {
 	int state;
 	struct task_struct *t;
-	struct list_head ptrace_dead, *_p, *_n;
 	struct pid *pgrp;
 
 	if (signal_pending(tsk) && !(tsk->signal->flags & SIGNAL_GROUP_EXIT)

@@ -776,8 +790,6 @@ static void exit_notify(struct task_stru
 		read_unlock(&tasklist_lock);
 	}
 
-	write_lock_irq(&tasklist_lock);
-
 	/*
 	 * This does two things:
 	 *

@@ -786,12 +798,9 @@ static void exit_notify(struct task_stru
 	 *	as a result of our exiting, and if they have any stopped
 	 *	jobs, send them a SIGHUP and then a SIGCONT.  (POSIX 3.2.2.2)
 	 */
+	forget_original_parent(tsk);
 
-	INIT_LIST_HEAD(&ptrace_dead);
-	forget_original_parent(tsk, &ptrace_dead);
-	BUG_ON(!list_empty(&tsk->children));
-	BUG_ON(!list_empty(&tsk->ptrace_children));
-
+	write_lock_irq(&tasklist_lock);
 	/*
 	 * Check to see if any process groups have become orphaned
 	 * as a result of our exiting, and if they have any stopped

@@ -801,9 +810,8 @@ static void exit_notify(struct task_stru
 	 * and we were the only connection outside, so our pgrp
 	 * is about to become orphaned.
 	 */
-	 
 	t = tsk->real_parent;
-	
+
 	pgrp = task_pgrp(tsk);
 	if ((task_pgrp(t) != pgrp) &&
 	    (task_session(t) == task_session(tsk)) &&

@@ -826,9 +834,8 @@ static void exit_notify(struct task_stru
 	 * If our self_exec id doesn't match our parent_exec_id then
 	 * we have changed execution domain as these two values started
 	 * the same after a fork.
-	 *	
 	 */
-	
+
 	if (tsk->exit_signal != SIGCHLD && tsk->exit_signal != -1 &&
 	    ( tsk->parent_exec_id != t->self_exec_id  ||
 	      tsk->self_exec_id != tsk->parent_exec_id)

@@ -856,12 +863,6 @@ static void exit_notify(struct task_stru
 
 	write_unlock_irq(&tasklist_lock);
 
-	list_for_each_safe(_p, _n, &ptrace_dead) {
-		list_del_init(_p);
-		t = list_entry(_p, struct task_struct, ptrace_list);
-		release_task(t);
-	}
-
 	/* If the process is dead, release it - nobody will wait for it */
 	if (state == EXIT_DEAD)
 		release_task(tsk);

Heh :) I've came to the same conclusion and now I'm checking for it. But my patch is much simpler that yours - it just checks for PF_EXITING in forget_original_parent:

@@ -712,7 +712,7 @@ forget_original_parent(struct task_struc

			reaper = task_child_reaper(father);
			break;
		}

- } while (reaper->exit_state);
+ } while (reaper->flags & PF_EXITING);

	/*
	 * There are only two places where our children can be:

:) Ok. Thanks, I then will check with your patch. If you don't mind I will carry it together with this set.

Thanks,
Pavel

I almost forgot about this one...

After reading the whole series, I can't understand the above explanation any longer. The parent can't be from different namespace, either we have another sub-thread, or we reparent the child to /sbin/init which should be from the same namespace.

If we are /sbin/init, we kill all tasks from the same namespace before doing exit_notify().

Could you clarify?

Oleg.

If the child that is a new namespace's init is exiting its parent is from the different namespace. Moreover, we will probably want to implement "entering" the pid namespace, so having tasks with parents from another namespace will be OK.

In that case it doesn't have childs. The were SIGKILL'ed before exit_notify().

Well. I saw this word "entering", but I don't know the meaning. Just curious, could you explain?

And, if an exiting task has a child which is already from another namespace, why can't we release our namespace before re-parenting? I guess I need to know what "entering" means to understand this...

Oleg.

It does not, but it's parent - does :)

"Entering" means "moving task to arbitrary namespace"

One of the desired actions was the following:
1. task X clones the new namespace with the child Y as this namespace's init;
2. task X waits for SIGCHILD to come informing that the namespace is dead. In this scenario we need to set the Y's pid as it is seen from X's namespace in siginfo.

Yes. But in that case forget_original_parent() is no-op! This means that it is not needed to move exit_task_namepsace() after.

Heh. Very much nontrivial, good luck :) At least we need to grow ->numbers[], and if its pid was pinned...

Yes sure. But again, how this is connected to "we should do exit_task_namespace() after forget_original_parent()" ?

I guess I missed something stupid and simple...

Oleg.

If task X is exiting and has already exit_task_namespaces()-ed task
Y will OOPs during its exit in determining parent's namespace. I agree that in that case this is not important what namespace X belongs to, but we need to handle the race with changing the nsproxy from not-NULL to NULL. This is OK to make this under task_lock() but what to add extra locking for if we can avoid it?

No, we can't take task_lock() under write_lock(tasklist).

Oleg.

I meant that we could save the namspace earlier (w/o tasklist), carry it up to the place we need and release it later. But all this is too complex and it's easier to get rid of children and then release the namespaces.

In other words. Let task X live in init_pid_ns, task Y is his child and lives int another namespace. task X and task Y both die. This will happen:

1. Task X call exit_task_namespaces()
and sets its nsproxy to NULL
1. Task Y is going to notify the parent (X) and dereferences its nsproxy -> OOPS
2. Task X reparents all its children

If we move the exit_task_namespace this will happen:

1. Task X reparents all its children
2. Task X call exit_task_namespaces()
and sets its nsproxy to NULL

In such case is tasy Y will dereference the parent's nsproxy it will not OOPS because either its parent will be not X already, or X's nsproxy is not yet released.

Ah, got it, thanks. So the problem is not namespace itself (parent's or child's), there are still valid (even if different but related).

We just can't get ->parent->nsproxy. I was greatly confused by the "parent can be from different namespace" above. We have exactly same problem if namespaces are not differ.

IOW, the problem is: we can't clear ->nsproxy (exit_task_namespaces) until we get rid of ->children. This have nothing to do with different namespace.

Oleg.

No. If the parent is always in the same namespace we do not need to get its nsproxy :) Problem is exactly in that the parent's namespace is to be known.

Yes yes, I see. I meant: once do_notify_parent() was modified to use parent->nsproxy to figure out correct pid_t, that problem has nothing to do with namespaces, it is just parent->nsproxy access.

But this is not safe, btw? do_notify_parent() can get parent->nsproxy which is under destruction (sys_unshare). Then we read its ->pid_ns, but at this time "struct nsproxy" could be kmem_cache_free()'ed ?

Of course, this is just theoretical, irqs are disabled, and the window is tiny.

Oleg.

This flag tells the .get_sb callback that this is a kern_mount() call so that it can trust *data pointer to be valid in-kernel one.

Running a few steps forward - this will be needed for proc to create the superblock and store a valid pid namespace on it during the namespace creation. The reason, why the namespace cannot live without proc mount is described in the appropriate patch.

---

fs/namespace.c     |    3 ++-
fs/super.c         |    6 +++---
include/linux/fs.h |    4 +++-
3 files changed, 8 insertions(+), 5 deletions(-)

diff -upr linux-2.6.23-rc1-mm1.orig/fs/namespace.c linux-2.6.23-rc1-mm1-7/fs/namespace.c
@@ -1579,7 +1579,8 @@ long do_mount(char *dev_name, char *dir_

		mnt_flags |= MNT_NOMNT;

	flags &= ~(MS_NOSUID | MS_NOEXEC | MS_NODEV | MS_ACTIVE |
-		   MS_NOATIME | MS_NODIRATIME | MS_RELATIME | MS_NOMNT);
+		   MS_NOATIME | MS_NODIRATIME | MS_RELATIME |
+		   MS_NOMNT | MS_KERNMOUNT);

	/* ... and get the mountpoint */
	retval = path_lookup(dir_name, LOOKUP_FOLLOW, &nd);

diff -upr linux-2.6.23-rc1-mm1.orig/fs/super.c linux-2.6.23-rc1-mm1-7/fs/super.c
@@ -944,9 +944,9 @@ do_kern_mount(const char *fstype, int fl

	return mnt;
}

-struct vfsmount *kern_mount(struct file_system_type *type) +struct vfsmount *kern_mount_data(struct file_system_type *type, void *data) {
- return vfs_kern_mount(type, 0, type->name, NULL);
+ return vfs_kern_mount(type, MS_KERNMOUNT, type->name, data);
}

-EXPORT_SYMBOL(kern_mount);
+EXPORT_SYMBOL_GPL(kern_mount_data);

diff -upr linux-2.6.23-rc1-mm1.orig/include/linux/fs.h linux-2.6.23-rc1-mm1-7/include/linux/fs.h
@@ -129,6 +129,7 @@ extern int dir_notify_enable;

#define MS_RELATIME (1<<21) /* Update atime relative to mtime/ctime. */
#define MS_SETUSER (1<<23) /* set mnt_uid to current user */
#define MS_NOMNT (1<<24) /* don't allow unprivileged submounts */
+#define MS_KERNMOUNT (1<<25) /* this is a kern_mount call */
#define MS_ACTIVE (1<<30)
#define MS_NOUSER (1<<31)

@@ -1459,7 +1460,8 @@ void unnamed_dev_init(void);

extern int register_filesystem(struct file_system_type *);
extern int unregister_filesystem(struct file_system_type *); -extern struct vfsmount *kern_mount(struct file_system_type *); +extern struct vfsmount *kern_mount_data(struct file_system_type *, void *data); +#define kern_mount(type) kern_mount_data(type, NULL)
extern int may_umount_tree(struct vfsmount *);
extern int may_umount(struct vfsmount *);
extern void umount_tree(struct vfsmount *, int, struct list_head *);

From: Sukadev Bhattiprolu <sukadev-r/[email protected]>

get_signal_to_deliver() checks to ensure that /sbin/init does not receive any unwanted signals. With implementation of multiple pid namespaces, we need to extend this check for all "container-init" processes (processes with pid == 1 in the pid namespace)

IOW, A container-init process, must not receive unwanted signals from within the container. However, the container-init must receive and honor any signals it receives from an ancestor pid namespace (i.e it must appear like a normal process in its parent namespace).

i.e for correct processing of the signal, either:

- the recepient of the signal (in get_signal_to_deliver()) must have some information (such as pid namespace) of the sender

- or the sender of the signal (in send_signal()) should know whether the signal is an "unwanted" signal from the recepient's POV.

This patch provides a simple mechanism, to pass additional information from the sender to the recepient (from send_signal() to get_signal_to_deliver()).

This patch is just a helper and a follow-on patch will use this infrastructure to actually pass in information about the sender.

Implementation note:

Most signal interfaces in the kernel operate on 'siginfo_t' data structure which is user-visible and cannot be easily modified/ extended. So this patch defines a wrapper, 'struct kern_siginfo', around the 'siginfo_t' and allows extending this wrapper for future needs.

TODO: This is more an exploratory patch and modifies only interfaces
necessary to implement correct signal semantics in pid namespaces.

If the approach is feasible, we could consistently use 'kern_siginfo' in other signal interfaces and possibly in 'struct sigqueue'.

We could modify dequeue_signal() to directly work with 'kern_siginfo' and remove dequeue_signal_kern_info().

---

include/linux/signal.h | 7 +++++++
kernel/signal.c | 38 +++++++++++++++++++++++++++++++-------

2 files changed, 38 insertions(+), 7 deletions(-)

diff -upr linux-2.6.23-rc1-mm1.orig/include/linux/signal.h linux-2.6.23-rc1-mm1-7/include/linux/signal.h
@@ -7,6 +7,13 @@

#ifdef __KERNEL__
#include <linux/list.h>

+struct kern_siginfo {
+ siginfo_t *info;
+ int flags;
+};
+
+#define KERN_SIGINFO_CINIT 0x1 /* True iff signalling container-init */
+
/*
* Real Time signals may be queued.
*/

diff -upr linux-2.6.23-rc1-mm1.orig/kernel/signal.c linux-2.6.23-rc1-mm1-7/kernel/signal.c
@@ -299,10 +299,12 @@ unblock_all_signals(void)

	spin_unlock_irqrestore(&current->sighand->siglock, flags);
}

-static int collect_signal(int sig, struct sigpending *list, siginfo_t *info) +static int collect_signal(int sig, struct sigpending *list,
+ struct kern_siginfo *kinfo)

{
	struct sigqueue *q, *first = NULL;
	int still_pending = 0;

+ siginfo_t *info = kinfo->info;

	if (unlikely(!sigismember(&list->signal, sig)))
		return 0;

@@ -343,7 +348,7 @@ static int collect_signal(int sig, struc }

static int __dequeue_signal(struct sigpending *pending, sigset_t *mask,
- siginfo_t *info)
+ struct kern_siginfo *kinfo)

{
	int sig = next_signal(pending, mask);

@@ -357,7 +364,7 @@ static int __dequeue_signal(struct sigpe

			}
		}

- if (!collect_signal(sig, pending, info))
+ if (!collect_signal(sig, pending, kinfo))

			sig = 0;
	}

@@ -370,18 +377,20 @@ static int __dequeue_signal(struct sigpe
*
* All callers have to hold the siglock.
*/
-int dequeue_signal(struct task_struct *tsk, sigset_t *mask, siginfo_t *info) +int dequeue_signal_kern_info(struct task_struct *tsk, sigset_t *mask,
+ struct kern_siginfo *kinfo)

{
	int signr = 0;

+ siginfo_t *info = kinfo->info;

	/* We only dequeue private signals from ourselves, we don't let
	 * signalfd steal them
	 */
	if (tsk == current)
-		signr = __dequeue_signal(&tsk->pending, mask, info);
+		signr = __dequeue_signal(&tsk->pending, mask, kinfo);
	if (!signr) {
		signr = __dequeue_signal(&tsk->signal->shared_pending,
-					 mask, info);
+					 mask, kinfo);
		/*
		 * itimer signal ?
		 *

@@ -441,6 +450,22 @@ int dequeue_signal(struct task_struct *t }

/*
+ * Dequeue a signal and return the element to the caller, which is
+ * expected to free it.
+ *
+ * All callers have to hold the siglock.
+ */
+int dequeue_signal(struct task_struct *tsk, sigset_t *mask, siginfo_t *info) +{
+ struct kern_siginfo kinfo;
+
+ kinfo.info = info;
+ kinfo.flags = 0;
+
+ return dequeue_signal_kern_info(tsk, mask, &kinfo);
+}
+
+/*
* Tell a process that it has a new active signal..
*
* NOTE! we rely on the previous spin_lock to
@@ -1779,6 +1873,10 @@ int get_signal_to_deliver(siginfo_t *inf

{
	sigset_t *mask = &current->blocked;
	int signr = 0;

+ struct kern_siginfo kinfo;
+
+ kinfo.info = info;
+ kinfo.flags = 0;

	try_to_freeze();

@@ -1791,7 +1889,7 @@ relock:

		    handle_group_stop())
			goto relock;

- signr = dequeue_signal(current, mask, info);
+ signr = dequeue_signal_kern_info(current, mask, &kinfo);

		if (!signr)
			break; /* will return 0 */

Well... I know, it is very easy to blame somebody else's patch, and probably my taste is not good...

But honestly, I personally think this approach is a horror, and any alternative is better :)

I'd rather change dequeue_signal() so that it takes "struct sigqueue *" parameter instead of "siginfo_t *", or add a new "int *flags".

OK, this doesn't work anyway, we should do something different. Perhaps just do all checks on sender's side.

It is a bit strange that this patch is 3/15, and the rest bits in 11/15, not very convenient for the review.

Oleg.

Yes. Signal handling in namespaces turned out to be the most complicated part of the set. I start thinking to drop this part till we have the "core" in -mm tree. Suka, what do you think?

Well, I thought that a split like
1. patches for kernel to prepare it for the set
2. the set itself
is the best to review. Maybe I was wrong, but how to make this then? E.g. I have a MS_KERNOUNT patch, but its changes are used *much* later.

Thanks,
Pavel

Pavel Emelianov [[email protected]] wrote:

| Oleg Nesterov wrote:
| >On 07/26, Pavel Emelyanov wrote:
| >>TODO: 	This is more an exploratory patch and modifies only 
| >>interfaces
| >>	necessary to implement correct signal semantics in pid namespaces.
| >>	
| >>	If the approach is feasible, we could consistently use 'kern_siginfo'
| >>	in other signal interfaces and possibly in 'struct sigqueue'.
| >>
| >>	We could modify dequeue_signal() to directly work with 'kern_siginfo'
| >>	and remove dequeue_signal_kern_info().
| >
| >Well... I know, it is very easy to blame somebody else's patch, and 
| >probably
| >my taste is not good...
| >
| >But honestly, I personally think this approach is a horror, and any 
| >alternative
| >is better :)

Hmm. My reasoning was that since siginfo_t was directly "shared" with user space, extending it even to add a flag was pain.

| >
| >I'd rather change dequeue_signal() so that it takes "struct sigqueue *"
| >parameter instead of "siginfo_t *", or add a new "int *flags".

My earlier version to Containers@ passed in "int *signal_cinit" couple of levels down and used that in get_signal_to_deliver() but that looked ugly :-) Passing in sigqueue to dequeue_signal() may be better, but anyway...

| >
| >OK, this doesn't work anyway, we should do something different. Perhaps
| >just do all checks on sender's side.
| 
| Yes. Signal handling in namespaces turned out to be the most complicated
| part of the set. I start thinking to drop this part till we have the "core"
| in -mm tree. Suka, what do you think?

Yes. Lets focus on the core for now and allow privileged user in a child-ns to terminate the container-init. I will try the signal approach from sender side also.

| 
| >It is a bit strange that this patch is 3/15, and the rest bits in 11/15,
| >not very convenient for the review.
| 
| Well, I thought that a split like
| 1. patches for kernel to prepare it for the set
| 2. the set itself
| is the best to review. Maybe I was wrong, but how to make this then?
| E.g. I have a MS_KERNOUNT patch, but its changes are used *much* later.
| 
| >Oleg.
| >
| >
| 
| Thanks,
| Pavel

Since a task will appear in more than one proc tree we need to shrink many trees. For this case we pass the struct pid to proc_flush_task() and shrink the mounts of all the namespaces this pid belongs to.

The NULL passed to it means that only global mount is to be flushed. This is a preparation for a real flushing patch.

---

fs/proc/base.c          |   18 +++++++++++++++---
include/linux/proc_fs.h |    6 ++++--
kernel/exit.c           |    2 +-
3 files changed, 20 insertions(+), 6 deletions(-)

diff -upr linux-2.6.23-rc1-mm1.orig/fs/proc/base.c linux-2.6.23-rc1-mm1-7/fs/proc/base.c
@@ -74,6 +74,7 @@

#include <linux/nsproxy.h>
#include <linux/oom.h>
#include <linux/elf.h>
+#include <linux/pid_namespace.h>
#include "internal.h"

/* NOTE:
@@ -2115,7 +2116,7 @@ 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.
*/
-void proc_flush_task(struct task_struct *task)
+static void proc_flush_task_mnt(struct task_struct *task, struct vfsmount *mnt)

{
	struct dentry *dentry, *leader, *dir;
	char buf[PROC_NUMBUF];

@@ -2123,7 +2124,7 @@ void proc_flush_task(struct task_struct

	name.name = buf;
	name.len = snprintf(buf, sizeof(buf), "%d", task->pid);
-	dentry = d_hash_and_lookup(proc_mnt->mnt_root, &name);
+	dentry = d_hash_and_lookup(mnt->mnt_root, &name);
	if (dentry) {
		shrink_dcache_parent(dentry);
		d_drop(dentry);

@@ -2135,7 +2136,7 @@ void proc_flush_task(struct task_struct

	name.name = buf;
	name.len = snprintf(buf, sizeof(buf), "%d", task->tgid);
-	leader = d_hash_and_lookup(proc_mnt->mnt_root, &name);
+	leader = d_hash_and_lookup(mnt->mnt_root, &name);
	if (!leader)
		goto out;

@@ -2161,6 +2162,17 @@ out:

	return;
}

+/*
+ * when flushing dentries from proc one need to flush them from global
+ * proc (proc_mnt) and from all the namespaces' procs this task was seen
+ * in. this call is supposed to make all this job.
+ */
+
+void proc_flush_task(struct task_struct *task, struct pid *pid)
+{
+ proc_flush_task_mnt(task, proc_mnt);
+}
+
static struct dentry *proc_pid_instantiate(struct inode *dir,
struct dentry * dentry, struct task_struct *task, const void *ptr)

diff -upr linux-2.6.23-rc1-mm1.orig/include/linux/proc_fs.h linux-2.6.23-rc1-mm1-7/include/linux/proc_fs.h
@@ -111,7 +111,7 @@ extern void proc_misc_init(void);
 

struct mm_struct;

-void proc_flush_task(struct task_struct *task);
+void proc_flush_task(struct task_struct *task, struct pid *pid);
struct dentry *proc_pid_lookup(struct inode *dir, struct dentry * dentry, struct nameidata *); int proc_pid_readdir(struct file * filp, void * dirent, filldir_t filldir); unsigned long task_vsize(struct mm_struct *);
@@ -223,7 +227,9 @@ static inline void proc_net_remove(const
#define proc_net_create(name, mode, info) ({ (void)(mode), NULL; })
static inline void proc_net_remove(const char *name) {}

-static inline void proc_flush_task(struct task_struct *task) { } +static inline void proc_flush_task(struct task_struct *task, struct pid *pid) +{
+}

static inline struct proc_dir_entry *create_proc_entry(const char *name,

	mode_t mode, struct proc_dir_entry *parent) { return NULL; }

diff -upr linux-2.6.23-rc1-mm1.orig/kernel/exit.c linux-2.6.23-rc1-mm1-7/kernel/exit.c
@@ -184,7 +199,7 @@ repeat:

	}

	write_unlock_irq(&tasklist_lock);
-	proc_flush_task(p);
+	proc_flush_task(p, NULL);
	release_thread(p);
	call_rcu(&p->rcu, delayed_put_task_struct);

From: Sukadev Bhattiprolu <sukadev-r/[email protected]>

Since task will be visible from different pid namespaces each of them have to be addressed by multiple pids. struct upid is to store the information about which id refers to which namespace.

The constuciton looks like this. Each struct pid carried the reference counter and the list of tasks attached to this pid. At its end it has a variable length array of struct upid-s. Each struct upid has a numerical id (pid itself), pointer to the namespace, this ID is valid in and is hashed into a pid_hash for searching the pids.

---

init_task.h | 9 ++++++---
pid.h | 12 +++++++++---

2 files changed, 15 insertions(+), 6 deletions(-)

diff -upr linux-2.6.23-rc1-mm1.orig/include/linux/init_task.h linux-2.6.23-rc1-mm1-7/include/linux/init_task.h
@@ -93,15 +93,18 @@ extern struct group_info init_groups;
 

#define INIT_STRUCT_PID {						\
	.count 		= ATOMIC_INIT(1),				\
-	.nr		= 0, 						\
-	/* Don't put this struct pid in pid_hash */			\
-	.pid_chain	= { .next = NULL, .pprev = NULL },		\
	.tasks		= {						\
		{ .first = &init_task.pids[PIDTYPE_PID].node },		\
		{ .first = &init_task.pids[PIDTYPE_PGID].node },	\
		{ .first = &init_task.pids[PIDTYPE_SID].node },		\
	},								\
	.rcu		= RCU_HEAD_INIT,				\

+ .level = 0, \
+ .numbers = { { \
+ .nr = 0, \
+ .ns = &init_pid_ns, \
+ .pid_chain = { .next = NULL, .pprev = NULL }, \
+ }, } \
}

#define INIT_PID_LINK(type) \

diff -upr linux-2.6.23-rc1-mm1.orig/include/linux/pid.h linux-2.6.23-rc1-mm1-7/include/linux/pid.h
@@ -40,15 +40,21 @@ enum pid_type
 * processes.
 */
 
-struct pid
-{
-	atomic_t count;
+struct upid {

	/* Try to keep pid_chain in the same cacheline as nr for find_pid */
	int nr;
+	struct pid_namespace *ns;
	struct hlist_node pid_chain;

+};
+
+struct pid
+{

+	atomic_t count;
	/* lists of tasks that use this pid */
	struct hlist_head tasks[PIDTYPE_MAX];
	struct rcu_head rcu;

+ int level;
+ struct upid numbers[1];
};

extern struct pid init_struct_pid;

Yes. U forgot to mention, that this patchset is git-bisect-not-safe :) I sent the safe split earlier, but it was harder to make and understand, so I decided not to waste the time and sent a badly-split set just to get comments about the approach. The ways a big patch is split wouldn't affect the comments about the ideas, bugs, etc.

We already have it :)

Possible, but how will struct pid *find_pid_nr_ns(int nr, struct pid_namespace *ns); look then? The only way (I see) is to make

hlist_for_each_entry (upid, ...)
if (upid->nr == nr && upid->ns == ns)
return container_of(upid, struct pid, ...)

Thanks,
Pavel

Each struct upid element of struct pid has to be initialized properly, i.e. its nr mst be allocated from appropriate pidmap and it must be inserted into the hash.

---

include/linux/pid.h | 2 +-
kernel/pid.c | 52 +++++++++++++++++++++++++++++++++++++---------------

2 files changed, 38 insertions(+), 16 deletions(-)

diff -upr linux-2.6.23-rc1-mm1.orig/include/linux/pid.h linux-2.6.23-rc1-mm1-7/include/linux/pid.h
@@ -83,7 +92,7 @@ extern void FASTCALL(detach_pid(struct t

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(struct pid_namespace *ns); extern void FASTCALL(free_pid(struct pid *pid));

static inline pid_t pid_nr(struct pid *pid)

diff -upr linux-2.6.23-rc1-mm1.orig/kernel/pid.c linux-2.6.23-rc1-mm1-7/kernel/pid.c
@@ -28,7 +28,8 @@

#include <linux/pid_namespace.h>
#include <linux/init_task.h>

-#define pid_hashfn(nr) hash_long((unsigned long)nr, pidhash_shift)
+#define pid_hashfn(nr, ns) \
+ hash_long((unsigned long)nr + (unsigned long)ns, pidhash_shift)
static struct hlist_head *pid_hash;
static int pidhash_shift;
struct pid init_struct_pid = INIT_STRUCT_PID;
@@ -187,11 +202,13 @@ fastcall void put_pid(struct pid *pid)

	if (!pid)
		return;

- /* FIXME - this must be the namespace this pid lives in */
- ns = &init_pid_ns;

+	ns = pid->numbers[pid->level].ns;
	if ((atomic_read(&pid->count) == 1) ||
-	     atomic_dec_and_test(&pid->count))
+	     atomic_dec_and_test(&pid->count)) {
		kmem_cache_free(ns->pid_cachep, pid);

+ if (ns != &init_pid_ns)
+ put_pid_ns(ns);
+ }
}
EXPORT_SYMBOL_GPL(put_pid);

@@ -204,45 +221,64 @@ static void delayed_put_pid(struct rcu_h fastcall void free_pid(struct pid *pid)

{
	/* We can be called with write_lock_irq(&tasklist_lock) held */
+	int i;
	unsigned long flags;

	spin_lock_irqsave(&pidmap_lock, flags);
-	hlist_del_rcu(&pid->pid_chain);
+	for (i = 0; i <= pid->level; i++)
+		hlist_del_rcu(&pid->numbers[i].pid_chain);
	spin_unlock_irqrestore(&pidmap_lock, flags);

- free_pidmap(&init_pid_ns, pid->nr);
+ for (i = 0; i <= pid->level; i++)
+ free_pidmap(pid->numbers[i].ns, pid->numbers[i].nr);
+

	call_rcu(&pid->rcu, delayed_put_pid);
}

-struct pid *alloc_pid(void)
+struct pid *alloc_pid(struct pid_namespace *ns)

{
	struct pid *pid;
	enum pid_type type;

- int nr = -1;
- struct pid_namespace *ns;
+ int i, nr;
+ struct pid_namespace *tmp;

-	ns = task_active_pid_ns(current);
	pid = kmem_cache_alloc(ns->pid_cachep, GFP_KERNEL);
	if (!pid)
		goto out;

- nr = alloc_pidmap(ns);
- if (nr < 0)
- goto out_free;
+ tmp = ns;
+ for (i = ns->level; i >= 0; i--) {
+ nr = alloc_pidmap(tmp);
+ if (nr < 0)
+ goto out_free;
+
+ pid->numbers[i].nr = nr;
+ pid->numbers[i].ns = tmp;
+ tmp = tmp->parent;
+ }

+ if (ns != &init_pid_ns)
+ get_pid_ns(ns);
+

+	pid->level = ns->level;
	atomic_set(&pid->count, 1);
-	pid->nr = nr;
	for (type = 0; type < PIDTYPE_MAX; ++type)
		INIT_HLIST_HEAD(&pid->tasks[type]);

	spin_lock_irq(&pidmap_lock);

- hlist_add_head_rcu(&pid->pid_chain, &pid_hash[pid_hashfn(pid->nr)]);
+ for (i = pid->level; i >= 0; i--)
+ hlist_add_head_rcu(&pid->numbers[i].pid_chain,
+ &pid_hash[pid_hashfn(pid->numbers[i].nr,

+					pid->numbers[i].ns)]);
	spin_unlock_irq(&pidmap_lock);

out:

	return pid;

out_free:
+ for (i++; i <= ns->level; i++)
+ free_pidmap(pid->numbers[i].ns, pid->numbers[i].nr);
+

	kmem_cache_free(ns->pid_cachep, pid);
	pid = NULL;
	goto out;

task_active_pid_ns() by newly created task, not the current! That's why we need to pass something to alloc_pid() to find this new namespace. Task or namespace itself - is the matter of choice - I selected the most obvious argument :)

Parent is added in another patch - 12/15. I will split it better when sending to Andrew - patches will be smaller and bisect-safe.

It is :)

I think you're right.

Yes. This set is not well split, sorry. I wanted to get comments about the approach, bugs, etc (I have already mentioned this in another letter...)

Thanks,
Pavel.

When showing pid to user or getting the pid numerical id for in-kernel use the value of this id may differ depending on the namespace.

This set of helpers is used to get the global pid nr, the virtual (i.e. seen by task in its namespace) nr and the nr as it is seen from the specified namespace.

---

include/linux/pid.h   |   24 ++++++++++-
include/linux/sched.h |  108 +++++++++++++++++++++++++++++++++++++++++++++-----
kernel/pid.c          |    8 +++
3 files changed, 129 insertions(+), 11 deletions(-)

diff -upr linux-2.6.23-rc1-mm1.orig/include/linux/pid.h linux-2.6.23-rc1-mm1-7/include/linux/pid.h
@@ -83,12 +92,34 @@ extern void FASTCALL(detach_pid(struct t
 

extern struct pid *alloc_pid(struct pid_namespace *ns); extern void FASTCALL(free_pid(struct pid *pid));
+
+/*
+ * the helpers to get the pid's id seen from different namespaces
+ *
+ * pid_nr() : global id, i.e. the id seen from the init namespace;
+ * pid_vnr() : virtual id, i.e. the id seen from the namespace this pid
+ * belongs to. this only makes sence when called in the
+ * context of the task that belongs to the same namespace;
+ * pid_nr_ns() : id seen from the ns specified.
+ *
+ * see also task_xid_nr() etc in include/linux/sched.h
+ */

static inline pid_t pid_nr(struct pid *pid)

{
	pid_t nr = 0;
	if (pid)

- nr = pid->nr;
+ nr = pid->numbers[0].nr;
+ return nr;
+}
+
+pid_t pid_nr_ns(struct pid *pid, struct pid_namespace *ns); +
+static inline pid_t pid_vnr(struct pid *pid)
+{
+ pid_t nr = 0;
+ if (pid)

+		nr = pid->numbers[pid->level].nr;
	return nr;
}

diff -upr linux-2.6.23-rc1-mm1.orig/include/linux/sched.h linux-2.6.23-rc1-mm1-7/include/linux/sched.h
@@ -1221,16 +1222,6 @@ static inline int rt_task(struct task_st

	return rt_prio(p->prio);
}

-static inline pid_t task_pgrp_nr(struct task_struct *tsk)
-{
- return tsk->signal->pgrp;
-}
-
-static inline pid_t task_session_nr(struct task_struct *tsk) -{
- return tsk->signal->__session;
-}
-
static inline void set_task_session(struct task_struct *tsk, pid_t session)

{
	tsk->signal->__session = session;
@@ -1256,6 +1247,104 @@ static inline struct pid *task_session(s
	return task->group_leader->pids[PIDTYPE_SID].pid;
}

+struct pid_namespace;
+
+/*
+ * the helpers to get the task's different pids as they are seen
+ * from various namespaces
+ *
+ * task_xid_nr() : global id, i.e. the id seen from the init namespace;
+ * task_xid_vnr() : virtual id, i.e. the id seen from the namespace the task
+ * belongs to. this only makes sence when called in the
+ * context of the task that belongs to the same namespace;
+ * task_xid_nr_ns() : id seen from the ns specified;
+ *
+ * set_task_vxid() : assigns a virtual id to a task;
+ *
+ * task_ppid_nr_ns() : the parent's id as seen from the namespace specified.

+ *                     the result depends on the namespace and whether the
+ *                     task in question is the namespace's init. e.g. for the
+ *                     namespace's init this will return 0 when called from
+ *                     the namespace of this init, or appropriate id otherwise.
+ *                     + *

+ * see also pid_nr() etc in include/linux/pid.h
+ */
+
+static inline pid_t task_pid_nr(struct task_struct *tsk) +{
+ return tsk->pid;
+}
+
+static inline pid_t task_pid_nr_ns(struct task_struct *tsk,
+ struct pid_namespace *ns)
+{
+ return pid_nr_ns(task_pid(tsk), ns);
+}
+
+static inline pid_t task_pid_vnr(struct task_struct *tsk) +{
+ return pid_vnr(task_pid(tsk));
+}
+
+
+static inline pid_t task_tgid_nr(struct task_struct *tsk) +{
+ return tsk->tgid;
+}
+
+static inline pid_t task_tgid_nr_ns(struct task_struct *tsk,
+ struct pid_namespace *ns)
+{
+ return pid_nr_ns(task_tgid(tsk), ns);
+}
+
+static inline pid_t task_tgid_vnr(struct task_struct *tsk) +{
+ return pid_vnr(task_tgid(tsk));
+}
+
+
+static inline pid_t task_pgrp_nr(struct task_struct *tsk) +{
+ return tsk->signal->pgrp;
+}
+
+static inline pid_t task_pgrp_nr_ns(struct task_struct *tsk,
+ struct pid_namespace *ns)
+{
+ return pid_nr_ns(task_pgrp(tsk), ns);
+}
+
+static inline pid_t task_pgrp_vnr(struct task_struct *tsk) +{
+ return pid_vnr(task_pgrp(tsk));
+}
+
+
+static inline pid_t task_session_nr(struct task_struct *tsk) +{
+ return tsk->signal->__session;
+}
+
+static inline pid_t task_session_nr_ns(struct task_struct *tsk,
+ struct pid_namespace *ns)
+{
+ return pid_nr_ns(task_session(tsk), ns);
+}
+
+static inline pid_t task_session_vnr(struct task_struct *tsk) +{
+ return pid_vnr(task_session(tsk));
+}
+
+
+static inline pid_t task_ppid_nr_ns(struct task_struct *tsk,
+ struct pid_namespace *ns)
+{
+ return pid_nr_ns(task_pid(rcu_dereference(tsk->real_parent)), ns);
+}
+
/**
* pid_alive - check that a task structure is not stale
* @p: Task structure to be checked.

diff -upr linux-2.6.23-rc1-mm1.orig/kernel/pid.c linux-2.6.23-rc1-mm1-7/kernel/pid.c
@@ -352,6 +436,14 @@ struct pid *find_get_pid(pid_t nr)

	return pid;
}

+pid_t pid_nr_ns(struct pid *pid, struct pid_namespace *ns)
+{
+ pid_t nr = 0;
+ if (pid && ns->level <= pid->level)
+ nr = pid->numbers[ns->level].nr;
+ return nr;
+}
+
/*
* Used by proc to find the first pid that is greater then or equal to nr. *

Can we give these some better names? I think "virtual" is pretty bad, especially if you consider the multiple level of pid namespaces that we might have some day. Processes can belong to multiple pid namespaces, and thus have multiple "virtual" ids.

Even though it will make the names longer, I think we need something in the names to say that "pid_nr()" is the top-level, global, init_pid_ns number. "pid_vnr()" is the pid for the lowest pid namespace in the hierarchy. Suka called this an "active pid namespace" because that is where the task actively interacts with its peers. But, I'm open to other suggestions, too.

When writing code, people are going to need to know which one to use:
pid_nr() or pid_vnr(). We can document the functions, but the names will help much more than any documentation.

-- Dave

We do have them now (multiple levels). The "virtual" stands for "the one that task sees by his own".

That's it.

I do not mind, but what other names can we have? pid_nr() for init namespace, pid_nr_ns for arbitrary namespace and pid_<what> for the lower-level namespace?

Thanks,
Pavel

pid_nr, find_task_by_pid and all other stuff, that existed in the kernel before the set are intended to work with global ids only (just as it was before the set).

Yes, that's a check for bad namespace passed.

Yes, we should, and you're right in that we must have BUG_ON() here.

Thanks,
Pavel

When searching the task by numerical id on may need to find it using global pid (as it is done now in kernel) or by its virtual id, e.g. when sending a signal to a task from one namespace the sender will specify the task's virtual id.

---

include/linux/pid.h   |   16 ++++++++++++++--
include/linux/sched.h |   31 +++++++++++++++++++++++++++++--
kernel/pid.c          |   32 +++++++++++++++++---------------
3 files changed, 60 insertions(+), 19 deletions(-)

diff -upr linux-2.6.23-rc1-mm1.orig/include/linux/pid.h linux-2.6.23-rc1-mm1-7/include/linux/pid.h
@@ -83,17 +92,29 @@ extern void FASTCALL(detach_pid(struct t

extern void FASTCALL(transfer_pid(struct task_struct *old,

				  struct task_struct *new, enum pid_type));

+struct pid_namespace;
+extern struct pid_namespace init_pid_ns;
+
/*
* look up a PID in the hash table. Must be called with the tasklist_lock
* or rcu_read_lock() held.
+ *
+ * find_pid_ns() finds the pid in the namespace specified
+ * find_pid() find the pid by its global id, i.e. in the init namespace
+ * find_vpid() finr the pid by its virtual id, i.e. in the current namespace
+ *
+ * see also find_task_by_pid() set in include/linux/sched.h
*/
-extern struct pid *FASTCALL(find_pid(int nr));
+extern struct pid *FASTCALL(find_pid_ns(int nr, struct pid_namespace *ns)); +
+#define find_vpid(pid) find_pid_ns(pid, current->nsproxy->pid_ns)
+#define find_pid(pid) find_pid_ns(pid, &init_pid_ns)

/*
* Lookup a PID in the hash table, and return with it's count elevated. */
extern struct pid *find_get_pid(int nr);
-extern struct pid *find_ge_pid(int nr);
+extern struct pid *find_ge_pid(int nr, struct pid_namespace *);

extern struct pid *alloc_pid(struct pid_namespace *ns); extern void FASTCALL(free_pid(struct pid *pid));

diff -upr linux-2.6.23-rc1-mm1.orig/include/linux/sched.h linux-2.6.23-rc1-mm1-7/include/linux/sched.h
@@ -1453,8 +1542,35 @@ extern struct task_struct init_task;
 

extern struct mm_struct init_mm;

-#define find_task_by_pid(nr) find_task_by_pid_type(PIDTYPE_PID, nr)
-extern struct task_struct *find_task_by_pid_type(int type, int pid); +extern struct pid_namespace init_pid_ns;
+
+/*
+ * find a task by one of its numerical ids
+ *
+ * find_task_by_pid_type_ns():
+ * it is the most generic call - it finds a task by all id,
+ * type and namespace specified
+ * find_task_by_pid_ns():
+ * finds a task by its pid in the specified namespace
+ * find_task_by_pid_type():
+ * finds a task by its global id with the specified type, e.g.
+ * by global session id
+ * find_task_by_pid():
+ * finds a task by its global pid
+ *
+ * see also find_pid() etc in include/linux/pid.h
+ */
+
+extern struct task_struct *find_task_by_pid_type_ns(int type, int pid,
+ struct pid_namespace *ns);
+
+#define find_task_by_pid_ns(nr, ns) \
+ find_task_by_pid_type_ns(PIDTYPE_PID, nr, ns)
+#define find_task_by_pid_type(type, nr) \
+ find_task_by_pid_type_ns(type, nr, &init_pid_ns)
+#define find_task_by_pid(nr) \
+ find_task_by_pid_type(PIDTYPE_PID, nr)
+
extern void __set_special_pids(pid_t session, pid_t pgrp);

/* per-UID process charging. */

diff -upr linux-2.6.23-rc1-mm1.orig/kernel/pid.c linux-2.6.23-rc1-mm1-7/kernel/pid.c
@@ -204,19 +221,20 @@ static void delayed_put_pid(struct rcu_h

	goto out;
}

-struct pid * fastcall find_pid(int nr)
+struct pid * fastcall find_pid_ns(int nr, struct pid_namespace *ns)

{
	struct hlist_node *elem;

- struct pid *pid;
+ struct upid *pnr;
+
+ hlist_for_each_entry_rcu(pnr, elem,
+ &pid_hash[pid_hashfn(nr, ns)], pid_chain)
+ if (pnr->nr == nr && pnr->ns == ns)
+ return container_of(pnr, struct pid,
+ numbers[ns->level]);

- hlist_for_each_entry_rcu(pid, elem,
- &pid_hash[pid_hashfn(nr)], pid_chain) {
- if (pid->nr == nr)
- return pid;
- }

	return NULL;
}

-EXPORT_SYMBOL_GPL(find_pid);
+EXPORT_SYMBOL_GPL(find_pid_ns);

/*
* attach_pid() must be called with the tasklist_lock write-held.
@@ -318,12 +355,13 @@ struct task_struct * fastcall pid_task(s /*
* Must be called under rcu_read_lock() or with tasklist_lock read-held. */
-struct task_struct *find_task_by_pid_type(int type, int nr) +struct task_struct *find_task_by_pid_type_ns(int type, int nr,
+ struct pid_namespace *ns)
{
- return pid_task(find_pid(nr), type);
+ return pid_task(find_pid_ns(nr, ns), type);
}

-EXPORT_SYMBOL(find_task_by_pid_type);
+EXPORT_SYMBOL(find_task_by_pid_type_ns);

struct pid *get_task_pid(struct task_struct *task, enum pid_type type) {
@@ -342,7 +426,7 @@ struct pid *find_get_pid(pid_t nr)

	struct pid *pid;

	rcu_read_lock();
-	pid = get_pid(find_pid(nr));
+	pid = get_pid(find_vpid(nr));
	rcu_read_unlock();

	return pid;

@@ -352,15 +436,15 @@ struct pid *find_get_pid(pid_t nr)
*
* If there is a pid at nr this function is exactly the same as find_pid. */
-struct pid *find_ge_pid(int nr)
+struct pid *find_ge_pid(int nr, struct pid_namespace *ns)

{
	struct pid *pid;

	do {
-		pid = find_pid(nr);
+		pid = find_pid_ns(nr, ns);
		if (pid)
			break;
-		nr = next_pidmap(task_active_pid_ns(current), nr);
+		nr = next_pidmap(ns, nr);
	} while (nr > 0);

	return pid;

No, but why not?
I can make them static inline functions, but why are macros that bad?

No. Iteration over the hash chain happens here only.

I'd better make it find_get_pid_ns() with two arguments and made a couple of macros (or static inlines) for global search and local search.

Thanks,
Pavel

Again, I think find_pid() should use current's active ns, not init_pid_ns. Just grep for find_pid/find_task_by_pid.

                                     ^^^^^^^^^^^^^

That's it :)

Hm... Worth thinking about, but this hash itself is large enough and its size depends on the node's number of pages, so we'll have

1. either to make per-namespace hash (much) smaller;
2. or to give (too) many memory for it.

Sorry :)

Thank,
Pavel

When we create new namespace we will need to allocate the struct pid, that will have one extra struct upid in array, comparing to the parent. Thus we need to know the new namespace (if any) in alloc_pid() to init this struct upid properly, so move the alloc_pid() call lower in copy_process().

This is a fix for Sukadev's patch that moved the alloc_pid() call from do_fork() into copy_process().

---

fork.c | 59 ++++++++++++++++++++++++++++++++++++++---------------------

1 files changed, 38 insertions(+), 21 deletions(-)

diff -upr linux-2.6.23-rc1-mm1.orig/kernel/fork.c linux-2.6.23-rc1-mm1-7/kernel/fork.c
@@ -1028,16 +1029,9 @@ static struct task_struct *copy_process(

	if (p->binfmt && !try_module_get(p->binfmt->module))
		goto bad_fork_cleanup_put_domain;

- if (pid != &init_struct_pid) {
- pid = alloc_pid();
- if (!pid)
- goto bad_fork_put_binfmt_module;
- }
-

	p->did_exec = 0;
	delayacct_tsk_init(p);	/* Must remain after dup_task_struct() */
	copy_flags(clone_flags, p);
-	p->pid = pid_nr(pid);
	INIT_LIST_HEAD(&p->children);
	INIT_LIST_HEAD(&p->sibling);
	p->vfork_done = NULL;
@@ -1112,10 +1106,6 @@ static struct task_struct *copy_process(
	p->blocked_on = NULL; /* not blocked yet */

#endif

- p->tgid = p->pid;
- if (clone_flags & CLONE_THREAD)
- p->tgid = current->tgid;
-

	if ((retval = security_task_alloc(p)))
		goto bad_fork_cleanup_policy;
	if ((retval = audit_alloc(p)))
@@ -1141,6 +1131,17 @@ static struct task_struct *copy_process(
	if (retval)
		goto bad_fork_cleanup_namespaces;

+ if (pid != &init_struct_pid) {
+ pid = alloc_pid(task_active_pid_ns(p));
+ if (!pid)
+ goto bad_fork_cleanup_namespaces;
+ }
+
+ p->pid = pid_nr(pid);
+ p->tgid = p->pid;
+ if (clone_flags & CLONE_THREAD)
+ p->tgid = current->tgid;
+

	p->set_child_tid = (clone_flags & CLONE_CHILD_SETTID) ? child_tidptr : NULL;
	/*
	 * Clear TID on mm_release()?
@@ -1237,7 +1242,7 @@ static struct task_struct *copy_process(
		spin_unlock(&current->sighand->siglock);
		write_unlock_irq(&tasklist_lock);
		retval = -ERESTARTNOINTR;
-		goto bad_fork_cleanup_namespaces;
+		goto bad_fork_free_pid;
	}

	if (clone_flags & CLONE_THREAD) {
@@ -1266,11 +1271,22 @@ static struct task_struct *copy_process(
			__ptrace_link(p, current->parent);

		if (thread_group_leader(p)) {

- p->signal->tty = current->signal->tty;
- p->signal->pgrp = task_pgrp_nr(current);
- set_task_session(p, task_session_nr(current));
- attach_pid(p, PIDTYPE_PGID, task_pgrp(current));
- attach_pid(p, PIDTYPE_SID, task_session(current));
+ if (clone_flags & CLONE_NEWPID) {
+ p->nsproxy->pid_ns->child_reaper = p;
+ p->signal->tty = NULL;
+ p->signal->pgrp = p->pid;
+ set_task_session(p, p->pid);
+ attach_pid(p, PIDTYPE_PGID, pid);
+ attach_pid(p, PIDTYPE_SID, pid);
+ } else {
+ p->signal->tty = current->signal->tty;
+ p->signal->pgrp = task_pgrp_nr(current);
+ set_task_session(p, task_session_nr(current));
+ attach_pid(p, PIDTYPE_PGID,
+ task_pgrp(current));
+ attach_pid(p, PIDTYPE_SID,
+ task_session(current));
+ }

			list_add_tail_rcu(&p->tasks, &init_task.tasks);
			__get_cpu_var(process_counts)++;
@@ -1288,6 +1304,9 @@ static struct task_struct *copy_process(
	container_post_fork(p);
	return p;

+bad_fork_free_pid:
+ if (pid != &init_struct_pid)
+ free_pid(pid);
bad_fork_cleanup_namespaces:

	exit_task_namespaces(p);
bad_fork_cleanup_keys:
@@ -1322,9 +1341,6 @@ bad_fork_cleanup_container:
#endif
	container_exit(p, container_callbacks_done);
	delayacct_tsk_free(p);
-	if (pid != &init_struct_pid)
-		free_pid(pid);
-bad_fork_put_binfmt_module:
	if (p->binfmt)
		module_put(p->binfmt->module);
bad_fork_cleanup_put_domain:
@@ -1406,7 +1422,13 @@ long do_fork(unsigned long clone_flags,
	if (!IS_ERR(p)) {
		struct completion vfork;

- nr = pid_nr(task_pid(p));
+ /*
+ * this is enough to call pid_nr_ns here, but this if
+ * improves optimisation of regular fork()
+ */
+ nr = (clone_flags & CLONE_NEWPID) ?
+ task_pid_nr_ns(p, current->nsproxy->pid_ns) :
+ task_pid_vnr(p);

		if (clone_flags & CLONE_VFORK) {
			p->vfork_done = &vfork;

... and this patch changes almost every line from Sukadev's patch. Sorry gents, but isn't it better to ask Andrew to drop that patch (which is quite useless by itself), and send a new one which incorporates all necessary changes? Imho, it would be much easier to understand.

It does. My bad :( I have reviewed Suka's patch badly and was sure it puts the alloc_pid() right where we need this.

Hm... Maybe it's better to ask him to fold these patches together?

Oops. We should. Thanks :)

Thanks,
Pavel

Pavel Emelianov [[email protected]] wrote:

| Oleg Nesterov wrote:
| >On 07/26, Pavel Emelyanov wrote:
| >>This is a fix for Sukadev's patch that moved the alloc_pid() call from
| >>do_fork() into copy_process().
| >
| >... and this patch changes almost every line from Sukadev's patch.
| 
| It does. My bad :( I have reviewed Suka's patch badly and was sure it
| puts the alloc_pid() right where we need this.

I should have reviewed Pavel's closely too. Sorry.

| 
| >Sorry gents, but isn't it better to ask Andrew to drop that patch
| >(which is quite useless by itself), and send a new one which incorporates
| >all necessary changes? Imho, it would be much easier to understand.
| 
| Hm... Maybe it's better to ask him to fold these patches together?

I think so, but even dropping my patch is fine with me.

| 
| >>@@ -1406,7 +1422,13 @@ long do_fork(unsigned long clone_flags,
| >>	if (!IS_ERR(p)) {
| >>		struct completion vfork;
| >>
| >>-		nr = pid_nr(task_pid(p));
| >>+		/*
| >>+		 * this is enough to call pid_nr_ns here, but this if
| >>+		 * improves optimisation of regular fork()
| >>+		 */
| >>+		nr = (clone_flags & CLONE_NEWPID) ?
| >>+			task_pid_nr_ns(p, current->nsproxy->pid_ns) :
| >>+				task_pid_vnr(p);
| >
| >Shouldn't we do the same for CLONE_PARENT_SETTID in copy_process() ?
| >Otherwise *parent_tidptr may have a wrong value which doesn't match
| >to what fork() returns.
| 
| Oops. We should. Thanks :)
| 
| >Oleg.
| 
| Thanks,
| Pavel

Pavel Emelianov [[email protected]] wrote:

| When we create new namespace we will need to allocate the struct pid,
| that will have one extra struct upid in array, comparing to the parent.
| Thus we need to know the new namespace (if any) in alloc_pid() to init
| this struct upid properly, so move the alloc_pid() call lower in
| copy_process().
| 
| This is a fix for Sukadev's patch that moved the alloc_pid() call from
| do_fork() into copy_process().
| 
| Signed-off-by: Pavel Emelyanov <[email protected]>
| 
| ---
| 
| fork.c |   59 ++++++++++++++++++++++++++++++++++++++---------------------
| 1 files changed, 38 insertions(+), 21 deletions(-)
| 
| diff -upr linux-2.6.23-rc1-mm1.orig/kernel/fork.c 
| linux-2.6.23-rc1-mm1-7/kernel/fork.c
| --- linux-2.6.23-rc1-mm1.orig/kernel/fork.c	2007-07-26 
| 16:34:45.000000000 +0400
| +++ linux-2.6.23-rc1-mm1-7/kernel/fork.c	2007-07-26 
| 16:36:37.000000000 +0400
| @@ -1028,16 +1029,9 @@ static struct task_struct *copy_process(
| 	if (p->binfmt && !try_module_get(p->binfmt->module))
| 		goto bad_fork_cleanup_put_domain;
| 
| -	if (pid != &init_struct_pid) {
| -		pid = alloc_pid();
| -		if (!pid)
| -			goto bad_fork_put_binfmt_module;
| -	}
| -
| 	p->did_exec = 0;
| 	delayacct_tsk_init(p);	/* Must remain after dup_task_struct() */
| 	copy_flags(clone_flags, p);
| -	p->pid = pid_nr(pid);
| 	INIT_LIST_HEAD(&p->children);
| 	INIT_LIST_HEAD(&p->sibling);
| 	p->vfork_done = NULL;
| @@ -1112,10 +1106,6 @@ static struct task_struct *copy_process(
| 	p->blocked_on = NULL; /* not blocked yet */
| #endif
| 
| -	p->tgid = p->pid;
| -	if (clone_flags & CLONE_THREAD)
| -		p->tgid = current->tgid;
| -
| 	if ((retval = security_task_alloc(p)))
| 		goto bad_fork_cleanup_policy;
| 	if ((retval = audit_alloc(p)))
| @@ -1141,6 +1131,17 @@ static struct task_struct *copy_process(
| 	if (retval)
| 		goto bad_fork_cleanup_namespaces;
| 
| +	if (pid != &init_struct_pid) {
| +		pid = alloc_pid(task_active_pid_ns(p));
| +		if (!pid)
| +			goto bad_fork_cleanup_namespaces;

We should set retval to -EAGAIN before this goto. Otherwise, when we take this goto, retval would be 0 (bc copy_thread() succeeded) and we crash in the caller.

| +	}
| +
| +	p->pid = pid_nr(pid);
| +	p->tgid = p->pid;
| +	if (clone_flags & CLONE_THREAD)
| +		p->tgid = current->tgid;
| +
| 	p->set_child_tid = (clone_flags & CLONE_CHILD_SETTID) ? child_tidptr 
| 	: NULL;
| 	/*
| 	 * Clear TID on mm_release()?
| @@ -1237,7 +1242,7 @@ static struct task_struct *copy_process(
| 		spin_unlock(&current->sighand->siglock);
| 		write_unlock_irq(&tasklist_lock);
| 		retval = -ERESTARTNOINTR;
| -		goto bad_fork_cleanup_namespaces;
| +		goto bad_fork_free_pid;
| 	}
| 
| 	if (clone_flags & CLONE_THREAD) {
| @@ -1266,11 +1271,22 @@ static struct task_struct *copy_process(
| 			__ptrace_link(p, current->parent);
| 
| 		if (thread_group_leader(p)) {
| -			p->signal->tty = current->signal->tty;
| -			p->signal->pgrp = task_pgrp_nr(current);
| -			set_task_session(p, task_session_nr(current));
| -			attach_pid(p, PIDTYPE_PGID, task_pgrp(current));
| -			attach_pid(p, PIDTYPE_SID, task_session(current));
| +			if (clone_flags & CLONE_NEWPID) {
| +				p->nsproxy->pid_ns->child_reaper = p;
| +				p->signal->tty = NULL;
| +				p->signal->pgrp = p->pid;
| +				set_task_session(p, p->pid);
| +				attach_pid(p, PIDTYPE_PGID, pid);
| +				attach_pid(p, PIDTYPE_SID, pid);
| +			} else {
| +				p->signal->tty = current->signal->tty;
| +				p->signal->pgrp = task_pgrp_nr(current);
| +				set_task_session(p, 
| task_session_nr(current));
| +				attach_pid(p, PIDTYPE_PGID,
| +						task_pgrp(current));
| +				attach_pid(p, PIDTYPE_SID,
| +						task_session(current));
| +			}
| 
| 			list_add_tail_rcu(&p->tasks, &init_task.tasks);
| 			__get_cpu_var(process_counts)++;
| @@ -1288,6 +1304,9 @@ static struct task_struct *copy_process(
| 	container_post_fork(p);
| 	return p;
| 
| +bad_fork_free_pid:
| +	if (pid != &init_struct_pid)
| +		free_pid(pid);
| bad_fork_cleanup_namespaces:
| 	exit_task_namespaces(p);
| bad_fork_cleanup_keys:
| @@ -1322,9 +1341,6 @@ bad_fork_cleanup_container:
| #endif
| 	container_exit(p, container_callbacks_done);
| 	delayacct_tsk_free(p);
| -	if (pid != &init_struct_pid)
| -		free_pid(pid);
| -bad_fork_put_binfmt_module:
| 	if (p->binfmt)
| 		module_put(p->binfmt->module);
| bad_fork_cleanup_put_domain:
| @@ -1406,7 +1422,13 @@ long do_fork(unsigned long clone_flags,
| 	if (!IS_ERR(p)) {
| 		struct completion vfork;
| 
| -		nr = pid_nr(task_pid(p));
| +		/*
| +		 * this is enough to call pid_nr_ns here, but this if
| +		 * improves optimisation of regular fork()
| +		 */
| +		nr = (clone_flags & CLONE_NEWPID) ?
| +			task_pid_nr_ns(p, current->nsproxy->pid_ns) :
| +				task_pid_vnr(p);
| 
| 		if (clone_flags & CLONE_VFORK) {
| 			p->vfork_done = &vfork;

The idea is the following: when a namespace is created the new proc superblock, pointing to this namespace, is created. When user accesses the proc tree from some pountmoint it oberves the tasks that live in the namespace, pointer by the according spuperblock.

When task dies, its pids must be flushed from several proc trees, i.e. the ones that are owned by the namespaces that can see the taks's struct pid.

Having the namespaces live without the proc mount is racy, so during the namespace creation we kern_mount the new proc instance.

We cannot just get the reference on the struct pid_namespace from the superblock, otherwise we'll have the ns->proc_mnt->sb->ns loop. To break this, when the init task dies, it releases the kern_mount-ed instance.

---

fs/proc/base.c                |   12 ++++++
fs/proc/inode.c               |    3 +
fs/proc/root.c                |   83 +++++++++++++++++++++++++++++++++++++++---
include/linux/pid_namespace.h |    3 +
include/linux/proc_fs.h       |   15 +++++++
kernel/exit.c                 |   18 ++++++++-
kernel/fork.c                 |    5 ++
kernel/pid.c                  |    1 8 files changed, 133 insertions(+), 7 deletions(-)

diff -upr linux-2.6.23-rc1-mm1.orig/fs/proc/base.c linux-2.6.23-rc1-mm1-7/fs/proc/base.c
@@ -2161,7 +2162,19 @@ out:
 

void proc_flush_task(struct task_struct *task, struct pid *pid) {
+ int i;
+ struct upid *upid;
+

	proc_flush_task_mnt(task, proc_mnt);

+ if (pid == NULL)
+ return;
+
+ for (i = 1; i <= pid->level; i++)
+ proc_flush_task_mnt(task, pid->numbers[i].ns->proc_mnt);
+
+ upid = &pid->numbers[pid->level];
+ if (upid->nr == 1)
+ pid_ns_release_proc(upid->ns);
}

static struct dentry *proc_pid_instantiate(struct inode *dir,

diff -upr linux-2.6.23-rc1-mm1.orig/fs/proc/inode.c linux-2.6.23-rc1-mm1-7/fs/proc/inode.c
@@ -16,6 +16,7 @@

#include <linux/init.h>
#include <linux/module.h>
#include <linux/smp_lock.h>
+#include <linux/pid_namespace.h>

#include <asm/system.h>
#include <asm/uaccess.h>
@@ -427,7 +428,7 @@ out_mod:

	return NULL;
}			

-int proc_fill_super(struct super_block *s, void *data, int silent) +int proc_fill_super(struct super_block *s)

{
	struct inode * root_inode;

diff -upr linux-2.6.23-rc1-mm1.orig/fs/proc/root.c linux-2.6.23-rc1-mm1-7/fs/proc/root.c
@@ -18,32 +18,90 @@

#include <linux/bitops.h>
#include <linux/smp_lock.h>
#include <linux/mount.h>
+#include <linux/pid_namespace.h>

#include "internal.h"

struct proc_dir_entry *proc_net, *proc_net_stat, *proc_bus, *proc_root_fs, *proc_root_driver;

+static int proc_test_super(struct super_block *sb, void *data) +{
+ return sb->s_fs_info == data;
+}
+
+static int proc_set_super(struct super_block *sb, void *data) +{
+ struct pid_namespace *ns;
+
+ ns = (struct pid_namespace *)data;
+ sb->s_fs_info = get_pid_ns(ns);
+ return set_anon_super(sb, NULL);
+}
+
static int proc_get_sb(struct file_system_type *fs_type,
int flags, const char *dev_name, void *data, struct vfsmount *mnt)
{
+ int err;
+ struct super_block *sb;
+ struct pid_namespace *ns;
+ struct proc_inode *ei;
+

	if (proc_mnt) {
		/* Seed the root directory with a pid so it doesn't need
		 * to be special in base.c.  I would do this earlier but
		 * the only task alive when /proc is mounted the first time
		 * is the init_task and it doesn't have any pids.
		 */
-		struct proc_inode *ei;
		ei = PROC_I(proc_mnt->mnt_sb->s_root->d_inode);
		if (!ei->pid)
			ei->pid = find_get_pid(1);
	}

- return get_sb_single(fs_type, flags, data, proc_fill_super, mnt);
+
+ if (flags & MS_KERNMOUNT)
+ ns = (struct pid_namespace *)data;
+ else
+ ns = current->nsproxy->pid_ns;
+
+ sb = sget(fs_type, proc_test_super, proc_set_super, ns);
+ if (IS_ERR(sb))
+ return PTR_ERR(sb);
+
+ if (!sb->s_root) {
+ sb->s_flags = flags;
+ err = proc_fill_super(sb);
+ if (err) {
+ up_write(&sb->s_umount);
+ deactivate_super(sb);
+ return err;
+ }
+
+ ei = PROC_I(sb->s_root->d_inode);
+ if (!ei->pid) {
+ rcu_read_lock();
+ ei->pid = get_pid(find_pid_ns(1, ns));
+ rcu_read_unlock();
+ }
+
+ sb->s_flags |= MS_ACTIVE;
+ ns->proc_mnt = mnt;
+ }
+
+ return simple_set_mnt(mnt, sb);
+}
+
+static void proc_kill_sb(struct super_block *sb)
+{
+ struct pid_namespace *ns;
+
+ ns = (struct pid_namespace *)sb->s_fs_info;
+ kill_anon_super(sb);
+ put_pid_ns(ns);
}

static struct file_system_type proc_fs_type = {
	.name		= "proc",
	.get_sb		= proc_get_sb,

- .kill_sb = kill_anon_super,
+ .kill_sb = proc_kill_sb,
};

void __init proc_root_init(void)
@@ -54,12 +112,13 @@ void __init proc_root_init(void)

	err = register_filesystem(&proc_fs_type);
	if (err)
		return;
-	proc_mnt = kern_mount(&proc_fs_type);
+	proc_mnt = kern_mount_data(&proc_fs_type, &init_pid_ns);
	err = PTR_ERR(proc_mnt);
	if (IS_ERR(proc_mnt)) {
		unregister_filesystem(&proc_fs_type);
		return;
	}
+
	proc_misc_init();
	proc_net = proc_mkdir("net", NULL);
	proc_net_stat = proc_mkdir("net/stat", NULL);
@@ -153,6 +212,22 @@ struct proc_dir_entry proc_root = {
	.parent		= &proc_root,
};

+int pid_ns_prepare_proc(struct pid_namespace *ns) +{
+ struct vfsmount *mnt;
+
+ mnt = kern_mount_data(&proc_fs_type, ns);
+ if (IS_ERR(mnt))
+ return PTR_ERR(mnt);
+
+ return 0;
+}
+
+void pid_ns_release_proc(struct pid_namespace *ns) +{
+ mntput(ns->proc_mnt);
+}
+
EXPORT_SYMBOL(proc_symlink); EXPORT_SYMBOL(proc_mkdir); EXPORT_SYMBOL(create_proc_entry);

diff -upr linux-2.6.23-rc1-mm1.orig/include/linux/pid_namespace.h linux-2.6.23-rc1-mm1-7/include/linux/pid_namespace.h
@@ -16,6 +15,9 @@ struct pidmap {

	int last_pid;
	struct task_struct *child_reaper;
	struct kmem_cache *pid_cachep;

+#ifdef CONFIG_PROC_FS
+ struct vfsmount *proc_mnt;
+#endif
};

extern struct pid_namespace init_pid_ns;

diff -upr linux-2.6.23-rc1-mm1.orig/include/linux/proc_fs.h linux-2.6.23-rc1-mm1-7/include/linux/proc_fs.h
@@ -126,7 +126,8 @@ extern struct proc_dir_entry *create_pro

extern void remove_proc_entry(const char *name, struct proc_dir_entry *parent);

extern struct vfsmount *proc_mnt;
-extern int proc_fill_super(struct super_block *,void *,int);
+struct pid_namespace;
+extern int proc_fill_super(struct super_block *);
extern struct inode *proc_get_inode(struct super_block *, unsigned int, struct proc_dir_entry *);

/*
@@ -143,6 +144,9 @@ extern const struct file_operations proc extern const struct file_operations proc_kmsg_operations; extern const struct file_operations ppc_htab_operations;

+extern int pid_ns_prepare_proc(struct pid_namespace *ns); +extern void pid_ns_release_proc(struct pid_namespace *ns); +
/*
* proc_tty.c
*/
@@ -248,6 +254,15 @@ static inline void proc_tty_unregister_d

extern struct proc_dir_entry proc_root;

+static inline int pid_ns_prepare_proc(struct pid_namespace *ns) +{
+ return 0;
+}
+
+static inline void pid_ns_release_proc(struct pid_namespace *ns) +{
+}
+
#endif /* CONFIG_PROC_FS */

#if !defined(CONFIG_PROC_KCORE)

diff -upr linux-2.6.23-rc1-mm1.orig/kernel/exit.c linux-2.6.23-rc1-mm1-7/kernel/exit.c
@@ -153,6 +153,7 @@ static void delayed_put_task_struct(stru
 

void release_task(struct task_struct * p)
{

+	struct pid *pid;
	struct task_struct *leader;
	int zap_leader;
repeat:
@@ -160,6 +161,20 @@ repeat:
	write_lock_irq(&tasklist_lock);
	ptrace_unlink(p);
	BUG_ON(!list_empty(&p->ptrace_list) || !list_empty(&p->ptrace_children));

+ /*
+ * we have to keep this pid till proc_flush_task() to make
+ * it possible to flush all dentries holding it. pid will
+ * be put ibidem
+ *
+ * however if the pid belogs to init namespace only, we can
+ * optimize this out
+ */
+ pid = task_pid(p);
+ if (pid->level != 0)
+ get_pid(pid);
+ else
+ pid = NULL;
+

	__exit_signal(p);

	/*
@@ -184,7 +199,8 @@ repeat:
	}

	write_unlock_irq(&tasklist_lock);
-	proc_flush_task(p, NULL);
+	proc_flush_task(p, pid);
+	put_pid(pid);
	release_thread(p);
	call_rcu(&p->rcu, delayed_put_task_struct);

diff -upr linux-2.6.23-rc1-mm1.orig/kernel/fork.c linux-2.6.23-rc1-mm1-7/kernel/fork.c
@@ -50,6 +50,7 @@

#include <linux/taskstats_kern.h>
#include <linux/random.h>
#include <linux/tty.h>
+#include <linux/proc_fs.h>

#include <asm/pgtable.h>
#include <asm/pgalloc.h>
@@ -1141,6 +1131,10 @@ static struct task_struct *copy_process(

		pid = alloc_pid(task_active_pid_ns(p));
		if (!pid)
			goto bad_fork_cleanup_namespaces;
+
+		if (clone_flags & CLONE_NEWPID)
+			if (pid_ns_prepare_proc(task_active_pid_ns(p)))
+				goto bad_fork_free_pid;
	}

	p->pid = pid_nr(pid);

diff -upr linux-2.6.23-rc1-mm1.orig/kernel/pid.c linux-2.6.23-rc1-mm1-7/kernel/pid.c
@@ -28,6 +28,7 @@

#include <linux/hash.h>
#include <linux/pid_namespace.h>
#include <linux/init_task.h>
+#include <linux/proc_fs.h>

#define pid_hashfn(nr, ns) \
hash_long((unsigned long)nr + (unsigned long)ns, pidhash_shift)

Oops. Yes, proc_flush_task() doesn't use ->numbers[].nr, but it should?

proc_flush_task_mnt() uses task->pid to flush /proc/$pid, this looks wrong, we should use ->number[ns->level].nr, no?

Oleg.

Indeed. Hmm, why then pids were released during my tests... Looks like we do still need struct pid for proc_flush_task().

Thanks,
Pavel

In proc_delete_inode()

It cannot. By the time release_task() is called the task is already exit_task_namespaces()-ed :(

So, that's what you mean. Well, that's sounds reasonable. Thanks.

Yes. Thanks!

Pavel

From: Sukadev Bhattiprolu <sukadev-r/[email protected]>

With support for multiple pid namespaces, each pid namespace has a separate child reaper and this process needs some special handling of signals.

- The child reaper should appear like a normal process to other processes in its ancestor namespaces and so should be killable (or not) in the usual way.

- The child reaper should receive, from processes in it's active and decendent namespaces, only those signals for which it has installed a signal handler.

- System-wide signals (eg: kill signum -1) from within a child namespace should only affect processes within that namespace and descendant namespaces. They should not be posted to processes in ancestor or sibling namespaces.

- If the sender of a signal does not have a pid_t in the receiver's namespace (eg: a process in init_pid_ns sends a signal to a process in a descendant namespace), the sender's pid should appear as 0 in the signal's siginfo structure.

- Existing rules for SIGIO delivery still apply and a process can choose any other process in its namespace and descendant namespaces to receive the SIGIO signal.

The following appears to be incorrect in the fcntl() man page for F_SETOWN.

Sending a signal to the owner process (group) specified by F_SETOWN is subject to the same permissions checks as are described for kill(2), where the sending process is the one that employs F_SETOWN (but see BUGS below).

Current behavior is that the SIGIO signal is delivered on behalf of the process that caused the event (eg: made data available on the file) and not the process that called fcntl().

To implement the above requirements, we:

- Add a check in check_kill_permission() for a process within a namespace sending the fast-pathed, SIGKILL signal.

- We use a flag, SIGQUEUE_CINIT, to tell the container-init if a signal posted to its queue is from a process within its own namespace. The flag is set in send_signal() if a process attempts to send a signal to its container-init.

The SIGQUEUE_CINIT flag is checked in collect_signal() - if the flag is set, collect_signal() sets the KERN_SIGINFO_CINIT flag in the kern_siginfo. The KERN_SIGINFO_CINIT flag indicates that the sender is from within the namespace and the container-init can choose to ignore the signal.

If the KERN_SIGINFO_CINIT flag is clear in get_signal_to_deliver(), the signal originated from an ancestor namespace and so the container-init honors the signal.

Note: We currently use two flags, SIGQUEUE_CINIT, KERN_SIGINFO_CINIT to
avoid modifying 'struct sigqueue'. If 'kern_siginfo' approach is feasible, we could use 'kern_siginfo' in sigqueue and eliminate SIGQUEUE_CINIT.

---

include/linux/pid.h    |    3 ++
include/linux/signal.h |    1 kernel/pid.c           |   46 +++++++++++++++++++++++++++++++++++
kernel/signal.c        |   63 ++++++++++++++++++++++++++++++++++++++++++++++++-
4 files changed, 112 insertions(+), 1 deletion(-)

diff -upr linux-2.6.23-rc1-mm1.orig/include/linux/pid.h linux-2.6.23-rc1-mm1-7/include/linux/pid.h
@@ -71,6 +77,9 @@ extern struct task_struct *FASTCALL(pid_

extern struct task_struct *FASTCALL(get_pid_task(struct pid *pid,

						enum pid_type));

+extern int task_visible_in_pid_ns(struct task_struct *tsk,
+ struct pid_namespace *ns);
+extern int pid_ns_equal(struct task_struct *tsk);
extern struct pid *get_task_pid(struct task_struct *task, enum pid_type type);

/*

diff -upr linux-2.6.23-rc1-mm1.orig/include/linux/signal.h linux-2.6.23-rc1-mm1-7/include/linux/signal.h
@@ -20,6 +27,7 @@ struct sigqueue {
 

/* flags values. */
#define SIGQUEUE_PREALLOC 1
+#define SIGQUEUE_CINIT 2

struct sigpending {
	struct list_head list;

diff -upr linux-2.6.23-rc1-mm1.orig/kernel/pid.c linux-2.6.23-rc1-mm1-7/kernel/pid.c
@@ -318,6 +355,52 @@ struct task_struct * fastcall pid_task(s

}

/*
+ * Return TRUE if the task @p is visible in the pid namespace @ns
+ *
+ * Note: @p is visible in @ns if the active-pid-ns of @p is either equal to
+ * @ns or is a descendant of @ns.
+ *
+ * @p is not visible in @ns if active-pid-ns of @p is an ancestor of @ns.
+ * Eg: Processes in init-pid-ns are not visible in child pid namespaces.
+ * They should not receive any system-wide signals from a child-pid-
+ * namespace for instance.
+ */
+int task_visible_in_pid_ns(struct task_struct *p, struct pid_namespace *ns) +{
+ int i;
+ struct pid *pid = task_pid(p);
+
+ if (!pid)
+ return 0;
+
+ for (i = 0; i <= pid->level; i++) {
+ if (pid->numbers[i].ns == ns)
+ return 1;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(task_visible_in_pid_ns);
+
+/*
+ * Return TRUE if the active pid namespace of @tsk is same as active
+ * pid namespace of 'current'
+ */
+
+static inline struct pid_namespace *pid_active_ns(struct pid *pid) +{
+ if (pid == NULL)
+ return NULL;
+
+ return pid->numbers[pid->level].ns;
+}
+
+int pid_ns_equal(struct task_struct *tsk)
+{
+ return pid_active_ns(task_pid(tsk)) == pid_active_ns(task_pid(current));
+}
+
+/*
* Must be called under rcu_read_lock() or with tasklist_lock read-held. */
struct task_struct *find_task_by_pid_type_ns(int type, int nr,

diff -upr linux-2.6.23-rc1-mm1.orig/kernel/signal.c linux-2.6.23-rc1-mm1-7/kernel/signal.c
@@ -323,6 +325,9 @@ static int collect_signal(int sig, struc

	if (first) {
		list_del_init(&first->list);
		copy_siginfo(info, &first->info);
+		if (first->flags & SIGQUEUE_CINIT)
+			kinfo->flags |= KERN_SIGINFO_CINIT;
+
		__sigqueue_free(first);
		if (!still_pending)
			sigdelset(&list->signal, sig);
@@ -343,6 +348,8 @@ static int collect_signal(int sig, struc
{
	int sig = next_signal(pending, mask);

+ kinfo->flags &= ~KERN_SIGINFO_CINIT;
+

	if (sig) {
		if (current->notifier) {
			if (sigismember(current->notifier_mask, sig)) {
@@ -522,6 +547,20 @@ static int rm_from_queue(unsigned long m
	return 1;
}

+static int deny_signal_to_container_init(struct task_struct *tsk, int sig) +{
+ /*
+ * If receiver is the container-init of sender and signal is SIGKILL
+ * reject it right-away. If signal is any other one, let the container
+ * init decide (in get_signal_to_deliver()) whether to handle it or
+ * ignore it.
+ */
+ if (is_container_init(tsk) && (sig == SIGKILL) && pid_ns_equal(tsk))
+ return -EPERM;
+
+ return 0;
+}
+
/*
* Bad permissions for sending the signal
*/
@@ -545,6 +584,10 @@ static int check_kill_permission(int sig

	    && !capable(CAP_KILL))
		return error;

+ error = deny_signal_to_container_init(t, sig);
+ if (error)
+ return error;
+

	return security_task_kill(t, info, sig, 0);
}

@@ -659,6 +702,34 @@ static void handle_stop_signal(int sig,

	}
}

+static void encode_sender_info(struct task_struct *t, struct sigqueue *q) +{
+ /*
+ * If sender (i.e 'current') and receiver have the same active
+ * pid namespace and the receiver is the container-init, set the
+ * SIGQUEUE_CINIT flag. This tells the container-init that the
+ * signal originated in its own namespace and so it can choose
+ * to ignore the signal.
+ *
+ * If the receiver is the container-init of a pid namespace,
+ * but the sender is from an ancestor pid namespace, the
+ * container-init cannot ignore the signal. So clear the
+ * SIGQUEUE_CINIT flag in this case.
+ *
+ * Also, if the sender does not have a pid_t in the receiver's
+ * active pid namespace, set si_pid to 0 and pretend it originated
+ * from the kernel.
+ */
+ if (pid_ns_equal(t)) {
+ if (is_container_init(t)) {
+ q->flags |= SIGQUEUE_CINIT;
+ }
+ } else {
+ q->info.si_pid = 0;
+ q->info.si_code = SI_KERNEL;
+ }
+}
+
static int send_signal(int sig, struct siginfo *info, struct task_struct *t,
struct sigpending *signals)
{
@@ -710,6 +781,7 @@ static int send_signal(int sig, struct s

			copy_siginfo(&q->info, info);
			break;
		}
+		encode_sender_info(t, q);
	} else if (!is_si_special(info)) {
		if (sig >= SIGRTMIN && info->si_code != SI_USER)
		/*
@@ -1149,6 +1221,8 @@ EXPORT_SYMBOL_GPL(kill_pid_info_as_uid);
static int kill_something_info(int sig, struct siginfo *info, int pid)
{
	int ret;
+	struct pid_namespace *my_ns = task_active_pid_ns(current);
+
	rcu_read_lock();
	if (!pid) {
		ret = kill_pgrp_info(sig, info, task_pgrp(current));

@@ -1158,6 +1232,13 @@ static int kill_something_info(int sig,

		read_lock(&tasklist_lock);
		for_each_process(p) {

+ /*
+ * System-wide signals apply only to the sender's
+ * pid namespace, unless issued from init_pid_ns.
+ */
+ if (!task_visible_in_pid_ns(p, my_ns))
+ continue;
+

			if (p->pid > 1 && p->tgid != current->tgid) {
				int err = group_send_sig_info(sig, info, p);
				++count;

@@ -1852,7 +1950,7 @@ relock:
* within that pid space. It can of course get signals from
* its parent pid space.
*/
- if (current == task_child_reaper(current))
+ if (kinfo.flags & KERN_SIGINFO_CINIT)

			continue;

		if (sig_kernel_stop(signr)) {

Damn. I don't have time to read these patches today (will try tomorrow), but when I glanced at this patch yesterday I had some suspicions...

Oh, that's OK. I was about to send the set to Andrew only the next week.

This patch is the most strange one and is to be discussed a lot.

We try to do the following two things:
1. signals going from the namespace, that the target task doesn't see must be seen as SI_KERNEL if siginfo is allocated;
2. signals to init of any namespace must be allowed to send from one of the parent namespaces only. From child namespace, init needs only those, that it's ready to handle (SIGCHLD).

As far as I understand Suka's approach (it's his patch, so I may be not 100% correct - it's better to wait for his comments) he is trying to carry the information about the signal up to the get_signal_to_deliver().

As far as the first issue is concerned, the solution is obvious - all the "calculations" can be done at the beginning of sending the signal, but the second issue is a bit more complicated and I have no good ideas of how to solve this :( yet.

Thanks,
Pavel

Yup. That's the problem. It seems to me that the only way to handle init's signals is to check for permissions in the sending path.

ptracing of tasks fro different namespaces is not possible at all, since strace utility determines the fork()-ed child pid from the parent's eax register, which would contain the pid value as this parent sees his child. But if the strace is in different namespace - it won't be able to find this child with the pid value from parent's eax.

Maybe it's worth disabling cross-namespaces ptracing...

Pavel Emelianov [[email protected]] wrote:

| Oleg Nesterov wrote:
| >Damn. I don't have time to read these patches today (will try tomorrow),
| 
| Oh, that's OK. I was about to send the set to Andrew only the next week.
| 
| This patch is the most strange one and is to be discussed a lot.
| 
| We try to do the following two things:
| 1. signals going from the namespace, that the target task doesn't
|   see must be seen as SI_KERNEL if siginfo is allocated;
| 2. signals to init of any namespace must be allowed to send from
|   one of the parent namespaces only. From child namespace, init
|   needs only those, that it's ready to handle (SIGCHLD).

Yes.

| 
| As far as I understand Suka's approach (it's his patch, so I may 
| be not 100% correct - it's better to wait for his comments) he is 
| trying to carry the information about the signal up to the
| get_signal_to_deliver().
| 
| As far as the first issue is concerned, the solution is obvious -
| all the "calculations" can be done at the beginning of sending the
| signal, but the second issue is a bit more complicated and I have
| no good ideas of how to solve this :( yet.

Even I am looking for a better approach.

| 
| Thanks,
| Pavel
| 
| >but when I glanced at this patch yesterday I had some suspicions...
| >
| >On 07/26, Pavel Emelyanov wrote:
| >>+++ linux-2.6.23-rc1-mm1-7/kernel/signal.c	2007-07-26 
| >>16:36:37.000000000 +0400
| >>@@ -323,6 +325,9 @@ static int collect_signal(int sig, struc
| >>	if (first) {
| >>		list_del_init(&first->list);
| >>		copy_siginfo(info, &first->info);
| >>+		if (first->flags & SIGQUEUE_CINIT)
| >>+			kinfo->flags |= KERN_SIGINFO_CINIT;
| >>+
| >>
| >>[...snip...]
| >>
| >>@@ -1852,7 +1950,7 @@ relock:
| >>		 * within that pid space. It can of course get signals from
| >>		 * its parent pid space.
| >>		 */
| >>-		if (current == task_child_reaper(current))
| >>+		if (kinfo.flags & KERN_SIGINFO_CINIT)
| >>			continue;
| >
| >I think the whole idea is broken, it assumes the sender put something into
| >"struct sigqueue".
| 
| Yup. That's the problem. It seems to me that the only way to handle init's
| signals is to check for permissions in the sending path.

We can check permissions in the sending path - and in fact we do check for SIGKILL case (deny_signal_to_container_init() below).

But the receiver knows/decides whether or not the signal is wanted/not. No ?

Are you saying we should check/special case all fatal signals ?

| 
| >Suppose that /sbin/init has no handler for (say) SIGTERM, and we send this
| >signal from the same namespace. send_signal() sets SIGQUEUE_CINIT, but it
| >is lost because __group_complete_signal() silently "converts" sig_fatal()
| >signals to SIGKILL using sigaddset().

Yes, I should have called it out, but this patch currently assumes /sbin/init (or container-init) has a handler for the fatal signals like SIGTERM and has a check for SIGKILL (in deny_signal_to_container_init() - as Oleg noted below).

Still looking for better ways to implement.

| >
| >>+static void encode_sender_info(struct task_struct *t, struct sigqueue *q)
| >>+{
| >>+	/*
| >>+	 * If sender (i.e 'current') and receiver have the same active
| >>+	 * pid namespace and the receiver is the container-init, set the
| >>+	 * SIGQUEUE_CINIT flag. This tells the container-init that the
| >>+	 * signal originated in its own namespace and so it can choose
| >>+	 * to ignore the signal.
| >>+	 *
| >>+	 * If the receiver is the container-init of a pid namespace,
| >>+	 * but the sender is from an ancestor pid namespace, the
| >>+	 * container-init cannot ignore the signal. So clear the
| >>+	 * SIGQUEUE_CINIT flag in this case.
| >>+	 *
| >>+	 * Also, if the sender does not have a pid_t in the receiver's
| >>+	 * active pid namespace, set si_pid to 0 and pretend it originated
| >>+	 * from the kernel.
| >>+	 */
| >>+	if (pid_ns_equal(t)) {
| >>+		if (is_container_init(t)) {
| >>+			q->flags |= SIGQUEUE_CINIT;
| >
| >Ironically, this change carefully preserves the bug we already have :)
| >
| >This doesn't protect init from "bad" signal if we send it to sub-thread
| >of init. Actually, this make the behaviour a bit worse compared to what
| >we currently have. Currently, at least the main init's thread survives
| >if we send SIGKILL to sub-thread.

Do you mean "init's main thread" ? But doesn't SIGKILL to any thread kill the entire process ?

| >
| >>static int send_signal(int sig, struct siginfo *info, struct task_struct 
| >>*t,
| >>			struct sigpending *signals)
| >>{
| >>@@ -710,6 +781,7 @@ static int send_signal(int sig, struct s
| >>			copy_siginfo(&q->info, info);
| >>			break;
| >>		}
| >>+		encode_sender_info(t, q);
| >
| >We still send the signal if __sigqueue_alloc() fails. In that case, the
| >dequeued siginfo won't have SIGQUEUE_CINIT/KERN_SIGINFO_CINIT, not good.

Yes.

| >
| >>@@ -1158,6 +1232,13 @@ static int kill_something_info(int sig, 
| >>
| >>		read_lock(&tasklist_lock);
| >>		for_each_process(p) {
| >>+			/*
| >>+			 * System-wide signals apply only to the sender's
| >>+			 * pid namespace, unless issued from init_pid_ns.
| >>+			 */
| >>+			if (!task_visible_in_pid_ns(p, my_ns))
| >>+				continue;
| >>+
| >>			if (p->pid > 1 && p->tgid != current->tgid) {
| >
| >This "p->pid > 1" check should die.
| >

Ok.

| >>+static int deny_signal_to_container_init(struct task_struct *tsk, int 
| >>sig)
| >>+{
| >>+	/*
| >>+	 * If receiver is the container-init of sender and signal is SIGKILL
| >>+	 * reject it right-away. If signal is any other one, let the 
| >>container
| >>+	 * init decide (in get_signal_to_deliver()) whether to handle it or
| >>+	 * ignore it.
| >>+	 */
| >>+	if (is_container_init(tsk) && (sig == SIGKILL) && pid_ns_equal(tsk))
| >>+		return -EPERM;
| >>+
| >>+	return 0;
| >>+}
| >>+
| >>/*
| >> * Bad permissions for sending the signal
| >> */
| >>@@ -545,6 +584,10 @@ static int check_kill_permission(int sig
| >>	    && !capable(CAP_KILL))
| >>		return error;
| >>
| >>+	error = deny_signal_to_container_init(t, sig);
| >>+	if (error)
| >>+		return error;
| >
| >Hm. Could you explain this change? Why do we need a special check for 
| >SIGKILL?

As you pointed out above, SIGKILL goes through the __group_complete_signal()/ sigaddset() path and bypasses/loses the KERN_SIGINFO_CINIT flag. Other sig_fatal() signals take this path too, but we assume for now, container-init has a handler.

| >
| >
| >(What about ptrace_attach() btw? If it is possible to send a signal to init
| > from the "parent" namespace, perhaps it makes sense to allow ptracing as
| > well).
| 
| ptracing of tasks fro different namespaces is not possible at all, since
| strace utility determines the fork()-ed child pid from the parent's eax
| register, which would contain the pid value as this parent sees his child.
| But if the strace is in different namespace - it won't be able to find
| this child with the pid value from parent's eax.
| 
| Maybe it's worth disabling cross-namespaces ptracing...

I think so too. Its probably not a serious limitation ?

| 
| >
| >Oleg.
| >
| >

Several people think we will implement 'namespace entering' through a ptrace hack, where maybe the admin ptraces the init in a child pidns, makes it fork, and makes the child execute what it wants (i.e. ps -ef).

You're talking about killing that functionality?

-serge

Serge E. Hallyn [serue-r/[email protected]] wrote:

| Quoting sukadev-r/[email protected] (sukadev-r/[email protected]):
| > Pavel Emelianov [[email protected]] wrote:
| > | Oleg Nesterov wrote:
| > | >(What about ptrace_attach() btw? If it is possible to send a signal to init
| > | > from the "parent" namespace, perhaps it makes sense to allow ptracing as
| > | > well).
| > | 
| > | ptracing of tasks fro different namespaces is not possible at all, since
| > | strace utility determines the fork()-ed child pid from the parent's eax
| > | register, which would contain the pid value as this parent sees his child.
| > | But if the strace is in different namespace - it won't be able to find
| > | this child with the pid value from parent's eax.
| > | 
| > | Maybe it's worth disabling cross-namespaces ptracing...
| > 
| > I think so too. Its probably not a serious limitation ?
| 
| Several people think we will implement 'namespace entering' through a
| ptrace hack, where maybe the admin ptraces the init in a child pidns,
| makes it fork, and makes the child execute what it wants (i.e. ps -ef).
| 
| You're talking about killing that functionality?

No. I was only thinking in terms of debugging container init and missed the namespace entering part.

Pavel, I am not sure I understand your comment about being unable to ptrace() a child ns.

BTW, I am able to gdb a process (incl container-init) from parent ns now.

|
| -serge

Ok. We have a task with pid 100, which tries to clone the new namespace. This task fork-s and we have a new task with a couple of pids (101, 1). Then this "init" forks again and we have the third task with pids (102, 2). The problem is that when the 3rd task appears the return value from fork(), that is - the new task's pid as it is seen by it's parent (2nd task), will go to eax register (for i386) and it will be 2! But the prtaces from the initial namespace cannot address this task with pid 2.

Debugging separate processes is possible, but when this task starts forking with namespaces creation - this becomes impossible.

[snip]

Why not implement namespace entering w/o any hacks? :)

No. We're talking about disabling the things that are not supposed to work at all.

I did, as a patch on top of the nsproxy container subsystem. The response was that that is a hack, and ptrace is cleaner :)

So the current options for namespace entering would be:

* using Cedric's bind_ns() functionality, which assigns an integer global id to a namespace, and allows a process to enter a namespace by that global id
* using my nsproxy container subsystem patch, which lets a process enter another namespace using
echo pid > /container/some/cont/directory/tasks
and eventually might allow construction of custom namespaces, i.e.
mkdir /container/c1/c2
ln -s /container/c1/c1/network /container/c1/c2/network echo $$ > /container/c1/c2/tasks
* using ptrace to coerce a process in the target namespace into forking and executing the desired program.

Uh, well in the abstract that sounds like a sound policy...

-serge

looks more or less good and what OVZ actually does. So I would prefer this one.

Sound ok and logical as well.

you'll need to change ptrace interface in this case imho... doesn't sound ok at all... at least for me. So I agree with Pavel.

Pavel simply meant that no one plans to disable functionality in question.

Thanks,
Kirill

I think this was Cedric's last post of it:

https://lists.linux-foundation.org/pipermail/containers/2007-June/005665.html

However I'm pretty sure Eric would be soundly against this.

I last posted this here:
http://www.mail-archive.com/[email protected]/msg00295.html
In the ensuing thread, the ptrace-based solution is also discussed.

Well maybe the problem is that while I think of it as ptrace-based, it's really only like ptrace in that it hijacks another process for a moment to clone it, but it likely will end up a completely different system call.

Eric, just curious, have you worked on this at all since february?

-serge

I can't understand your question. Yes, this is what we are doing currently, but this is broken by this patch.

Changelog says nothing about that. And in that case we don't need any complications except a) deny_signal_to_container_init() (should be named deny_SIGKILL_to_container_init) and b) "cross-namespace signals must have si_code == SI_KERNEL".

I don't know whether this limitation (/sbin/init must install the handler for each fatal signal) acceptable or not.

However, we should also take care about sig_kernel_stop() signals, and please note that it is not possible to install a handler for SIGSTOP.

Yes.

It should, but it doesn't if it was sent to init's sub-thread, exactly because of child_reaper() check in get_signal_to_deliver().

No, SIGKILL doesn't bypasses send_signal(). IOW, if other parts of this patch were correct, we don't need this change. If init has a handler, we don't neeed other parts.

My question was not clear, sorry. And I was confused because I had a false impression that ptrace_attach() was already changed to use is_container_init().

Afaics, the cross-namespaces ptracing should work (modulo fork() problem pointed out by Pavel), and probably it is useful.

But we should fix ptrace_attach(), it should not be possible to do PTRACE_ATTACH to /sbin/init from the _same_ namespace.

Oleg.

This includes
* the headers dependency fix
* task_child_reaper() correct declaration
* fixes for is_global_init() and is_container_init()

Maybe this should come before all the other stuff...

---

include/linux/pid_namespace.h |    4 ++--
include/linux/sched.h         |    4 ++--
kernel/pid.c                  |   16 +++++++++++++---
3 files changed, 17 insertions(+), 7 deletions(-)

diff -upr linux-2.6.23-rc1-mm1.orig/include/linux/pid_namespace.h linux-2.6.23-rc1-mm1-7/include/linux/pid_namespace.h
@@ -4,7 +4,6 @@

#include <linux/sched.h>
#include <linux/mm.h>
#include <linux/threads.h>
-#include <linux/pid.h>
#include <linux/nsproxy.h>
#include <linux/kref.h>

@@ -46,7 +53,8 @@ static inline struct pid_namespace *task

static inline struct task_struct *task_child_reaper(struct task_struct *tsk) {
- return init_pid_ns.child_reaper;
+ BUG_ON(tsk != current);
+ return tsk->nsproxy->pid_ns->child_reaper;
}

#endif /* _LINUX_PID_NS_H */

diff -upr linux-2.6.23-rc1-mm1.orig/include/linux/sched.h linux-2.6.23-rc1-mm1-7/include/linux/sched.h
@@ -1277,13 +1366,13 @@ static inline int pid_alive(struct task_
 *
 * TODO: We should inline this function after some cleanups in pid_namespace.h
 */
-extern int is_global_init(struct task_struct *tsk);
+extern int is_container_init(struct task_struct *tsk);
 

/*
* is_container_init:
* check whether in the task is init in its own pid namespace. */
-static inline int is_container_init(struct task_struct *tsk) +static inline int is_global_init(struct task_struct *tsk)

{
	return tsk->pid == 1;
}

diff -upr linux-2.6.23-rc1-mm1.orig/kernel/pid.c linux-2.6.23-rc1-mm1-7/kernel/pid.c
@@ -60,11 +62,21 @@ static inline int mk_pid(struct pid_name

};
EXPORT_SYMBOL(init_pid_ns);

-int is_global_init(struct task_struct *tsk)
+int is_container_init(struct task_struct *tsk)
{
- return tsk == init_pid_ns.child_reaper;
+ int ret;
+ struct pid *pid;
+
+ ret = 0;
+ rcu_read_lock();
+ pid = task_pid(tsk);
+ if (pid != NULL && pid->numbers[pid->level].nr == 1)
+ ret = 1;
+ rcu_read_unlock();
+
+ return ret;
}
-EXPORT_SYMBOL(is_global_init);
+EXPORT_SYMBOL(is_container_init);

/*
* Note: disable interrupts while the pidmap_lock is held as an

Pavel Emelianov [[email protected]] wrote:

| This includes
|  * the headers dependency fix
|  * task_child_reaper() correct declaration
|  * fixes for is_global_init() and is_container_init()
| 
| Maybe this should come before all the other stuff...
| 
| Signed-off-by: Pavel Emelyanov <[email protected]>
| 
| ---
| 
| include/linux/pid_namespace.h |    4 ++--
| include/linux/sched.h         |    4 ++--
| kernel/pid.c                  |   16 +++++++++++++---
| 3 files changed, 17 insertions(+), 7 deletions(-)
| 
| diff -upr linux-2.6.23-rc1-mm1.orig/include/linux/pid_namespace.h 
| linux-2.6.23-rc1-mm1-7/include/linux/pid_namespace.h
| --- linux-2.6.23-rc1-mm1.orig/include/linux/pid_namespace.h	2007-07-26 
| 16:34:45.000000000 +0400
| +++ linux-2.6.23-rc1-mm1-7/include/linux/pid_namespace.h	2007-07-26 
| 16:36:36.000000000 +0400
| @@ -4,7 +4,6 @@
| #include <linux/sched.h>
| #include <linux/mm.h>
| #include <linux/threads.h>
| -#include <linux/pid.h>
| #include <linux/nsproxy.h>
| #include <linux/kref.h>
| 
| @@ -46,7 +53,8 @@ static inline struct pid_namespace *task
| 
| static inline struct task_struct *task_child_reaper(struct task_struct *tsk)
| {
| -	return init_pid_ns.child_reaper;
| +	BUG_ON(tsk != current);
| +	return tsk->nsproxy->pid_ns->child_reaper;
| }
| 
| #endif /* _LINUX_PID_NS_H */
| diff -upr linux-2.6.23-rc1-mm1.orig/include/linux/sched.h 
| linux-2.6.23-rc1-mm1-7/include/linux/sched.h
| --- linux-2.6.23-rc1-mm1.orig/include/linux/sched.h	2007-07-26 
| 16:34:45.000000000 +0400
| +++ linux-2.6.23-rc1-mm1-7/include/linux/sched.h	2007-07-26 
| 16:36:37.000000000 +0400
| @@ -1277,13 +1366,13 @@ static inline int pid_alive(struct task_
|  *
|  * TODO: We should inline this function after some cleanups in 
|  pid_namespace.h
|  */

We can now remove this TODO.

| -extern int is_global_init(struct task_struct *tsk);
| +extern int is_container_init(struct task_struct *tsk);
| 
| /*
|  * is_container_init:
|  * check whether in the task is init in its own pid namespace.
|  */

The function header describes is_container_init() but the function is actually is_global_init(). The opp is true for the function heaer of is_container_init() above.

| -static inline int is_container_init(struct task_struct *tsk)
| +static inline int is_global_init(struct task_struct *tsk)
| {
| 	return tsk->pid == 1;
| }
| diff -upr linux-2.6.23-rc1-mm1.orig/kernel/pid.c 
| linux-2.6.23-rc1-mm1-7/kernel/pid.c
| --- linux-2.6.23-rc1-mm1.orig/kernel/pid.c	2007-07-26 
| 16:34:45.000000000 +0400
| +++ linux-2.6.23-rc1-mm1-7/kernel/pid.c	2007-07-26 
| 16:36:37.000000000 +0400
| @@ -60,11 +62,21 @@ static inline int mk_pid(struct pid_name
| };
| EXPORT_SYMBOL(init_pid_ns);
| 
| -int is_global_init(struct task_struct *tsk)
| +int is_container_init(struct task_struct *tsk)
| {
| -	return tsk == init_pid_ns.child_reaper;
| +	int ret;

Initialize ret = 0 here would save a line of code below :-)

| +	struct pid *pid;
| +
| +	ret = 0;
| +	rcu_read_lock();
| +	pid = task_pid(tsk);
| +	if (pid != NULL && pid->numbers[pid->level].nr == 1)
| +		ret = 1;
| +	rcu_read_unlock();
| +
| +	return ret;
| }
| -EXPORT_SYMBOL(is_global_init);
| +EXPORT_SYMBOL(is_container_init);
| 
| /*
|  * Note: disable interrupts while the pidmap_lock is held as an

Ok.

:)

Well, I don't actually like initializing variables right when they are declared. Even with trivial values :)

When clone() is invoked with CLONE_NEWPID, create a new pid namespace Since the active pid namespace is special and expected to be the first entry in pid->upid_list, preserve the order of pid namespaces.

Pid namespaces can be nested and the nesting depth is unlimited. When a process clones its pid namespace, we create additional pid caches as necessary and use the pid cache to allocate 'struct pids' for that depth.

TODO:
One of the reasons the free_work() was introduced was to cleanup the proc in non-atomic context, but since proc is now released from proc_flush_task() this looks to be unneeded, but I have to recheck this.

---

include/linux/pid_namespace.h |    7 ++
include/linux/sched.h         |    1 kernel/nsproxy.c              |    3 -
kernel/pid.c                  |  102 +++++++++++++++++++++++++++++++++++++-----
4 files changed, 101 insertions(+), 12 deletions(-)

diff -upr linux-2.6.23-rc1-mm1.orig/include/linux/pid_namespace.h linux-2.6.23-rc1-mm1-7/include/linux/pid_namespace.h
@@ -16,11 +15,16 @@ struct pidmap {

#define PIDMAP_ENTRIES ((PID_MAX_LIMIT + 8*PAGE_SIZE - 1)/PAGE_SIZE/8)

struct pid_namespace {
- struct kref kref;
+ union {
+ struct kref kref;
+ struct work_struct free_work;

+	};
	struct pidmap pidmap[PIDMAP_ENTRIES];
	int last_pid;
	struct task_struct *child_reaper;
	struct kmem_cache *pid_cachep;
+	int level;
+	struct pid_namespace *parent;
#ifdef CONFIG_PROC_FS
	struct vfsmount *proc_mnt;

#endif

diff -upr linux-2.6.23-rc1-mm1.orig/include/linux/sched.h linux-2.6.23-rc1-mm1-7/include/linux/sched.h
@@ -27,6 +27,7 @@

#define CLONE_NEWUTS 0x04000000 /* New utsname group? */
#define CLONE_NEWIPC 0x08000000 /* New ipcs */
#define CLONE_NEWUSER 0x10000000 /* New user namespace */
+#define CLONE_NEWPID 0x20000000 /* New pids */

/*
* Scheduling policies

diff -upr linux-2.6.23-rc1-mm1.orig/kernel/nsproxy.c linux-2.6.23-rc1-mm1-7/kernel/nsproxy.c
@@ -132,7 +132,8 @@ int copy_namespaces(unsigned long flags,
 

	get_nsproxy(old_ns);

-	if (!(flags & (CLONE_NEWNS | CLONE_NEWUTS | CLONE_NEWIPC | CLONE_NEWUSER)))
+	if (!(flags & (CLONE_NEWNS | CLONE_NEWUTS | CLONE_NEWIPC |
+					CLONE_NEWUSER | CLONE_NEWPID)))
		return 0;

	if (!capable(CAP_SYS_ADMIN)) {

diff -upr linux-2.6.23-rc1-mm1.orig/kernel/pid.c linux-2.6.23-rc1-mm1-7/kernel/pid.c
@@ -60,14 +62,17 @@ static inline int mk_pid(struct pid_name
 * the scheme scales to up to 4 million PIDs, runtime.
 */

struct pid_namespace init_pid_ns = {
- .kref = {
- .refcount = ATOMIC_INIT(2),
+ {
+ .kref = {
+ .refcount = ATOMIC_INIT(2),
+ },

	},
	.pidmap = {
		[ 0 ... PIDMAP_ENTRIES-1] = { ATOMIC_INIT(BITS_PER_PAGE), NULL }
	},
	.last_pid = 0,

- .child_reaper = &init_task
+ .level = 0,
+ .child_reaper = &init_task,
};
EXPORT_SYMBOL(init_pid_ns);

@@ -409,8 +501,8 @@ static struct kmem_cache *create_pid_cac

	snprintf(pcache->name, sizeof(pcache->name), "pid_%d", nr_ids);
	cachep = kmem_cache_create(pcache->name,
-			/* FIXME add numerical ids here */
-			sizeof(struct pid), 0, SLAB_HWCACHE_ALIGN, NULL);
+			sizeof(struct pid) + (nr_ids - 1) * sizeof(struct upid),
+			0, SLAB_HWCACHE_ALIGN, NULL);
	if (cachep == NULL)
		goto err_cachep;

@@ -428,11 +520,89 @@ err_alloc:

	return NULL;
}

-struct pid_namespace *copy_pid_ns(unsigned long flags, struct pid_namespace *old_ns) +static struct pid_namespace *create_pid_namespace(int level)
+{
+ struct pid_namespace *ns;
+ int i;
+
+ ns = kmalloc(sizeof(struct pid_namespace), GFP_KERNEL);
+ if (ns == NULL)
+ goto out;
+
+ ns->pidmap[0].page = kzalloc(PAGE_SIZE, GFP_KERNEL);
+ if (!ns->pidmap[0].page)
+ goto out_free;
+
+ ns->pid_cachep = create_pid_cachep(level + 1);
+ if (ns->pid_cachep == NULL)
+ goto out_free_map;
+
+ kref_init(&ns->kref);
+ ns->last_pid = 0;
+ ns->child_reaper = NULL;
+ ns->level = level;
+
+ set_bit(0, ns->pidmap[0].page);
+ atomic_set(&ns->pidmap[0].nr_free, BITS_PER_PAGE - 1);
+ get_pid_ns(ns);
+
+ for (i = 1; i < PIDMAP_ENTRIES; i++) {
+ ns->pidmap[i].page = 0;
+ atomic_set(&ns->pidmap[i].nr_free, BITS_PER_PAGE);
+ }
+
+ return ns;
+
+out_free_map:
+ kfree(ns->pidmap[0].page);
+out_free:
+ kfree(ns);
+out:
+ return ERR_PTR(-ENOMEM);
+}
+
+static void destroy_pid_namespace(struct pid_namespace *ns)
{
- BUG_ON(!old_ns);
- get_pid_ns(old_ns);
- return old_ns;
+ int i;
+
+ for (i = 0; i < PIDMAP_ENTRIES; i++)
+ kfree(ns->pidmap[i].page);
+ kfree(ns);
+}
+
+ struct pid_namespace *copy_pid_ns(unsigned long flags, struct pid_namespace *old_ns)
+ {
+ struct pid_namespace *new_ns;
+
+ BUG_ON(!old_ns);
+ new_ns = get_pid_ns(old_ns);
+ if (!(flags & CLONE_NEWPID))
+ goto out;
+
+ new_ns = ERR_PTR(-EINVAL);
+ if (flags & CLONE_THREAD)
+ goto out_put;
+
+ new_ns = create_pid_namespace(old_ns->level + 1);
+ if (new_ns != NULL)
+ new_ns->parent = get_pid_ns(old_ns);
+
+out_put:
+ put_pid_ns(old_ns);
+out:
+ return new_ns;
+}
+
+static void do_free_pid_ns(struct work_struct *w)
+{
+ struct pid_namespace *ns, *parent;
+
+ ns = container_of(w, struct pid_namespace, free_work);
+ parent = ns->parent;
+ destroy_pid_namespace(ns);
+
+ if (parent != NULL)
+ put_pid_ns(parent);
}

void free_pid_ns(struct kref *kref)
@@ -440,7 +648,8 @@ void free_pid_ns(struct kref *kref)

	struct pid_namespace *ns;

	ns = container_of(kref, struct pid_namespace, kref);

- kfree(ns);
+ INIT_WORK(&ns->free_work, do_free_pid_ns);
+ schedule_work(&ns->free_work);
}

/*

From: Sukadev Bhattiprolu <sukadev-r/[email protected]>

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.

---

include/linux/pid.h  |    1 +
include/linux/wait.h |    4 ++++
kernel/exit.c        |   10 ++++++----
kernel/pid.c         |   38 ++++++++++++++++++++++++++++++++++++++
4 files changed, 49 insertions(+), 4 deletions(-)

diff -upr linux-2.6.23-rc1-mm1.orig/include/linux/pid.h linux-2.6.23-rc1-mm1-7/include/linux/pid.h
@@ -83,6 +92,7 @@ extern void FASTCALL(detach_pid(struct t
 

extern struct pid *alloc_pid(struct pid_namespace *ns); extern void FASTCALL(free_pid(struct pid *pid)); +extern void zap_pid_ns_processes(struct pid_namespace *pid_ns);

/*
* the helpers to get the pid's id seen from different namespaces