Commit d5ed10bb authored by Linus Torvalds's avatar Linus Torvalds

Merge branch 'x86-uaccess-cleanup': x86 uaccess header cleanups

Merge my x86 uaccess updates branch.

The LAM ("Linear Address Masking") updates in this release made me
unhappy about how "access_ok()" was done, and it actually turned out to
have a couple of small bugs in it too.  This is my cleanup of the code:

 - use the sign bit of the __user pointer rather than masking the
   address and checking it against the TASK_SIZE range.

   We already did this part for the get/put_user() side, but
   'access_ok()' did the naïve "mask and range check" thing, which not
   only generates nasty code, but also ended up meaning that __access_ok
   itself didn't do a good job, and so copy_from_user_nmi() didn't get
   the check right.

 - move all the code that is 64-bit only into the 64-bit version of the
   header file, so that we don't unnecessarily pollute the shared x86
   code and make it look like LAM might work in 32-bit too.

 - fix a bug in the address masking (that doesn't end up mattering: in
   this case the fix was to just remove the buggy code entirely).

 - a couple of trivial cleanups and added commentary about the
   access_ok() rules.

* x86-uaccess-cleanup:
  x86-64: mm: clarify the 'positive addresses' user address rules
  x86: mm: remove 'sign' games from LAM untagged_addr*() macros
  x86: uaccess: move 32-bit and 64-bit parts into proper <asm/uaccess_N.h> header
  x86: mm: remove architecture-specific 'access_ok()' define
  x86-64: make access_ok() independent of LAM
parents 982365a8 798dec33
...@@ -16,88 +16,12 @@ ...@@ -16,88 +16,12 @@
#include <asm/extable.h> #include <asm/extable.h>
#include <asm/tlbflush.h> #include <asm/tlbflush.h>
#ifdef CONFIG_DEBUG_ATOMIC_SLEEP #ifdef CONFIG_X86_32
static inline bool pagefault_disabled(void); # include <asm/uaccess_32.h>
# define WARN_ON_IN_IRQ() \
WARN_ON_ONCE(!in_task() && !pagefault_disabled())
#else
# define WARN_ON_IN_IRQ()
#endif
#ifdef CONFIG_ADDRESS_MASKING
/*
* Mask out tag bits from the address.
*
* Magic with the 'sign' allows to untag userspace pointer without any branches
* while leaving kernel addresses intact.
*/
static inline unsigned long __untagged_addr(unsigned long addr)
{
long sign;
/*
* Refer tlbstate_untag_mask directly to avoid RIP-relative relocation
* in alternative instructions. The relocation gets wrong when gets
* copied to the target place.
*/
asm (ALTERNATIVE("",
"sar $63, %[sign]\n\t" /* user_ptr ? 0 : -1UL */
"or %%gs:tlbstate_untag_mask, %[sign]\n\t"
"and %[sign], %[addr]\n\t", X86_FEATURE_LAM)
: [addr] "+r" (addr), [sign] "=r" (sign)
: "m" (tlbstate_untag_mask), "[sign]" (addr));
return addr;
}
#define untagged_addr(addr) ({ \
unsigned long __addr = (__force unsigned long)(addr); \
(__force __typeof__(addr))__untagged_addr(__addr); \
})
static inline unsigned long __untagged_addr_remote(struct mm_struct *mm,
unsigned long addr)
{
long sign = addr >> 63;
mmap_assert_locked(mm);
addr &= (mm)->context.untag_mask | sign;
return addr;
}
#define untagged_addr_remote(mm, addr) ({ \
unsigned long __addr = (__force unsigned long)(addr); \
(__force __typeof__(addr))__untagged_addr_remote(mm, __addr); \
})
#else #else
#define untagged_addr(addr) (addr) # include <asm/uaccess_64.h>
#endif #endif
/**
* access_ok - Checks if a user space pointer is valid
* @addr: User space pointer to start of block to check
* @size: Size of block to check
*
* Context: User context only. This function may sleep if pagefaults are
* enabled.
*
* Checks if a pointer to a block of memory in user space is valid.
*
* Note that, depending on architecture, this function probably just
* checks that the pointer is in the user space range - after calling
* this function, memory access functions may still return -EFAULT.
*
* Return: true (nonzero) if the memory block may be valid, false (zero)
* if it is definitely invalid.
*/
#define access_ok(addr, size) \
({ \
WARN_ON_IN_IRQ(); \
likely(__access_ok(untagged_addr(addr), size)); \
})
#include <asm-generic/access_ok.h> #include <asm-generic/access_ok.h>
extern int __get_user_1(void); extern int __get_user_1(void);
...@@ -586,14 +510,6 @@ extern struct movsl_mask { ...@@ -586,14 +510,6 @@ extern struct movsl_mask {
#define ARCH_HAS_NOCACHE_UACCESS 1 #define ARCH_HAS_NOCACHE_UACCESS 1
#ifdef CONFIG_X86_32
unsigned long __must_check clear_user(void __user *mem, unsigned long len);
unsigned long __must_check __clear_user(void __user *mem, unsigned long len);
# include <asm/uaccess_32.h>
#else
# include <asm/uaccess_64.h>
#endif
/* /*
* The "unsafe" user accesses aren't really "unsafe", but the naming * The "unsafe" user accesses aren't really "unsafe", but the naming
* is a big fat warning: you have to not only do the access_ok() * is a big fat warning: you have to not only do the access_ok()
......
...@@ -33,4 +33,7 @@ __copy_from_user_inatomic_nocache(void *to, const void __user *from, ...@@ -33,4 +33,7 @@ __copy_from_user_inatomic_nocache(void *to, const void __user *from,
return __copy_from_user_ll_nocache_nozero(to, from, n); return __copy_from_user_ll_nocache_nozero(to, from, n);
} }
unsigned long __must_check clear_user(void __user *mem, unsigned long len);
unsigned long __must_check __clear_user(void __user *mem, unsigned long len);
#endif /* _ASM_X86_UACCESS_32_H */ #endif /* _ASM_X86_UACCESS_32_H */
...@@ -12,6 +12,87 @@ ...@@ -12,6 +12,87 @@
#include <asm/cpufeatures.h> #include <asm/cpufeatures.h>
#include <asm/page.h> #include <asm/page.h>
#ifdef CONFIG_ADDRESS_MASKING
/*
* Mask out tag bits from the address.
*/
static inline unsigned long __untagged_addr(unsigned long addr)
{
/*
* Refer tlbstate_untag_mask directly to avoid RIP-relative relocation
* in alternative instructions. The relocation gets wrong when gets
* copied to the target place.
*/
asm (ALTERNATIVE("",
"and %%gs:tlbstate_untag_mask, %[addr]\n\t", X86_FEATURE_LAM)
: [addr] "+r" (addr) : "m" (tlbstate_untag_mask));
return addr;
}
#define untagged_addr(addr) ({ \
unsigned long __addr = (__force unsigned long)(addr); \
(__force __typeof__(addr))__untagged_addr(__addr); \
})
static inline unsigned long __untagged_addr_remote(struct mm_struct *mm,
unsigned long addr)
{
mmap_assert_locked(mm);
return addr & (mm)->context.untag_mask;
}
#define untagged_addr_remote(mm, addr) ({ \
unsigned long __addr = (__force unsigned long)(addr); \
(__force __typeof__(addr))__untagged_addr_remote(mm, __addr); \
})
#endif
/*
* The virtual address space space is logically divided into a kernel
* half and a user half. When cast to a signed type, user pointers
* are positive and kernel pointers are negative.
*/
#define valid_user_address(x) ((long)(x) >= 0)
/*
* User pointers can have tag bits on x86-64. This scheme tolerates
* arbitrary values in those bits rather then masking them off.
*
* Enforce two rules:
* 1. 'ptr' must be in the user half of the address space
* 2. 'ptr+size' must not overflow into kernel addresses
*
* Note that addresses around the sign change are not valid addresses,
* and will GP-fault even with LAM enabled if the sign bit is set (see
* "CR3.LAM_SUP" that can narrow the canonicality check if we ever
* enable it, but not remove it entirely).
*
* So the "overflow into kernel addresses" does not imply some sudden
* exact boundary at the sign bit, and we can allow a lot of slop on the
* size check.
*
* In fact, we could probably remove the size check entirely, since
* any kernel accesses will be in increasing address order starting
* at 'ptr', and even if the end might be in kernel space, we'll
* hit the GP faults for non-canonical accesses before we ever get
* there.
*
* That's a separate optimization, for now just handle the small
* constant case.
*/
static inline bool __access_ok(const void __user *ptr, unsigned long size)
{
if (__builtin_constant_p(size <= PAGE_SIZE) && size <= PAGE_SIZE) {
return valid_user_address(ptr);
} else {
unsigned long sum = size + (unsigned long)ptr;
return valid_user_address(sum) && sum >= (unsigned long)ptr;
}
}
#define __access_ok __access_ok
/* /*
* Copy To/From Userspace * Copy To/From Userspace
*/ */
...@@ -106,7 +187,7 @@ static __always_inline __must_check unsigned long __clear_user(void __user *addr ...@@ -106,7 +187,7 @@ static __always_inline __must_check unsigned long __clear_user(void __user *addr
static __always_inline unsigned long clear_user(void __user *to, unsigned long n) static __always_inline unsigned long clear_user(void __user *to, unsigned long n)
{ {
if (access_ok(to, n)) if (__access_ok(to, n))
return __clear_user(to, n); return __clear_user(to, n);
return n; return n;
} }
......
...@@ -130,10 +130,36 @@ static bool ex_handler_fprestore(const struct exception_table_entry *fixup, ...@@ -130,10 +130,36 @@ static bool ex_handler_fprestore(const struct exception_table_entry *fixup,
return true; return true;
} }
/*
* On x86-64, we end up being imprecise with 'access_ok()', and allow
* non-canonical user addresses to make the range comparisons simpler,
* and to not have to worry about LAM being enabled.
*
* In fact, we allow up to one page of "slop" at the sign boundary,
* which means that we can do access_ok() by just checking the sign
* of the pointer for the common case of having a small access size.
*/
static bool gp_fault_address_ok(unsigned long fault_address)
{
#ifdef CONFIG_X86_64
/* Is it in the "user space" part of the non-canonical space? */
if (valid_user_address(fault_address))
return true;
/* .. or just above it? */
fault_address -= PAGE_SIZE;
if (valid_user_address(fault_address))
return true;
#endif
return false;
}
static bool ex_handler_uaccess(const struct exception_table_entry *fixup, static bool ex_handler_uaccess(const struct exception_table_entry *fixup,
struct pt_regs *regs, int trapnr) struct pt_regs *regs, int trapnr,
unsigned long fault_address)
{ {
WARN_ONCE(trapnr == X86_TRAP_GP, "General protection fault in user access. Non-canonical address?"); WARN_ONCE(trapnr == X86_TRAP_GP && !gp_fault_address_ok(fault_address),
"General protection fault in user access. Non-canonical address?");
return ex_handler_default(fixup, regs); return ex_handler_default(fixup, regs);
} }
...@@ -189,10 +215,12 @@ static bool ex_handler_imm_reg(const struct exception_table_entry *fixup, ...@@ -189,10 +215,12 @@ static bool ex_handler_imm_reg(const struct exception_table_entry *fixup,
} }
static bool ex_handler_ucopy_len(const struct exception_table_entry *fixup, static bool ex_handler_ucopy_len(const struct exception_table_entry *fixup,
struct pt_regs *regs, int trapnr, int reg, int imm) struct pt_regs *regs, int trapnr,
unsigned long fault_address,
int reg, int imm)
{ {
regs->cx = imm * regs->cx + *pt_regs_nr(regs, reg); regs->cx = imm * regs->cx + *pt_regs_nr(regs, reg);
return ex_handler_uaccess(fixup, regs, trapnr); return ex_handler_uaccess(fixup, regs, trapnr, fault_address);
} }
int ex_get_fixup_type(unsigned long ip) int ex_get_fixup_type(unsigned long ip)
...@@ -238,7 +266,7 @@ int fixup_exception(struct pt_regs *regs, int trapnr, unsigned long error_code, ...@@ -238,7 +266,7 @@ int fixup_exception(struct pt_regs *regs, int trapnr, unsigned long error_code,
case EX_TYPE_FAULT_MCE_SAFE: case EX_TYPE_FAULT_MCE_SAFE:
return ex_handler_fault(e, regs, trapnr); return ex_handler_fault(e, regs, trapnr);
case EX_TYPE_UACCESS: case EX_TYPE_UACCESS:
return ex_handler_uaccess(e, regs, trapnr); return ex_handler_uaccess(e, regs, trapnr, fault_addr);
case EX_TYPE_COPY: case EX_TYPE_COPY:
return ex_handler_copy(e, regs, trapnr); return ex_handler_copy(e, regs, trapnr);
case EX_TYPE_CLEAR_FS: case EX_TYPE_CLEAR_FS:
...@@ -269,7 +297,7 @@ int fixup_exception(struct pt_regs *regs, int trapnr, unsigned long error_code, ...@@ -269,7 +297,7 @@ int fixup_exception(struct pt_regs *regs, int trapnr, unsigned long error_code,
case EX_TYPE_FAULT_SGX: case EX_TYPE_FAULT_SGX:
return ex_handler_sgx(e, regs, trapnr); return ex_handler_sgx(e, regs, trapnr);
case EX_TYPE_UCOPY_LEN: case EX_TYPE_UCOPY_LEN:
return ex_handler_ucopy_len(e, regs, trapnr, reg, imm); return ex_handler_ucopy_len(e, regs, trapnr, fault_addr, reg, imm);
case EX_TYPE_ZEROPAD: case EX_TYPE_ZEROPAD:
return ex_handler_zeropad(e, regs, fault_addr); return ex_handler_zeropad(e, regs, fault_addr);
} }
......
...@@ -2970,6 +2970,8 @@ static int internal_get_user_pages_fast(unsigned long start, ...@@ -2970,6 +2970,8 @@ static int internal_get_user_pages_fast(unsigned long start,
len = nr_pages << PAGE_SHIFT; len = nr_pages << PAGE_SHIFT;
if (check_add_overflow(start, len, &end)) if (check_add_overflow(start, len, &end))
return 0; return 0;
if (end > TASK_SIZE_MAX)
return -EFAULT;
if (unlikely(!access_ok((void __user *)start, len))) if (unlikely(!access_ok((void __user *)start, len)))
return -EFAULT; return -EFAULT;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment