Quoting Eric W. Biederman (ebiederm@xxxxxxxxxxxx):
>
> Serge Hallyn <serge.hallyn@xxxxxxxxxx> writes:
> > Hi Oleg,
> >
> > commit 40a0d32d1eaffe6aac7324ca92604b6b3977eb0e :
> > "fork: unify and tighten up CLONE_NEWUSER/CLONE_NEWPID checks"
> > breaks lxc-attach in 3.12. That code forks a child which does
> > setns() and then does a clone(CLONE_PARENT). That way the
> > grandchild can be in the right namespaces (which the child was
> > not) and be a child of the original task, which is the monitor.
> >
> > lxc-attach in 3.11 was working fine with no side effects that I
> > could see. Is there a real danger in allowing CLONE_PARENT
> > when current->nsproxy->pidns_for_children is not our pidns,
> > or was this done out of an "over-abundance of caution"? Can we
> > safely revert that new extra check?
>
> The two fundamental things I know we can not allow are:
> - A shared signal queue aka CLONE_THREAD. Because we compute the pid
> and uid of the signal when we place it in the queue.
>
> - Changing the pid and by extention pid_namespace of an existing
> process.
>
> >From a parents perspective there is nothing special about the pid
> namespace, to deny CLONE_PARENT, because the parent simply won't know or
> care.
>
> >From the childs perspective all that is special really are shared signal
> queues.
>
> User mode threading with CLONE_PARENT|CLONE_VM|CLONE_SIGHAND and tasks
> in different pid namespaces is almost certainly going to break because
> it is complicated. But shared signal handlers can look at per thread
> information to know which pid namespace a process is in, so I don't know
> of any reason not to support CLONE_PARENT|CLONE_VM|CLONE_SIGHAND threads
> at the kernel level. It would be absolutely stupid to implement but
> that is a different thing.
>
> So hmm.
>
> Because it can do no harm, and because it is a regression let's remove
> the CLONE_PARENT check and send it stable.
> Cc: stable@xxxxxxxxxxxxxxx
> Acked-by: Oleg Nesterov <oleg@xxxxxxxxxx>
> Acked-by: Andy Lutomirski <luto@xxxxxxxxxxxxxx>
> Acked-by: Serge E. Hallyn <serge.hallyn@xxxxxxxxxx>
Thanks, Eric.
> Signed-off-by: "Eric W. Biederman" <ebiederm@xxxxxxxxxxxx>
> ---
> kernel/fork.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/kernel/fork.c b/kernel/fork.c
> index 728d5be9548c..f82fa2ee7581 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -1171,7 +1171,7 @@ static struct task_struct *copy_process(unsigned long clone_flags,
> * do not allow it to share a thread group or signal handlers or
> * parent with the forking task.
> */
> - if (clone_flags & (CLONE_SIGHAND | CLONE_PARENT)) {
> + if (clone_flags & CLONE_SIGHAND) {
> if ((clone_flags & (CLONE_NEWUSER | CLONE_NEWPID)) ||
> (task_active_pid_ns(current) !=
> current->nsproxy->pid_ns_for_children))
> --
> 1.7.5.4
_______________________________________________
Containers mailing list
Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/containers
