Skip to content

Commit e870878

Browse files
Peter Zijlstravineetgarc
authored andcommitted
ARC: Improve cmpxchg syscall implementation
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 <[email protected]> Cc: [email protected] Signed-off-by: Alexey Brodkin <[email protected]> Signed-off-by: Peter Zijlstra <[email protected]> Signed-off-by: Vineet Gupta <[email protected]> [vgupta: rewrote the changelog]
1 parent ec58ba1 commit e870878

File tree

1 file changed

+36
-11
lines changed

1 file changed

+36
-11
lines changed

arch/arc/kernel/process.c

Lines changed: 36 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,8 @@ SYSCALL_DEFINE0(arc_gettls)
4747
SYSCALL_DEFINE3(arc_usr_cmpxchg, int *, uaddr, int, expected, int, new)
4848
{
4949
struct pt_regs *regs = current_pt_regs();
50-
int uval = -EFAULT;
50+
u32 uval;
51+
int ret;
5152

5253
/*
5354
* This is only for old cores lacking LLOCK/SCOND, which by defintion
@@ -60,23 +61,47 @@ SYSCALL_DEFINE3(arc_usr_cmpxchg, int *, uaddr, int, expected, int, new)
6061
/* Z indicates to userspace if operation succeded */
6162
regs->status32 &= ~STATUS_Z_MASK;
6263

63-
if (!access_ok(VERIFY_WRITE, uaddr, sizeof(int)))
64-
return -EFAULT;
64+
ret = access_ok(VERIFY_WRITE, uaddr, sizeof(*uaddr));
65+
if (!ret)
66+
goto fail;
6567

68+
again:
6669
preempt_disable();
6770

68-
if (__get_user(uval, uaddr))
69-
goto done;
71+
ret = __get_user(uval, uaddr);
72+
if (ret)
73+
goto fault;
7074

71-
if (uval == expected) {
72-
if (!__put_user(new, uaddr))
73-
regs->status32 |= STATUS_Z_MASK;
74-
}
75+
if (uval != expected)
76+
goto out;
7577

76-
done:
77-
preempt_enable();
78+
ret = __put_user(new, uaddr);
79+
if (ret)
80+
goto fault;
81+
82+
regs->status32 |= STATUS_Z_MASK;
7883

84+
out:
85+
preempt_enable();
7986
return uval;
87+
88+
fault:
89+
preempt_enable();
90+
91+
if (unlikely(ret != -EFAULT))
92+
goto fail;
93+
94+
down_read(&current->mm->mmap_sem);
95+
ret = fixup_user_fault(current, current->mm, (unsigned long) uaddr,
96+
FAULT_FLAG_WRITE, NULL);
97+
up_read(&current->mm->mmap_sem);
98+
99+
if (likely(!ret))
100+
goto again;
101+
102+
fail:
103+
force_sig(SIGSEGV, current);
104+
return ret;
80105
}
81106

82107
#ifdef CONFIG_ISA_ARCV2

0 commit comments

Comments
 (0)