Messages in this thread | | | From | Andy Lutomirski <> | Date | Thu, 22 Aug 2013 11:29:12 -0700 | Subject | Re: [PATCH 3/3] fork: unify and tighten up CLONE_NEWUSER/CLONE_NEWPID checks |
| |
On Thu, Aug 22, 2013 at 11:15 AM, Oleg Nesterov <oleg@redhat.com> wrote: > On 08/22, Andy Lutomirski wrote: >> >> On Thu, Aug 22, 2013 at 10:10 AM, Oleg Nesterov <oleg@redhat.com> wrote: >> > do_fork() denies CLONE_THREAD | CLONE_PARENT if NEWUSER | NEWPID. >> > >> > Then later copy_process() denies CLONE_SIGHAND if the new process >> > will be in a different pid namespace (task_active_pid_ns() doesn't >> > match current->nsproxy->pid_ns). >> > >> > This looks confusing and inconsistent. CLONE_NEWPID is very similar >> > to the case when ->pid_ns was already unshared, we want the same >> > restrictions so copy_process() should also nack CLONE_PARENT. >> > >> > And it would be better to deny CLONE_NEWUSER && CLONE_SIGHAND as >> > well just for consistency. >> > >> > Kill the "CLONE_NEWUSER | CLONE_NEWPID" check in do_fork() and >> > change copy_process() to the same check along with nsproxy->pid_ns >> > we already have. >> >> Did the old code actually prevent clone(CLONE_PARENT | CLONE_NEWPID)? >> The new code explicitly does, and that looks like a good thing. > > > Yes. Before this patch do_fork() did: > > if (clone_flags & (CLONE_NEWUSER | CLONE_NEWPID)) { > if (clone_flags & (CLONE_THREAD|CLONE_PARENT)) > return -EINVAL; > } > > however, let me repeat, CLONE_PARENT after unshare(CLONE_NEWPID) was > allowed. With this patch CLONE_PARENT is nacked in both cases. >
Is this -stable-worthy?
--Andy
> Oleg. >
-- Andy Lutomirski AMA Capital Management, LLC
| |