Skip to content

Commit

Permalink
ARC: Improve cmpxchg syscall implementation
Browse files Browse the repository at this point in the history
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]
  • Loading branch information
Peter Zijlstra authored and Vineet Gupta committed Jul 9, 2018
1 parent ec58ba1 commit e870878
Showing 1 changed file with 36 additions and 11 deletions.
47 changes: 36 additions & 11 deletions arch/arc/kernel/process.c
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,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
Expand All @@ -60,23 +61,47 @@ SYSCALL_DEFINE3(arc_usr_cmpxchg, int *, uaddr, int, expected, int, new)
/* 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(&current->mm->mmap_sem);
ret = fixup_user_fault(current, current->mm, (unsigned long) uaddr,
FAULT_FLAG_WRITE, NULL);
up_read(&current->mm->mmap_sem);

if (likely(!ret))
goto again;

fail:
force_sig(SIGSEGV, current);
return ret;
}

#ifdef CONFIG_ISA_ARCV2
Expand Down

0 comments on commit e870878

Please sign in to comment.