* [PATCH 4.9 093/130] ARC: Improve cmpxchg syscall implementation [not found] <20180823074927.161454870@linuxfoundation.org> @ 2018-08-23 7:53 ` Greg Kroah-Hartman 2018-08-23 7:53 ` Greg Kroah-Hartman 0 siblings, 1 reply; 2+ messages in thread From: Greg Kroah-Hartman @ 2018-08-23 7:53 UTC (permalink / raw) To: linux-kernel Cc: Greg Kroah-Hartman, stable, Max Filippov, linux-arch, Alexey Brodkin, Peter Zijlstra, Vineet Gupta, Sasha Levin 4.9-stable review patch. If anyone has any objections, please let me know. ------------------ From: Peter Zijlstra <peterz@infradead.org> [ Upstream commit e8708786d4fe21c043d38d760f768949a3d71185 ] This is used in configs lacking hardware atomics to emulate atomic r-m-w for user space, implemented by disabling preemption in kernel. However there are issues in current implementation: 1. Process not terminated if invalid user pointer passed: i.e. __get_user() failed. 2. The reason for this patch was __put_user() failure not being handled either, specifically for the COW break scenario. The zero page is initially wired up and read from __get_user() succeeds. A subsequent write by __put_user() induces a Protection Violation, but COW can't finish as Linux page fault handler is disabled due to preempt disable. And what's worse is we silently return the stale value to user space. Fix this specific case by re-enabling preemption and explicitly fixing up the fault and retrying the whole sequence over. Cc: Max Filippov <jcmvbkbc@gmail.com> Cc: linux-arch@vger.kernel.org Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com> Signed-off-by: Peter Zijlstra <peterz@infradead.org> Signed-off-by: Vineet Gupta <vgupta@synopsys.com> [vgupta: rewrote the changelog] Signed-off-by: Sasha Levin <alexander.levin@microsoft.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> --- arch/arc/kernel/process.c | 47 +++++++++++++++++++++++++++++++++++----------- 1 file changed, 36 insertions(+), 11 deletions(-) --- a/arch/arc/kernel/process.c +++ b/arch/arc/kernel/process.c @@ -44,7 +44,8 @@ SYSCALL_DEFINE0(arc_gettls) SYSCALL_DEFINE3(arc_usr_cmpxchg, int *, uaddr, int, expected, int, new) { struct pt_regs *regs = current_pt_regs(); - int uval = -EFAULT; + u32 uval; + int ret; /* * This is only for old cores lacking LLOCK/SCOND, which by defintion @@ -57,23 +58,47 @@ SYSCALL_DEFINE3(arc_usr_cmpxchg, int *, /* Z indicates to userspace if operation succeded */ regs->status32 &= ~STATUS_Z_MASK; - if (!access_ok(VERIFY_WRITE, uaddr, sizeof(int))) - return -EFAULT; + ret = access_ok(VERIFY_WRITE, uaddr, sizeof(*uaddr)); + if (!ret) + goto fail; +again: preempt_disable(); - if (__get_user(uval, uaddr)) - goto done; + ret = __get_user(uval, uaddr); + if (ret) + goto fault; - if (uval == expected) { - if (!__put_user(new, uaddr)) - regs->status32 |= STATUS_Z_MASK; - } + if (uval != expected) + goto out; -done: - preempt_enable(); + ret = __put_user(new, uaddr); + if (ret) + goto fault; + + regs->status32 |= STATUS_Z_MASK; +out: + preempt_enable(); return uval; + +fault: + preempt_enable(); + + if (unlikely(ret != -EFAULT)) + goto fail; + + down_read(¤t->mm->mmap_sem); + ret = fixup_user_fault(current, current->mm, (unsigned long) uaddr, + FAULT_FLAG_WRITE, NULL); + up_read(¤t->mm->mmap_sem); + + if (likely(!ret)) + goto again; + +fail: + force_sig(SIGSEGV, current); + return ret; } void arch_cpu_idle(void) ^ permalink raw reply [flat|nested] 2+ messages in thread
* [PATCH 4.9 093/130] ARC: Improve cmpxchg syscall implementation 2018-08-23 7:53 ` [PATCH 4.9 093/130] ARC: Improve cmpxchg syscall implementation Greg Kroah-Hartman @ 2018-08-23 7:53 ` Greg Kroah-Hartman 0 siblings, 0 replies; 2+ messages in thread From: Greg Kroah-Hartman @ 2018-08-23 7:53 UTC (permalink / raw) To: linux-kernel Cc: Greg Kroah-Hartman, stable, Max Filippov, linux-arch, Alexey Brodkin, Peter Zijlstra, Vineet Gupta, Sasha Levin 4.9-stable review patch. If anyone has any objections, please let me know. ------------------ From: Peter Zijlstra <peterz@infradead.org> [ Upstream commit e8708786d4fe21c043d38d760f768949a3d71185 ] This is used in configs lacking hardware atomics to emulate atomic r-m-w for user space, implemented by disabling preemption in kernel. However there are issues in current implementation: 1. Process not terminated if invalid user pointer passed: i.e. __get_user() failed. 2. The reason for this patch was __put_user() failure not being handled either, specifically for the COW break scenario. The zero page is initially wired up and read from __get_user() succeeds. A subsequent write by __put_user() induces a Protection Violation, but COW can't finish as Linux page fault handler is disabled due to preempt disable. And what's worse is we silently return the stale value to user space. Fix this specific case by re-enabling preemption and explicitly fixing up the fault and retrying the whole sequence over. Cc: Max Filippov <jcmvbkbc@gmail.com> Cc: linux-arch@vger.kernel.org Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com> Signed-off-by: Peter Zijlstra <peterz@infradead.org> Signed-off-by: Vineet Gupta <vgupta@synopsys.com> [vgupta: rewrote the changelog] Signed-off-by: Sasha Levin <alexander.levin@microsoft.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> --- arch/arc/kernel/process.c | 47 +++++++++++++++++++++++++++++++++++----------- 1 file changed, 36 insertions(+), 11 deletions(-) --- a/arch/arc/kernel/process.c +++ b/arch/arc/kernel/process.c @@ -44,7 +44,8 @@ SYSCALL_DEFINE0(arc_gettls) SYSCALL_DEFINE3(arc_usr_cmpxchg, int *, uaddr, int, expected, int, new) { struct pt_regs *regs = current_pt_regs(); - int uval = -EFAULT; + u32 uval; + int ret; /* * This is only for old cores lacking LLOCK/SCOND, which by defintion @@ -57,23 +58,47 @@ SYSCALL_DEFINE3(arc_usr_cmpxchg, int *, /* Z indicates to userspace if operation succeded */ regs->status32 &= ~STATUS_Z_MASK; - if (!access_ok(VERIFY_WRITE, uaddr, sizeof(int))) - return -EFAULT; + ret = access_ok(VERIFY_WRITE, uaddr, sizeof(*uaddr)); + if (!ret) + goto fail; +again: preempt_disable(); - if (__get_user(uval, uaddr)) - goto done; + ret = __get_user(uval, uaddr); + if (ret) + goto fault; - if (uval == expected) { - if (!__put_user(new, uaddr)) - regs->status32 |= STATUS_Z_MASK; - } + if (uval != expected) + goto out; -done: - preempt_enable(); + ret = __put_user(new, uaddr); + if (ret) + goto fault; + + regs->status32 |= STATUS_Z_MASK; +out: + preempt_enable(); return uval; + +fault: + preempt_enable(); + + if (unlikely(ret != -EFAULT)) + goto fail; + + down_read(¤t->mm->mmap_sem); + ret = fixup_user_fault(current, current->mm, (unsigned long) uaddr, + FAULT_FLAG_WRITE, NULL); + up_read(¤t->mm->mmap_sem); + + if (likely(!ret)) + goto again; + +fail: + force_sig(SIGSEGV, current); + return ret; } void arch_cpu_idle(void) ^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2018-08-23 11:41 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20180823074927.161454870@linuxfoundation.org>
2018-08-23 7:53 ` [PATCH 4.9 093/130] ARC: Improve cmpxchg syscall implementation Greg Kroah-Hartman
2018-08-23 7:53 ` Greg Kroah-Hartman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).