Messages in this thread | | | Date | Sat, 22 Dec 2012 13:39:20 -0600 | From | Rob Landley <> | Subject | Re: [PATCH review 1/3] pidns: Outlaw thread creation after unshare(CLONE_NEWPID) |
| |
On 12/21/2012 10:57:34 PM, Eric W. Biederman wrote: > > The sequence: > unshare(CLONE_NEWPID) > clone(CLONE_THREAD|CLONE_SIGHAND|CLONE_VM) > > Creates a new process in the new pid namespace without setting > pid_ns->child_reaper. After forking this results in a NULL > pointer dereference. > > Avoid this and other nonsense scenarios that can show up after > creating a new pid namespace with unshare by adding a new > check in copy_prodcess. > > Pointed-out-by: Oleg Nesterov <oleg@redhat.com> > Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> > --- > kernel/fork.c | 8 ++++++++ > 1 files changed, 8 insertions(+), 0 deletions(-) > > diff --git a/kernel/fork.c b/kernel/fork.c > index a31b823..65ca6d2 100644 > --- a/kernel/fork.c > +++ b/kernel/fork.c > @@ -1166,6 +1166,14 @@ static struct task_struct > *copy_process(unsigned long clone_flags, > current->signal->flags & > SIGNAL_UNKILLABLE) > return ERR_PTR(-EINVAL); > > + /* > + * If the new process will be in a different pid namespace > + * don't allow the creation of threads. > + */ > + if ((clone_flags & (CLONE_VM|CLONE_NEWPID)) && > + (task_active_pid_ns(current) != current->nsproxy->pid_ns)) > + return ERR_PTR(-EINVAL); > +
Since the first bit will trigger if clone_flags has just CLONE_VM without CLONE_NEWPID, or vice versa, I'm guessing this is a fast path optimization? (Otherwise you meant (clone_flags & (CLONE_VM|CLONE_NEWPID)) == CLONE_VM|CLONE_NEWPID ?)
(Just trying to wrap my head around it...)
Rob
| |