Re: PATCH? fix unshare(NEWPID) && vfork()

2013-08-22 Thread Oleg Nesterov
On 08/21, Oleg Nesterov wrote: > > Still. Can't we make a single check? Like the initial patch I sent, but > this one moves the check into copy_process() and checks CLONE_* first. > Looks a bit simpler. And more understandable to me but this is subjective. Seriously. CLONE_NEWPID and

Re: PATCH? fix unshare(NEWPID) vfork()

2013-08-22 Thread Oleg Nesterov
On 08/21, Oleg Nesterov wrote: Still. Can't we make a single check? Like the initial patch I sent, but this one moves the check into copy_process() and checks CLONE_* first. Looks a bit simpler. And more understandable to me but this is subjective. Seriously. CLONE_NEWPID and

Re: PATCH? fix unshare(NEWPID) && vfork()

2013-08-21 Thread Oleg Nesterov
On 08/20, Eric W. Biederman wrote: > > Oleg Nesterov writes: > > >> > >> The patch below also needs CLONE_SIGHAND. You can't meaningfully share > >> signal handlers if you can't represent the pid in the siginfo. pids and > >> signals are too interconnected. > > > > I don't really understand. If

Re: PATCH? fix unshare(NEWPID) && vfork()

2013-08-21 Thread Oleg Nesterov
On 08/20, Andy Lutomirski wrote: > > On Tue, Aug 20, 2013 at 12:23 PM, Oleg Nesterov wrote: > > > >> but vfork(); unshare(CLONE_NEWPID) will fail? (I > >> admit I haven't tested it.) > > > > Do you mean that the child does unshare(CLONE_NEWPID) before exec? > > It should fail with or without

Re: PATCH? fix unshare(NEWPID) vfork()

2013-08-21 Thread Oleg Nesterov
On 08/20, Andy Lutomirski wrote: On Tue, Aug 20, 2013 at 12:23 PM, Oleg Nesterov o...@redhat.com wrote: but vfork(); unshare(CLONE_NEWPID) will fail? (I admit I haven't tested it.) Do you mean that the child does unshare(CLONE_NEWPID) before exec? It should fail with or without this

Re: PATCH? fix unshare(NEWPID) vfork()

2013-08-21 Thread Oleg Nesterov
On 08/20, Eric W. Biederman wrote: Oleg Nesterov o...@redhat.com writes: The patch below also needs CLONE_SIGHAND. You can't meaningfully share signal handlers if you can't represent the pid in the siginfo. pids and signals are too interconnected. I don't really understand. If we

Re: PATCH? fix unshare(NEWPID) && vfork()

2013-08-20 Thread Eric W. Biederman
Oleg Nesterov writes: > On 08/20, Eric W. Biederman wrote: >> >> Oleg Nesterov writes: >> >> > On 08/19, Andy Lutomirski wrote: >> >> >> >> On Mon, Aug 19, 2013 at 11:33 AM, Oleg Nesterov wrote: >> >> > >> >> > So do you think this change is fine or not (ignoring the fact it needs >> >> >

Re: PATCH? fix unshare(NEWPID) && vfork()

2013-08-20 Thread Eric W. Biederman
Andy Lutomirski writes: > On Tue, Aug 20, 2013 at 11:50 AM, Oleg Nesterov wrote: >> On 08/20, Andy Lutomirski wrote: >>> >>> On Mon, Aug 19, 2013 at 11:43 AM, Oleg Nesterov wrote: >>> > On 08/19, Andy Lutomirski wrote: >>> >> >>> >> On Mon, Aug 19, 2013 at 11:33 AM, Oleg Nesterov wrote: >>>

Re: PATCH? fix unshare(NEWPID) && vfork()

2013-08-20 Thread Andy Lutomirski
On Tue, Aug 20, 2013 at 12:23 PM, Oleg Nesterov wrote: > On 08/20, Andy Lutomirski wrote: >> >> On Tue, Aug 20, 2013 at 12:05 PM, Oleg Nesterov wrote: >> > On 08/20, Andy Lutomirski wrote: >> Huh? Doesn't this mean that unshare(CLONE_NEWPID); vfork() will work >> with your patches, > > I hope,

Re: PATCH? fix unshare(NEWPID) && vfork()

2013-08-20 Thread Oleg Nesterov
On 08/20, Andy Lutomirski wrote: > > On Tue, Aug 20, 2013 at 12:05 PM, Oleg Nesterov wrote: > > On 08/20, Andy Lutomirski wrote: > >> > >> On Tue, Aug 20, 2013 at 11:50 AM, Oleg Nesterov wrote: > >> > > >> >> Currently (with or without your patch), vfork() followed by > >> >>

Re: PATCH? fix unshare(NEWPID) && vfork()

2013-08-20 Thread Andy Lutomirski
On Tue, Aug 20, 2013 at 12:05 PM, Oleg Nesterov wrote: > On 08/20, Andy Lutomirski wrote: >> >> On Tue, Aug 20, 2013 at 11:50 AM, Oleg Nesterov wrote: >> > >> >> Currently (with or without your patch), vfork() followed by >> >> unshare(CLONE_NEWUSER) or unshare(CLONE_NEWPID) will unshare the VM.

Re: PATCH? fix unshare(NEWPID) && vfork()

2013-08-20 Thread Oleg Nesterov
On 08/20, Andy Lutomirski wrote: > > On Tue, Aug 20, 2013 at 11:50 AM, Oleg Nesterov wrote: > > > >> Currently (with or without your patch), vfork() followed by > >> unshare(CLONE_NEWUSER) or unshare(CLONE_NEWPID) will unshare the VM. > > > > Could you spell please? > > > > We never unshare the

Re: PATCH? fix unshare(NEWPID) && vfork()

2013-08-20 Thread Andy Lutomirski
On Tue, Aug 20, 2013 at 11:50 AM, Oleg Nesterov wrote: > On 08/20, Andy Lutomirski wrote: >> >> On Mon, Aug 19, 2013 at 11:43 AM, Oleg Nesterov wrote: >> > On 08/19, Andy Lutomirski wrote: >> >> >> >> On Mon, Aug 19, 2013 at 11:33 AM, Oleg Nesterov wrote: >> >> > >> >> > So do you think this

Re: PATCH? fix unshare(NEWPID) && vfork()

2013-08-20 Thread Oleg Nesterov
On 08/20, Andy Lutomirski wrote: > > On Mon, Aug 19, 2013 at 11:43 AM, Oleg Nesterov wrote: > > On 08/19, Andy Lutomirski wrote: > >> > >> On Mon, Aug 19, 2013 at 11:33 AM, Oleg Nesterov wrote: > >> > > >> > So do you think this change is fine or not (ignoring the fact it needs > >> > cleanups)

Re: PATCH? fix unshare(NEWPID) && vfork()

2013-08-20 Thread Oleg Nesterov
On 08/20, Eric W. Biederman wrote: > > Oleg Nesterov writes: > > > On 08/19, Andy Lutomirski wrote: > >> > >> On Mon, Aug 19, 2013 at 11:33 AM, Oleg Nesterov wrote: > >> > > >> > So do you think this change is fine or not (ignoring the fact it needs > >> > cleanups) ? > >> > >> I think that

Re: PATCH? fix unshare(NEWPID) && vfork()

2013-08-20 Thread Andy Lutomirski
On Mon, Aug 19, 2013 at 11:43 AM, Oleg Nesterov wrote: > On 08/19, Andy Lutomirski wrote: >> >> On Mon, Aug 19, 2013 at 11:33 AM, Oleg Nesterov wrote: >> > >> > So do you think this change is fine or not (ignoring the fact it needs >> > cleanups) ? >> >> I think that removing the CLONE_VM check

Re: PATCH? fix unshare(NEWPID) && vfork()

2013-08-20 Thread Eric W. Biederman
Oleg Nesterov writes: > On 08/19, Andy Lutomirski wrote: >> >> On Mon, Aug 19, 2013 at 11:33 AM, Oleg Nesterov wrote: >> > >> > So do you think this change is fine or not (ignoring the fact it needs >> > cleanups) ? >> >> I think that removing the CLONE_VM check is fine (although there are >>

Re: PATCH? fix unshare(NEWPID) vfork()

2013-08-20 Thread Eric W. Biederman
Oleg Nesterov o...@redhat.com writes: On 08/19, Andy Lutomirski wrote: On Mon, Aug 19, 2013 at 11:33 AM, Oleg Nesterov o...@redhat.com wrote: So do you think this change is fine or not (ignoring the fact it needs cleanups) ? I think that removing the CLONE_VM check is fine (although

Re: PATCH? fix unshare(NEWPID) vfork()

2013-08-20 Thread Andy Lutomirski
On Mon, Aug 19, 2013 at 11:43 AM, Oleg Nesterov o...@redhat.com wrote: On 08/19, Andy Lutomirski wrote: On Mon, Aug 19, 2013 at 11:33 AM, Oleg Nesterov o...@redhat.com wrote: So do you think this change is fine or not (ignoring the fact it needs cleanups) ? I think that removing the

Re: PATCH? fix unshare(NEWPID) vfork()

2013-08-20 Thread Oleg Nesterov
On 08/20, Eric W. Biederman wrote: Oleg Nesterov o...@redhat.com writes: On 08/19, Andy Lutomirski wrote: On Mon, Aug 19, 2013 at 11:33 AM, Oleg Nesterov o...@redhat.com wrote: So do you think this change is fine or not (ignoring the fact it needs cleanups) ? I think that

Re: PATCH? fix unshare(NEWPID) vfork()

2013-08-20 Thread Oleg Nesterov
On 08/20, Andy Lutomirski wrote: On Mon, Aug 19, 2013 at 11:43 AM, Oleg Nesterov o...@redhat.com wrote: On 08/19, Andy Lutomirski wrote: On Mon, Aug 19, 2013 at 11:33 AM, Oleg Nesterov o...@redhat.com wrote: So do you think this change is fine or not (ignoring the fact it needs

Re: PATCH? fix unshare(NEWPID) vfork()

2013-08-20 Thread Andy Lutomirski
On Tue, Aug 20, 2013 at 11:50 AM, Oleg Nesterov o...@redhat.com wrote: On 08/20, Andy Lutomirski wrote: On Mon, Aug 19, 2013 at 11:43 AM, Oleg Nesterov o...@redhat.com wrote: On 08/19, Andy Lutomirski wrote: On Mon, Aug 19, 2013 at 11:33 AM, Oleg Nesterov o...@redhat.com wrote: So

Re: PATCH? fix unshare(NEWPID) vfork()

2013-08-20 Thread Oleg Nesterov
On 08/20, Andy Lutomirski wrote: On Tue, Aug 20, 2013 at 11:50 AM, Oleg Nesterov o...@redhat.com wrote: Currently (with or without your patch), vfork() followed by unshare(CLONE_NEWUSER) or unshare(CLONE_NEWPID) will unshare the VM. Could you spell please? We never unshare the VM.

Re: PATCH? fix unshare(NEWPID) vfork()

2013-08-20 Thread Andy Lutomirski
On Tue, Aug 20, 2013 at 12:05 PM, Oleg Nesterov o...@redhat.com wrote: On 08/20, Andy Lutomirski wrote: On Tue, Aug 20, 2013 at 11:50 AM, Oleg Nesterov o...@redhat.com wrote: Currently (with or without your patch), vfork() followed by unshare(CLONE_NEWUSER) or unshare(CLONE_NEWPID) will

Re: PATCH? fix unshare(NEWPID) vfork()

2013-08-20 Thread Oleg Nesterov
On 08/20, Andy Lutomirski wrote: On Tue, Aug 20, 2013 at 12:05 PM, Oleg Nesterov o...@redhat.com wrote: On 08/20, Andy Lutomirski wrote: On Tue, Aug 20, 2013 at 11:50 AM, Oleg Nesterov o...@redhat.com wrote: Currently (with or without your patch), vfork() followed by

Re: PATCH? fix unshare(NEWPID) vfork()

2013-08-20 Thread Andy Lutomirski
On Tue, Aug 20, 2013 at 12:23 PM, Oleg Nesterov o...@redhat.com wrote: On 08/20, Andy Lutomirski wrote: On Tue, Aug 20, 2013 at 12:05 PM, Oleg Nesterov o...@redhat.com wrote: On 08/20, Andy Lutomirski wrote: Huh? Doesn't this mean that unshare(CLONE_NEWPID); vfork() will work with your

Re: PATCH? fix unshare(NEWPID) vfork()

2013-08-20 Thread Eric W. Biederman
Andy Lutomirski l...@amacapital.net writes: On Tue, Aug 20, 2013 at 11:50 AM, Oleg Nesterov o...@redhat.com wrote: On 08/20, Andy Lutomirski wrote: On Mon, Aug 19, 2013 at 11:43 AM, Oleg Nesterov o...@redhat.com wrote: On 08/19, Andy Lutomirski wrote: On Mon, Aug 19, 2013 at 11:33 AM,

Re: PATCH? fix unshare(NEWPID) vfork()

2013-08-20 Thread Eric W. Biederman
Oleg Nesterov o...@redhat.com writes: On 08/20, Eric W. Biederman wrote: Oleg Nesterov o...@redhat.com writes: On 08/19, Andy Lutomirski wrote: On Mon, Aug 19, 2013 at 11:33 AM, Oleg Nesterov o...@redhat.com wrote: So do you think this change is fine or not (ignoring the fact it

Re: PATCH? fix unshare(NEWPID) && vfork()

2013-08-19 Thread Oleg Nesterov
On 08/19, Andy Lutomirski wrote: > > On Mon, Aug 19, 2013 at 11:33 AM, Oleg Nesterov wrote: > > > > So do you think this change is fine or not (ignoring the fact it needs > > cleanups) ? > > I think that removing the CLONE_VM check is fine (although there are > some other ones that should

Re: PATCH? fix unshare(NEWPID) && vfork()

2013-08-19 Thread Andy Lutomirski
On Mon, Aug 19, 2013 at 11:33 AM, Oleg Nesterov wrote: > On 08/19, Andy Lutomirski wrote: >> >> On Mon, Aug 19, 2013 at 10:25 AM, Oleg Nesterov wrote: >> > Hello. >> > >> > Colin reports that vfork() doesn't work after unshare(PIDNS). The >> > reason is trivial, copy_process() does: >> > >> >

Re: PATCH? fix unshare(NEWPID) && vfork()

2013-08-19 Thread Oleg Nesterov
On 08/19, Andy Lutomirski wrote: > > On Mon, Aug 19, 2013 at 10:25 AM, Oleg Nesterov wrote: > > Hello. > > > > Colin reports that vfork() doesn't work after unshare(PIDNS). The > > reason is trivial, copy_process() does: > > > > /* > > * If the new process will be in a different

Re: PATCH? fix unshare(NEWPID) && vfork()

2013-08-19 Thread Andy Lutomirski
On Mon, Aug 19, 2013 at 10:25 AM, Oleg Nesterov wrote: > Hello. > > Colin reports that vfork() doesn't work after unshare(PIDNS). The > reason is trivial, copy_process() does: > > /* > * If the new process will be in a different pid namespace > * don't allow the creation

Re: PATCH? fix unshare(NEWPID) && vfork()

2013-08-19 Thread Oleg Nesterov
On 08/19, Linus Torvalds wrote: > > So I think your patch is correct, although I'm not sure why you move > the test. I didn't really, we have 2 tests, in do_fork() and copy_process(). I think it would be better to do the similar checks in one place. This is off-topic and needs a separate change

Re: PATCH? fix unshare(NEWPID) && vfork()

2013-08-19 Thread Linus Torvalds
On Mon, Aug 19, 2013 at 10:25 AM, Oleg Nesterov wrote: > > Colin reports that vfork() doesn't work after unshare(PIDNS). The > reason is trivial, copy_process() does: > > /* > * If the new process will be in a different pid namespace > * don't allow the creation of

PATCH? fix unshare(NEWPID) && vfork()

2013-08-19 Thread Oleg Nesterov
Hello. Colin reports that vfork() doesn't work after unshare(PIDNS). The reason is trivial, copy_process() does: /* * If the new process will be in a different pid namespace * don't allow the creation of threads. */ if ((clone_flags &

PATCH? fix unshare(NEWPID) vfork()

2013-08-19 Thread Oleg Nesterov
Hello. Colin reports that vfork() doesn't work after unshare(PIDNS). The reason is trivial, copy_process() does: /* * If the new process will be in a different pid namespace * don't allow the creation of threads. */ if ((clone_flags

Re: PATCH? fix unshare(NEWPID) vfork()

2013-08-19 Thread Linus Torvalds
On Mon, Aug 19, 2013 at 10:25 AM, Oleg Nesterov o...@redhat.com wrote: Colin reports that vfork() doesn't work after unshare(PIDNS). The reason is trivial, copy_process() does: /* * If the new process will be in a different pid namespace * don't allow the creation

Re: PATCH? fix unshare(NEWPID) vfork()

2013-08-19 Thread Oleg Nesterov
On 08/19, Linus Torvalds wrote: So I think your patch is correct, although I'm not sure why you move the test. I didn't really, we have 2 tests, in do_fork() and copy_process(). I think it would be better to do the similar checks in one place. This is off-topic and needs a separate change

Re: PATCH? fix unshare(NEWPID) vfork()

2013-08-19 Thread Andy Lutomirski
On Mon, Aug 19, 2013 at 10:25 AM, Oleg Nesterov o...@redhat.com wrote: Hello. Colin reports that vfork() doesn't work after unshare(PIDNS). The reason is trivial, copy_process() does: /* * If the new process will be in a different pid namespace * don't allow the

Re: PATCH? fix unshare(NEWPID) vfork()

2013-08-19 Thread Oleg Nesterov
On 08/19, Andy Lutomirski wrote: On Mon, Aug 19, 2013 at 10:25 AM, Oleg Nesterov o...@redhat.com wrote: Hello. Colin reports that vfork() doesn't work after unshare(PIDNS). The reason is trivial, copy_process() does: /* * If the new process will be in a different

Re: PATCH? fix unshare(NEWPID) vfork()

2013-08-19 Thread Andy Lutomirski
On Mon, Aug 19, 2013 at 11:33 AM, Oleg Nesterov o...@redhat.com wrote: On 08/19, Andy Lutomirski wrote: On Mon, Aug 19, 2013 at 10:25 AM, Oleg Nesterov o...@redhat.com wrote: Hello. Colin reports that vfork() doesn't work after unshare(PIDNS). The reason is trivial, copy_process() does:

Re: PATCH? fix unshare(NEWPID) vfork()

2013-08-19 Thread Oleg Nesterov
On 08/19, Andy Lutomirski wrote: On Mon, Aug 19, 2013 at 11:33 AM, Oleg Nesterov o...@redhat.com wrote: So do you think this change is fine or not (ignoring the fact it needs cleanups) ? I think that removing the CLONE_VM check is fine (although there are some other ones that should