OpenVZ Forum - RDF feed
type: channel
items: [PATCH 1/2] pidns: Don't allow new pids after the namespace is dead.
items: [PATCH 2/2] pidns: Support unsharing the pid namespace.
items: Re: [PATCH 1/2] pidns: Don't allow new pids after the namespace is dead.
items: Re: [PATCH 2/2] pidns: Support unsharing the pid namespace.
items: [PATCH 0/1] Was: pidns: Support unsharing the pid namespace.
items: [PATCH 1/1][3rd resend] sys_unshare: remove the dead CLONE_THREAD/SIGHAND/VM code
items: Re: [PATCH 1/1][3rd resend] sys_unshare: remove the dead CLONE_THREAD/SIGHAND/VM code

[PATCH 1/2] pidns: Don't allow new pids after the namespace is dead.
type: item
description: From: Eric W. Biederman <ebiederm@xmission.com> In the case of unsharing or joining a pid namespace, it becomes possible to attempt to allocate a pid after zap_pid_namespace has killed everything in the namespace. Close the hole for now by simply not allowing any of those pid allocations to succeed. At least for now it is too strange to think about. Signed-off-by: Eric W. Biederman <ebiederm@xmission.com> Signed-off-by: Daniel Lezcano <daniel.lezcano@free.fr> --- include/linux/pid_namespace.h | 1 + kernel/pid.c | 4 ++++ kernel/pid_namespace.c | 2 ++ 3 files changed, 7 insertions(+), 0 deletions(-) diff --git a/include/linux/pid_namespace.h b/include/linux/pid_namespace.h index 38d1032..b447d37 100644 --- a/include/linux/pid_namespace.h +++ b/include/linux/pid_namespace.h @@ -20,6 +20,7 @@ struct pid_namespace { struct kref kref; struct pidmap pidmap[PIDMAP_ENTRIES]; int last_pid; + atomic_t dead; struct task_struct *child_reaper; struct kmem_cache *pid_cachep; unsigned int level; diff --git a/kernel/pid.c b/kernel/pid.c index 39b65b6..e996950 100644 --- a/kernel/pid.c +++ b/kernel/pid.c @@ -282,6 +282,10 @@ struct pid *alloc_pid(struct pid_namespace *ns) struct pid_namespace *tmp; struct upid *upid; + pid = NULL; + if (atomic_read(&ns->dead)) + goto out; + pid = kmem_cache_alloc(ns->pid_cachep, GFP_KERNEL); if (!pid) goto out; diff --git a/kernel/pid_namespace.c b/kernel/pid_namespace.c index e9c9adc..e8ea25d 100644 --- a/kernel/pid_namespace.c +++ b/kernel/pid_namespace.c @@ -90,6 +90,7 @@ static struct pid_namespace *create_pid_namespace(struct pid_namespace *parent_p kref_init(&ns->kref); ns->level = level; ns->parent = get_pid_ns(parent_pid_ns); + atomic_set(&ns->dead, 0); set_bit(0, ns->pidmap[0].page); atomic_set(&ns->pidmap[0].nr_free, BITS_PER_PAGE - 1); @@ -164,6 +165,7 @@ void zap_pid_ns_processes(struct pid_namespace *pid_ns) * */ read_lock(&tasklist_lock); + atomic_set(&pid_ns->dead, 1); nr = next_pidmap(pid_ns, 1); while (nr > 0) { rcu_read_lock(); -- 1…
creator: Daniel Lezcano
date: 2011-02-15T16:53:43-00:00

[PATCH 2/2] pidns: Support unsharing the pid namespace.
type: item
description: From: Eric W. Biederman <ebiederm@xmission.com> - Allow CLONEW_NEWPID into unshare. - Pass both nsproxy->pid_ns and task_active_pid_ns to copy_pid_ns As they can now be different. Unsharing of the pid namespace unlike unsharing of other namespaces does not take effect immediately. Instead it affects the children created with fork and clone. The first of these children becomes the init process of the new pid namespace, the rest become oddball children of pid 0. From the point of view of the new pid namespace the process that created it is pid 0, as it's pid does not map. A couple of different semantics were considered but this one was settled on because it is easy to implement and it is usable from pam modules. The core reasons for the existence of unshare. I took a survey of the callers of pam modules and the following appears to be a representative sample of their logic. { setup stuff include pam child = fork(); if (!child) { setuid() exec /bin/bash } waitpid(child); pam and other cleanup } As you can see there is a fork to create the unprivileged user space process. Which means that the unprivileged user space process will appear as pid 1 in the new pid namespace. Further most login processes do not cope with extraneous children which means shifting the duty of reaping extraneous child process to the creator of those extraneous children makes the system more comprehensible. The practical reason for this set of pid namespace semantics is that it is simple to implement and verify they work correctly. Whereas an implementation that requires changing the struct pid on a process comes with a lot more races and pain. Not the least of which is that glibc caches getpid(). These semantics are implemented by having two notions of the pid namespace of a process. There is task_active_pid_ns which is the pid namspace the process was created with and the pid namespace that all pids are presented to that process in. The task_active_pid_ns is stored in the struct pid of the task.…
creator: Daniel Lezcano
date: 2011-02-15T16:53:44-00:00

Re: [PATCH 1/2] pidns: Don't allow new pids after the namespace is dead.
type: item
description: On 02/15, Daniel Lezcano wrote: > > In the case of unsharing or joining a pid namespace, it becomes > possible to attempt to allocate a pid after zap_pid_namespace has > killed everything in the namespace. Close the hole for now by simply > not allowing any of those pid allocations to succeed. Daniel, please explain more. It seems, a long ago I knew the reason for this patch, but now I can't recall and can't understand this change. > --- a/include/linux/pid_namespace.h > +++ b/include/linux/pid_namespace.h > @@ -20,6 +20,7 @@ struct pid_namespace { > struct kref kref; > struct pidmap pidmap[PIDMAP_ENTRIES]; > int last_pid; > + atomic_t dead; Why atomic_t? It is used as a plain boolean. And I can't unde > --- a/kernel/pid.c > +++ b/kernel/pid.c > @@ -282,6 +282,10 @@ struct pid *alloc_pid(struct pid_namespace *ns) > struct pid_namespace *tmp; > struct upid *upid; > > + pid = NULL; > + if (atomic_read(&ns->dead)) > + goto out; > + So why this is needed? If we see ns->dead != 0 we are already killed by zap_pid_ns_processes() which sets ns->dead = 1. Oleg. _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: Oleg Nesterov
date: 2011-02-15T18:30:28-00:00

Re: [PATCH 2/2] pidns: Support unsharing the pid namespace.
type: item
description: On 02/15, Daniel Lezcano wrote: > > - Pass both nsproxy->pid_ns and task_active_pid_ns to copy_pid_ns > As they can now be different. But since they can be different we have to convert some users of current->nsproxy first? But that patch was dropped. > Unsharing of the pid namespace unlike unsharing of other namespaces > does not take effect immediately. Instead it affects the children > created with fork and clone. IOW, unshare(CLONE_NEWPID) implicitly affects the subsequent fork(), using the very subtle way. I have to admit, I can't say I like this very much. OK, if we need this, can't we just put something into, say, signal->flags so that copy_process can check and create the new namespace. Also. I remember, I already saw something like this and google found my questions. I didn't actually read the new version, perhaps my concerns were already answered... But what if the task T does unshare(CLONE_NEWPID) and then, say, pthread_create() ? Unless I missed something, the new thread won't be able to see T ? and, in this case the exiting sub-namespace init also kills its parent? OK, suppose it does fork() after unshare(), then another fork(). In this case the second child lives in the same namespace with init created by the 1st fork, but it is not descendant ? This means in particular that if the new init exits, zap_pid_ns_processes()-> do_wait() can't work. Or not? Oleg. _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: Oleg Nesterov
date: 2011-02-15T19:01:18-00:00

[PATCH 0/1] Was: pidns: Support unsharing the pid namespace.
type: item
description: On 02/15, Oleg Nesterov wrote: > > On 02/15, Daniel Lezcano wrote: > > > > - Pass both nsproxy->pid_ns and task_active_pid_ns to copy_pid_ns > > As they can now be different. > > But since they can be different we have to convert some users of > current->nsproxy first? But that patch was dropped. > > > Unsharing of the pid namespace unlike unsharing of other namespaces > > does not take effect immediately. Instead it affects the children > > created with fork and clone. > > IOW, unshare(CLONE_NEWPID) implicitly affects the subsequent fork(), > using the very subtle way. > > I have to admit, I can't say I like this very much. OK, if we need > this, can't we just put something into, say, signal->flags so that > copy_process can check and create the new namespace. > > Also. I remember, I already saw something like this and google found > my questions. I didn't actually read the new version, perhaps my > concerns were already answered... > > But what if the task T does unshare(CLONE_NEWPID) and then, say, > pthread_create() ? Unless I missed something, the new thread won't > be able to see T ? > > and, in this case the exiting sub-namespace init also kills its > parent? > > OK, suppose it does fork() after unshare(), then another fork(). > In this case the second child lives in the same namespace with > init created by the 1st fork, but it is not descendant ? This means > in particular that if the new init exits, zap_pid_ns_processes()-> > do_wait() can't work. > > Or not? And, can't resist. If we are going to change sys_unshare(), I'd like very much to cleanup it first. Dear all! I promise, I will resend this patch forever until somebody explains me why it is constantly ignored ;) Oleg. _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: Oleg Nesterov
date: 2011-02-15T19:15:21-00:00

[PATCH 1/1][3rd resend] sys_unshare: remove the dead CLONE_THREAD/SIGHAND/VM code
type: item
description: Cleanup: kill the dead code which does nothing but complicates the code and confuses the reader. sys_unshare(CLONE_THREAD/SIGHAND/VM) is not really implemented, and I doubt very much it will ever work. At least, nobody even tried since the original "unshare system call -v5: system call handler function" commit 99d1419d96d7df9cfa56bc977810be831bd5ef64 was applied more than 4 years ago. And the code is not consistent. unshare_thread() always fails unconditionally, while unshare_sighand() and unshare_vm() pretend to work if there is nothing to unshare. Remove unshare_thread(), unshare_sighand(), unshare_vm() helpers and related variables and add a simple CLONE_THREAD | CLONE_SIGHAND| CLONE_VM check into check_unshare_flags(). Also, move the "CLONE_NEWNS needs CLONE_FS" check from check_unshare_flags() to sys_unshare(). This looks more consistent and matches the similar do_sysvsem check in sys_unshare(). Note: with or without this patch "atomic_read(mm->mm_users) > 1" can give a false positive due to get_task_mm(). Signed-off-by: Oleg Nesterov <oleg@redhat.com> Acked-by: Roland McGrath <roland@redhat.com> --- kernel/fork.c | 123 +++++++++++----------------------------------------------- 1 file changed, 25 insertions(+), 98 deletions(-) --- 2.6.37/kernel/fork.c~unshare-killcrap 2010-11-05 18:03:28.000000000 +0100 +++ 2.6.37/kernel/fork.c 2010-11-05 18:09:52.000000000 +0100 @@ -1522,38 +1522,24 @@ void __init proc_caches_init(void) } /* - * Check constraints on flags passed to the unshare system call and - * force unsharing of additional process context as appropriate. + * Check constraints on flags passed to the unshare system call. */ -static void check_unshare_flags(unsigned long *flags_ptr) +static int check_unshare_flags(unsigned long unshare_flags) { + if (unshare_flags & ~(CLONE_THREAD|CLONE_FS|CLONE_NEWNS|CLONE_SIGHAND| + CLONE_VM|CLONE_FILES|CLONE_SYSVSEM| + CLONE_NEWUTS|CLONE_NEWIPC|CLONE_NEWNET)) + return -EINVAL; /* - * If unsharing a thread from a thread grou…
creator: Oleg Nesterov
date: 2011-02-15T19:17:11-00:00

Re: [PATCH 1/2] pidns: Don't allow new pids after the namespace is dead.
type: item
description: On 02/15/2011 07:30 PM, Oleg Nesterov wrote: > On 02/15, Daniel Lezcano wrote: >> In the case of unsharing or joining a pid namespace, it becomes >> possible to attempt to allocate a pid after zap_pid_namespace has >> killed everything in the namespace. Close the hole for now by simply >> not allowing any of those pid allocations to succeed. > Daniel, please explain more. It seems, a long ago I knew the reason > for this patch, but now I can't recall and can't understand this change. The idea behind unsharing the pid namespace is the current pid is not mapped in the newly created pid namespace and appears as the pid 0. When it forks, the child process becomes the init pid of the new pid namespace. When this pid namespace dies because the init pid exited, the parent process (aka pid 0) can no longer fork because the pid namespace is flagged dead. This is what does this patch. The next patch allows a single process to spawn different processes in different pid namespace. You can argue we can already do that with clone(CLONE_NEWPID). That's true. But if we are able to unshare the pid namespace, then the next patchset (which will come right after this one) will allow to attach a process to a namespace and the implementation will be very simple and consistent with attaching to any namespace. >> --- a/include/linux/pid_namespace.h >> +++ b/include/linux/pid_namespace.h >> @@ -20,6 +20,7 @@ struct pid_namespace { >> struct kref kref; >> struct pidmap pidmap[PIDMAP_ENTRIES]; >> int last_pid; >> + atomic_t dead; > Why atomic_t? It is used as a plain boolean. > > And I can't unde I think Eric used an atomic because it is lockless with alloc_pid vs zap_pid_ns_processes. >> --- a/kernel/pid.c >> +++ b/kernel/pid.c >> @@ -282,6 +282,10 @@ struct pid *alloc_pid(struct pid_namespace *ns) >> struct pid_namespace *tmp; >> struct upid *upid; >> >> + pid = NULL; >> + if (atomic_read(&ns->dead)) >> + goto out; >> + > So why this is needed? > > If we see ns->dead != 0 we are already kil…
creator: Daniel Lezcano
date: 2011-02-16T23:21:12-00:00

Re: [PATCH 2/2] pidns: Support unsharing the pid namespace.
type: item
description: On 02/15/2011 08:01 PM, Oleg Nesterov wrote: > On 02/15, Daniel Lezcano wrote: >> - Pass both nsproxy->pid_ns and task_active_pid_ns to copy_pid_ns >> As they can now be different. > But since they can be different we have to convert some users of > current->nsproxy first? But that patch was dropped. > >> Unsharing of the pid namespace unlike unsharing of other namespaces >> does not take effect immediately. Instead it affects the children >> created with fork and clone. > IOW, unshare(CLONE_NEWPID) implicitly affects the subsequent fork(), > using the very subtle way. > > I have to admit, I can't say I like this very much. OK, if we need > this, can't we just put something into, say, signal->flags so that > copy_process can check and create the new namespace. > > Also. I remember, I already saw something like this and google found > my questions. I didn't actually read the new version, perhaps my > concerns were already answered... > > But what if the task T does unshare(CLONE_NEWPID) and then, say, > pthread_create() ? Unless I missed something, the new thread won't > be able to see T ? Right. Is it really a problem ? I mean it is a weird use case where we fall in a weird situation. I suppose we can do the same weird combination with clone. IMHO, the userspace is responsible of how it uses the syscalls. Until the system is safe, everything is ok, no ? > and, in this case the exiting sub-namespace init also kills its > parent? I don't think so because the zap_pid_ns_processes does not hit the parent process when it browses the pidmap. I tried the following program without problem: #include <stdio.h> #define _GNU_SOURCE #include <sched.h> #include <pthread.h> void *routine(void *data) { printf("pid %d!\n", getpid()); return NULL; } int main(int argc, char *argv[]) { char **aux = &argv[1]; pthread_t t; if (unshare(CLONE_NEWPID)) { perror("unshare"); return -1; } if (pthread_create(&t, NULL, routine, NULL)) { perror("pthread_create"); return -1; } if (pthread_join(t, …
creator: Daniel Lezcano
date: 2011-02-16T23:47:37-00:00

Re: [PATCH 2/2] pidns: Support unsharing the pid namespace.
type: item
description: On 02/17, Daniel Lezcano wrote: > > On 02/15/2011 08:01 PM, Oleg Nesterov wrote: >> >> I have to admit, I can't say I like this very much. OK, if we need >> this, can't we just put something into, say, signal->flags so that >> copy_process can check and create the new namespace. >> >> Also. I remember, I already saw something like this and google found >> my questions. I didn't actually read the new version, perhaps my >> concerns were already answered... >> >> But what if the task T does unshare(CLONE_NEWPID) and then, say, >> pthread_create() ? Unless I missed something, the new thread won't >> be able to see T ? > > Right. Is it really a problem ? I mean it is a weird use case where we > fall in a weird situation. But this is really weird! How it is possible that the parent can't see its own child? No matter which thread did fork(), the new process is the child of any sub-thread. More precisely, it is the child of thread group. > I suppose we can do the same weird combination with clone. No, or we have the bug. If nothing else, kill() or wait() should work equally for any sub-thread. (OK, __WNOTHREAD hack is the only exception). >> and, in this case the exiting sub-namespace init also kills its >> parent? > > I don't think so because the zap_pid_ns_processes does not hit the > parent process when it browses the pidmap. OK... Honestly, right now I can't understand my own question, it was written a long ago. Probably I missed something.... but I'll recheck ;) >> OK, suppose it does fork() after unshare(), then another fork(). >> In this case the second child lives in the same namespace with >> init created by the 1st fork, but it is not descendant ? This means >> in particular that if the new init exits, zap_pid_ns_processes()-> >> do_wait() can't work. > > Hmm, good question. IMO, we should prevent such case for now in the same > way we added the flag 'dead', IOW adding a flag 'busy' for example. I dunno. As I said, I do not like this approach at all. But please f…
creator: Oleg Nesterov
date: 2011-02-17T20:29:59-00:00

Re: [PATCH 1/2] pidns: Don't allow new pids after the namespace is dead.
type: item
description: On 02/17, Daniel Lezcano wrote: > > On 02/15/2011 07:30 PM, Oleg Nesterov wrote: >> On 02/15, Daniel Lezcano wrote: >>> In the case of unsharing or joining a pid namespace, it becomes >>> possible to attempt to allocate a pid after zap_pid_namespace has >>> killed everything in the namespace. Close the hole for now by simply >>> not allowing any of those pid allocations to succeed. >> Daniel, please explain more. It seems, a long ago I knew the reason >> for this patch, but now I can't recall and can't understand this change. > > The idea behind unsharing the pid namespace is the current pid is not > mapped in the newly created pid namespace and appears as the pid 0. Well, not exactly afaics... but doesn't matter. > When > it forks, the child process becomes the init pid of the new pid > namespace. Yes, I see. And this is what I personally dislike. Because, iow, unshare(PID) changes current->nspory->pid_ns to affect the behaviour of copy_process() , this really looks like "action at a distance" to me. Too subtle and fragile. But, once again, this is just imho, feel free to ignore. > When this pid namespace dies because the init pid exited, the > parent process (aka pid 0) can no longer fork because the pid namespace > is flagged dead. This is what does this patch. OK, thanks. I seem to understand. May be ;) I'd suggest you to add this explanation to the changelog. >>> --- a/include/linux/pid_namespace.h >>> +++ b/include/linux/pid_namespace.h >>> @@ -20,6 +20,7 @@ struct pid_namespace { >>> struct kref kref; >>> struct pidmap pidmap[PIDMAP_ENTRIES]; >>> int last_pid; >>> + atomic_t dead; >> Why atomic_t? It is used as a plain boolean. >> >> And I can't unde > > I think Eric used an atomic because it is lockless with alloc_pid vs > zap_pid_ns_processes. Can't understand... But anyway, I strongly believe atomic_t buys nothing in this patch. May be it is needed for the next changes, I dunno. Oleg. _______________________________________________ Containers mailing list …
creator: Oleg Nesterov
date: 2011-02-17T20:54:58-00:00

Re: [PATCH 2/2] pidns: Support unsharing the pid namespace.
type: item
description: On 02/17/2011 09:29 PM, Oleg Nesterov wrote: > On 02/17, Daniel Lezcano wrote: >> >> On 02/15/2011 08:01 PM, Oleg Nesterov wrote: >>> >>> I have to admit, I can't say I like this very much. OK, if we need >>> this, can't we just put something into, say, signal->flags so that >>> copy_process can check and create the new namespace. >>> >>> Also. I remember, I already saw something like this and google found >>> my questions. I didn't actually read the new version, perhaps my >>> concerns were already answered... >>> >>> But what if the task T does unshare(CLONE_NEWPID) and then, say, >>> pthread_create() ? Unless I missed something, the new thread won't >>> be able to see T ? >> >> Right. Is it really a problem ? I mean it is a weird use case where we >> fall in a weird situation. > > But this is really weird! How it is possible that the parent can't see > its own child? No matter which thread did fork(), the new process is Hmmm... I guess you mean the opposite. The way pid namespaces are nested, parents always see their children. But indeed, the child thread can't see its group leader and that's kind of unusual. Unshare a pid namespace at your own risk. :) > the child of any sub-thread. More precisely, it is the child of thread > group. > >> I suppose we can do the same weird combination with clone. > > No, or we have the bug. If nothing else, kill() or wait() should work > equally for any sub-thread. (OK, __WNOTHREAD hack is the only exception). > >>> and, in this case the exiting sub-namespace init also kills its >>> parent? >> >> I don't think so because the zap_pid_ns_processes does not hit the >> parent process when it browses the pidmap. > > OK... Honestly, right now I can't understand my own question, it was > written a long ago. Probably I missed something.... but I'll recheck ;) > >>> OK, suppose it does fork() after unshare(), then another fork(). >>> In this case the second child lives in the same namespace with >>> init created by the 1st fork, but it is…
creator: Greg Kurz
date: 2011-02-17T22:35:59-00:00

Re: [PATCH 2/2] pidns: Support unsharing the pid namespace.
type: item
description: On 02/17, Greg Kurz wrote: > > On 02/17/2011 09:29 PM, Oleg Nesterov wrote: >> On 02/17, Daniel Lezcano wrote: >>> >>> On 02/15/2011 08:01 PM, Oleg Nesterov wrote: >>>> >>>> I have to admit, I can't say I like this very much. OK, if we need >>>> this, can't we just put something into, say, signal->flags so that >>>> copy_process can check and create the new namespace. >>>> >>>> Also. I remember, I already saw something like this and google found >>>> my questions. I didn't actually read the new version, perhaps my >>>> concerns were already answered... >>>> >>>> But what if the task T does unshare(CLONE_NEWPID) and then, say, >>>> pthread_create() ? Unless I missed something, the new thread won't >>>> be able to see T ? >>> >>> Right. Is it really a problem ? I mean it is a weird use case where we >>> fall in a weird situation. >> >> But this is really weird! How it is possible that the parent can't see >> its own child? No matter which thread did fork(), the new process is > > Hmmm... I guess you mean the opposite. The way pid namespaces are > nested, parents always see their children. Well, yes. But it can't see this child using the same pid number, unless I missed something. > But indeed, the child thread > can't see its group leader and that's kind of unusual. This too. And to me this is more "kind of buggy". But yes, I am biased because I dislike this approach in general ;) And, once again, this patch also lacks the necessary s/nsproxy/atcive_pid_ns/ changes. Anyway. It is very possible I missed something. As I said, I didn't actually read this version and I forgot all I knew about this change before. But afaics this patch is buggy in its current form. Oleg. _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containe rs
creator: Oleg Nesterov
date: 2011-02-18T14:40:19-00:00

Re: [PATCH 1/1][3rd resend] sys_unshare: remove the dead CLONE_THREAD/SIGHAND/VM code
type: item
description: Quoting Oleg Nesterov (oleg@redhat.com): > Cleanup: kill the dead code which does nothing but complicates the code > and confuses the reader. > > sys_unshare(CLONE_THREAD/SIGHAND/VM) is not really implemented, and I doubt > very much it will ever work. At least, nobody even tried since the original > "unshare system call -v5: system call handler function" commit > 99d1419d96d7df9cfa56bc977810be831bd5ef64 was applied more than 4 years ago. > > And the code is not consistent. unshare_thread() always fails unconditionally, > while unshare_sighand() and unshare_vm() pretend to work if there is nothing > to unshare. > > Remove unshare_thread(), unshare_sighand(), unshare_vm() helpers and related > variables and add a simple CLONE_THREAD | CLONE_SIGHAND| CLONE_VM check into > check_unshare_flags(). > > Also, move the "CLONE_NEWNS needs CLONE_FS" check from check_unshare_flags() > to sys_unshare(). This looks more consistent and matches the similar > do_sysvsem check in sys_unshare(). > > Note: with or without this patch "atomic_read(mm->mm_users) > 1" can give > a false positive due to get_task_mm(). > > Signed-off-by: Oleg Nesterov <oleg@redhat.com> > Acked-by: Roland McGrath <roland@redhat.com> Yes, please. Acked-by: Serge Hallyn <serge.hallyn@canonical.com> thanks, -serge > --- > > kernel/fork.c | 123 +++++++++++----------------------------------------------- > 1 file changed, 25 insertions(+), 98 deletions(-) > > --- 2.6.37/kernel/fork.c~unshare-killcrap 2010-11-05 18:03:28.000000000 +0100 > +++ 2.6.37/kernel/fork.c 2010-11-05 18:09:52.000000000 +0100 > @@ -1522,38 +1522,24 @@ void __init proc_caches_init(void) > } > > /* > - * Check constraints on flags passed to the unshare system call and > - * force unsharing of additional process context as appropriate. > + * Check constraints on flags passed to the unshare system call. > */ > -static void check_unshare_flags(unsigned long *flags_ptr) > +static int check_unshare_flags(unsigned long unshare_flags) > { > + if (unsh…
creator: serge
date: 2011-02-21T00:17:43-00:00

Re: [PATCH 2/2] pidns: Support unsharing the pid namespace.
type: item
description: On 02/15/2011 10:53 AM, Daniel Lezcano wrote: > From: Eric W. Biederman <ebiederm@xmission.com> > > - Allow CLONEW_NEWPID into unshare. - Pass both nsproxy->pid_ns and > task_active_pid_ns to copy_pid_ns As they can now be different. > > Unsharing of the pid namespace unlike unsharing of other namespaces > does not take effect immediately. Instead it affects the children > created with fork and clone. The first of these children becomes the > init process of the new pid namespace, the rest become oddball > children of pid 0. From the point of view of the new pid namespace > the process that created it is pid 0, as it's pid does not map. > > A couple of different semantics were considered but this one was > settled on because it is easy to implement and it is usable from pam > modules. The core reasons for the existence of unshare. Hmmm... The userspace semantics I expected were for unshare(CLONE_NEWPID) to: A) make the current process be PID 1 in the new namespace. B) reparent_to_init() any existing children as if the process that called unshare() had exited. (Because those children are not in the new PID namespace.) Is there a reason to implement it in some way other than that? if (!fork) { unshare(CLONE_NEWUSER); exec(); } > I took a survey of the callers of pam modules and the following > appears to be a representative sample of their logic. { setup stuff > include pam child = fork(); if (!child) { setuid() exec /bin/bash } > waitpid(child); > > pam and other cleanup } And calling unshare() right before calling setuid() seems the logical thing to do there...? Currently unshare() works like chroot(). You're making it act like vfork() where the process that did this is in a strange halfway state until it creates new children at which point magic happens. I don't understand why this is an improvement, especially since none of the other flags you can feed to unshare do that. > As you can see there is a fork to create the unprivileged user space > process. Which means…
creator: Rob Landley
date: 2011-02-24T01:12:46-00:00