Skip to content

Commit

Permalink
arm64: Make USER_DS an inclusive limit
Browse files Browse the repository at this point in the history
Currently, USER_DS represents an exclusive limit while KERNEL_DS is
inclusive. In order to do some clever trickery for speculation-safe
masking, we need them both to behave equivalently - there aren't enough
bits to make KERNEL_DS exclusive, so we have precisely one option. This
also happens to correct a longstanding false negative for a range
ending on the very top byte of kernel memory.

Mark Rutland points out that we've actually got the semantics of
addresses vs. segments muddled up in most of the places we need to
amend, so shuffle the {USER,KERNEL}_DS definitions around such that we
can correct those properly instead of just pasting "-1"s everywhere.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
  • Loading branch information
Robin Murphy authored and Catalin Marinas committed Feb 6, 2018
1 parent 022620e commit 51369e3
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 23 deletions.
3 changes: 3 additions & 0 deletions arch/arm64/include/asm/processor.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@

#define TASK_SIZE_64 (UL(1) << VA_BITS)

#define KERNEL_DS UL(-1)
#define USER_DS (TASK_SIZE_64 - 1)

#ifndef __ASSEMBLY__

/*
Expand Down
45 changes: 26 additions & 19 deletions arch/arm64/include/asm/uaccess.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,7 @@
#include <asm/compiler.h>
#include <asm/extable.h>

#define KERNEL_DS (-1UL)
#define get_ds() (KERNEL_DS)

#define USER_DS TASK_SIZE_64
#define get_fs() (current_thread_info()->addr_limit)

static inline void set_fs(mm_segment_t fs)
Expand Down Expand Up @@ -66,22 +63,32 @@ static inline void set_fs(mm_segment_t fs)
* Returns 1 if the range is valid, 0 otherwise.
*
* This is equivalent to the following test:
* (u65)addr + (u65)size <= current->addr_limit
*
* This needs 65-bit arithmetic.
* (u65)addr + (u65)size <= (u65)current->addr_limit + 1
*/
#define __range_ok(addr, size) \
({ \
unsigned long __addr = (unsigned long)(addr); \
unsigned long flag, roksum; \
__chk_user_ptr(addr); \
asm("adds %1, %1, %3; ccmp %1, %4, #2, cc; cset %0, ls" \
: "=&r" (flag), "=&r" (roksum) \
: "1" (__addr), "Ir" (size), \
"r" (current_thread_info()->addr_limit) \
: "cc"); \
flag; \
})
static inline unsigned long __range_ok(unsigned long addr, unsigned long size)
{
unsigned long limit = current_thread_info()->addr_limit;

__chk_user_ptr(addr);
asm volatile(
// A + B <= C + 1 for all A,B,C, in four easy steps:
// 1: X = A + B; X' = X % 2^64
" adds %0, %0, %2\n"
// 2: Set C = 0 if X > 2^64, to guarantee X' > C in step 4
" csel %1, xzr, %1, hi\n"
// 3: Set X' = ~0 if X >= 2^64. For X == 2^64, this decrements X'
// to compensate for the carry flag being set in step 4. For
// X > 2^64, X' merely has to remain nonzero, which it does.
" csinv %0, %0, xzr, cc\n"
// 4: For X < 2^64, this gives us X' - C - 1 <= 0, where the -1
// comes from the carry in being clear. Otherwise, we are
// testing X' - C == 0, subject to the previous adjustments.
" sbcs xzr, %0, %1\n"
" cset %0, ls\n"
: "+r" (addr), "+r" (limit) : "Ir" (size) : "cc");

return addr;
}

/*
* When dealing with data aborts, watchpoints, or instruction traps we may end
Expand All @@ -90,7 +97,7 @@ static inline void set_fs(mm_segment_t fs)
*/
#define untagged_addr(addr) sign_extend64(addr, 55)

#define access_ok(type, addr, size) __range_ok(addr, size)
#define access_ok(type, addr, size) __range_ok((unsigned long)(addr), size)
#define user_addr_max get_fs

#define _ASM_EXTABLE(from, to) \
Expand Down
4 changes: 2 additions & 2 deletions arch/arm64/kernel/entry.S
Original file line number Diff line number Diff line change
Expand Up @@ -167,10 +167,10 @@ alternative_else_nop_endif
.else
add x21, sp, #S_FRAME_SIZE
get_thread_info tsk
/* Save the task's original addr_limit and set USER_DS (TASK_SIZE_64) */
/* Save the task's original addr_limit and set USER_DS */
ldr x20, [tsk, #TSK_TI_ADDR_LIMIT]
str x20, [sp, #S_ORIG_ADDR_LIMIT]
mov x20, #TASK_SIZE_64
mov x20, #USER_DS
str x20, [tsk, #TSK_TI_ADDR_LIMIT]
/* No need to reset PSTATE.UAO, hardware's already set it to 0 for us */
.endif /* \el == 0 */
Expand Down
4 changes: 2 additions & 2 deletions arch/arm64/mm/fault.c
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ static inline bool is_permission_fault(unsigned int esr, struct pt_regs *regs,
if (fsc_type == ESR_ELx_FSC_PERM)
return true;

if (addr < USER_DS && system_uses_ttbr0_pan())
if (addr < TASK_SIZE && system_uses_ttbr0_pan())
return fsc_type == ESR_ELx_FSC_FAULT &&
(regs->pstate & PSR_PAN_BIT);

Expand Down Expand Up @@ -414,7 +414,7 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
mm_flags |= FAULT_FLAG_WRITE;
}

if (addr < USER_DS && is_permission_fault(esr, regs, addr)) {
if (addr < TASK_SIZE && is_permission_fault(esr, regs, addr)) {
/* regs->orig_addr_limit may be 0 if we entered from EL0 */
if (regs->orig_addr_limit == KERNEL_DS)
die("Accessing user space memory with fs=KERNEL_DS", regs, esr);
Expand Down

0 comments on commit 51369e3

Please sign in to comment.