Re: memcg && uaccess (Was: [PATCH 1/2] kernel/fork: handle put_user errors for CLONE_CHILD_SETTID/CLEARTID)

2015-02-10 Thread Oleg Nesterov
On 02/10, Johannes Weiner wrote: > > We had reports of systems deadlocking because Yes, yes, to some degree I understand why it was done this way. Not that I understand the details of course. Thanks for your explanations. > > How can a system call know it should return -ENOMEM if put_user() can

Re: memcg && uaccess (Was: [PATCH 1/2] kernel/fork: handle put_user errors for CLONE_CHILD_SETTID/CLEARTID)

2015-02-10 Thread Johannes Weiner
On Fri, Feb 06, 2015 at 09:32:46PM +0100, Oleg Nesterov wrote: > Add cc's. > > On 02/06, Oleg Nesterov wrote: > > > > And in fact I think that this is not set_child_tid/etc-specific. Perhaps > > I am totally confused, but I think that put_user() simply should not fail > > this way. Say, why a

Re: memcg uaccess (Was: [PATCH 1/2] kernel/fork: handle put_user errors for CLONE_CHILD_SETTID/CLEARTID)

2015-02-10 Thread Johannes Weiner
On Fri, Feb 06, 2015 at 09:32:46PM +0100, Oleg Nesterov wrote: Add cc's. On 02/06, Oleg Nesterov wrote: And in fact I think that this is not set_child_tid/etc-specific. Perhaps I am totally confused, but I think that put_user() simply should not fail this way. Say, why a syscall should

Re: memcg uaccess (Was: [PATCH 1/2] kernel/fork: handle put_user errors for CLONE_CHILD_SETTID/CLEARTID)

2015-02-10 Thread Oleg Nesterov
On 02/10, Johannes Weiner wrote: We had reports of systems deadlocking because Yes, yes, to some degree I understand why it was done this way. Not that I understand the details of course. Thanks for your explanations. How can a system call know it should return -ENOMEM if put_user() can only

memcg && uaccess (Was: [PATCH 1/2] kernel/fork: handle put_user errors for CLONE_CHILD_SETTID/CLEARTID)

2015-02-06 Thread Oleg Nesterov
Add cc's. On 02/06, Oleg Nesterov wrote: > > And in fact I think that this is not set_child_tid/etc-specific. Perhaps > I am totally confused, but I think that put_user() simply should not fail > this way. Say, why a syscall should return -EFAULT if memory allocation > "silently" fails? Confused.

Re: [PATCH 1/2] kernel/fork: handle put_user errors for CLONE_CHILD_SETTID/CLEARTID

2015-02-06 Thread Konstantin Khlebnikov
On Fri, Feb 6, 2015 at 10:55 PM, Oleg Nesterov wrote: > On 02/06, Oleg Nesterov wrote: >> >> On 02/06, Konstantin Khlebnikov wrote: >> > >> > Whole sequence looks like: task calls fork, glibc calls syscall clone with >> > CLONE_CHILD_SETTID and passes pointer to TLS THREAD_SELF->tid as argument.

Re: [PATCH 1/2] kernel/fork: handle put_user errors for CLONE_CHILD_SETTID/CLEARTID

2015-02-06 Thread Oleg Nesterov
On 02/06, Oleg Nesterov wrote: > > On 02/06, Konstantin Khlebnikov wrote: > > > > Whole sequence looks like: task calls fork, glibc calls syscall clone with > > CLONE_CHILD_SETTID and passes pointer to TLS THREAD_SELF->tid as argument. > > Child task gets read-only copy of VM including TLS. Child

Re: [PATCH 1/2] kernel/fork: handle put_user errors for CLONE_CHILD_SETTID/CLEARTID

2015-02-06 Thread Oleg Nesterov
On 02/06, Konstantin Khlebnikov wrote: > > Whole sequence looks like: task calls fork, glibc calls syscall clone with > CLONE_CHILD_SETTID and passes pointer to TLS THREAD_SELF->tid as argument. > Child task gets read-only copy of VM including TLS. Child calls put_user() > to handle

[PATCH 1/2] kernel/fork: handle put_user errors for CLONE_CHILD_SETTID/CLEARTID

2015-02-06 Thread Konstantin Khlebnikov
Currently kernel ignores put_user() errors when it writes tid for syscall clone flags CLONE_CHILD_SETTID and CLONE_CHILD_CLEARTID. Unfortunately this code always worked in this way. We cannot abort syscall if client tid pointer is invalid. This patch adds get_user() after failed put_user(): if

Re: [PATCH 1/2] kernel/fork: handle put_user errors for CLONE_CHILD_SETTID/CLEARTID

2015-02-06 Thread Konstantin Khlebnikov
On Fri, Feb 6, 2015 at 10:55 PM, Oleg Nesterov o...@redhat.com wrote: On 02/06, Oleg Nesterov wrote: On 02/06, Konstantin Khlebnikov wrote: Whole sequence looks like: task calls fork, glibc calls syscall clone with CLONE_CHILD_SETTID and passes pointer to TLS THREAD_SELF-tid as argument.

Re: [PATCH 1/2] kernel/fork: handle put_user errors for CLONE_CHILD_SETTID/CLEARTID

2015-02-06 Thread Oleg Nesterov
On 02/06, Konstantin Khlebnikov wrote: Whole sequence looks like: task calls fork, glibc calls syscall clone with CLONE_CHILD_SETTID and passes pointer to TLS THREAD_SELF-tid as argument. Child task gets read-only copy of VM including TLS. Child calls put_user() to handle CLONE_CHILD_SETTID

Re: [PATCH 1/2] kernel/fork: handle put_user errors for CLONE_CHILD_SETTID/CLEARTID

2015-02-06 Thread Oleg Nesterov
On 02/06, Oleg Nesterov wrote: On 02/06, Konstantin Khlebnikov wrote: Whole sequence looks like: task calls fork, glibc calls syscall clone with CLONE_CHILD_SETTID and passes pointer to TLS THREAD_SELF-tid as argument. Child task gets read-only copy of VM including TLS. Child calls

memcg uaccess (Was: [PATCH 1/2] kernel/fork: handle put_user errors for CLONE_CHILD_SETTID/CLEARTID)

2015-02-06 Thread Oleg Nesterov
Add cc's. On 02/06, Oleg Nesterov wrote: And in fact I think that this is not set_child_tid/etc-specific. Perhaps I am totally confused, but I think that put_user() simply should not fail this way. Say, why a syscall should return -EFAULT if memory allocation silently fails? Confused.

[PATCH 1/2] kernel/fork: handle put_user errors for CLONE_CHILD_SETTID/CLEARTID

2015-02-06 Thread Konstantin Khlebnikov
Currently kernel ignores put_user() errors when it writes tid for syscall clone flags CLONE_CHILD_SETTID and CLONE_CHILD_CLEARTID. Unfortunately this code always worked in this way. We cannot abort syscall if client tid pointer is invalid. This patch adds get_user() after failed put_user(): if