Skip to content

Commit

Permalink
powerpc/uaccess: Evaluate macro arguments once, before user access is…
Browse files Browse the repository at this point in the history
… allowed

commit d02f6b7 upstream.

get/put_user() can be called with nontrivial arguments. fs/proc/page.c
has a good example:

    if (put_user(stable_page_flags(ppage), out)) {

stable_page_flags() is quite a lot of code, including spin locks in
the page allocator.

Ensure these arguments are evaluated before user access is allowed.

This improves security by reducing code with access to userspace, but
it also fixes a PREEMPT bug with KUAP on powerpc/64s:
stable_page_flags() is currently called with AMR set to allow writes,
it ends up calling spin_unlock(), which can call preempt_schedule. But
the task switch code can not be called with AMR set (it relies on
interrupts saving the register), so this blows up.

It's fine if the code inside allow_user_access() is preemptible,
because a timer or IPI will save the AMR, but it's not okay to
explicitly cause a reschedule.

Fixes: de78a9c ("powerpc: Add a framework for Kernel Userspace Access Protection")
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/20200407041245.600651-1-npiggin@gmail.com
Signed-off-by: Daniel Axtens <dja@axtens.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
Nicholas Piggin authored and Greg Kroah-Hartman committed Nov 22, 2020
1 parent 92e7ec2 commit 6b36099
Showing 1 changed file with 35 additions and 14 deletions.
49 changes: 35 additions & 14 deletions arch/powerpc/include/asm/uaccess.h
Original file line number Diff line number Diff line change
Expand Up @@ -167,32 +167,44 @@ do { \
({ \
long __pu_err; \
__typeof__(*(ptr)) __user *__pu_addr = (ptr); \
__typeof__(*(ptr)) __pu_val = (x); \
__typeof__(size) __pu_size = (size); \
\
if (!is_kernel_addr((unsigned long)__pu_addr)) \
might_fault(); \
__chk_user_ptr(ptr); \
__chk_user_ptr(__pu_addr); \
if (do_allow) \
__put_user_size((x), __pu_addr, (size), __pu_err); \
__put_user_size(__pu_val, __pu_addr, __pu_size, __pu_err); \
else \
__put_user_size_allowed((x), __pu_addr, (size), __pu_err); \
__put_user_size_allowed(__pu_val, __pu_addr, __pu_size, __pu_err); \
\
__pu_err; \
})

#define __put_user_check(x, ptr, size) \
({ \
long __pu_err = -EFAULT; \
__typeof__(*(ptr)) __user *__pu_addr = (ptr); \
__typeof__(*(ptr)) __pu_val = (x); \
__typeof__(size) __pu_size = (size); \
\
might_fault(); \
if (access_ok(VERIFY_WRITE, __pu_addr, size)) \
__put_user_size((x), __pu_addr, (size), __pu_err); \
if (access_ok(VERIFY_WRITE, __pu_addr, __pu_size)) \
__put_user_size(__pu_val, __pu_addr, __pu_size, __pu_err); \
\
__pu_err; \
})

#define __put_user_nosleep(x, ptr, size) \
({ \
long __pu_err; \
__typeof__(*(ptr)) __user *__pu_addr = (ptr); \
__chk_user_ptr(ptr); \
__put_user_size((x), __pu_addr, (size), __pu_err); \
__typeof__(*(ptr)) __pu_val = (x); \
__typeof__(size) __pu_size = (size); \
\
__chk_user_ptr(__pu_addr); \
__put_user_size(__pu_val, __pu_addr, __pu_size, __pu_err); \
\
__pu_err; \
})

Expand Down Expand Up @@ -284,15 +296,18 @@ do { \
long __gu_err; \
__long_type(*(ptr)) __gu_val; \
__typeof__(*(ptr)) __user *__gu_addr = (ptr); \
__chk_user_ptr(ptr); \
__typeof__(size) __gu_size = (size); \
\
__chk_user_ptr(__gu_addr); \
if (!is_kernel_addr((unsigned long)__gu_addr)) \
might_fault(); \
barrier_nospec(); \
if (do_allow) \
__get_user_size(__gu_val, __gu_addr, (size), __gu_err); \
__get_user_size(__gu_val, __gu_addr, __gu_size, __gu_err); \
else \
__get_user_size_allowed(__gu_val, __gu_addr, (size), __gu_err); \
__get_user_size_allowed(__gu_val, __gu_addr, __gu_size, __gu_err); \
(x) = (__typeof__(*(ptr)))__gu_val; \
\
__gu_err; \
})

Expand All @@ -301,12 +316,15 @@ do { \
long __gu_err = -EFAULT; \
__long_type(*(ptr)) __gu_val = 0; \
__typeof__(*(ptr)) __user *__gu_addr = (ptr); \
__typeof__(size) __gu_size = (size); \
\
might_fault(); \
if (access_ok(VERIFY_READ, __gu_addr, (size))) { \
if (access_ok(VERIFY_READ, __gu_addr, __gu_size)) { \
barrier_nospec(); \
__get_user_size(__gu_val, __gu_addr, (size), __gu_err); \
__get_user_size(__gu_val, __gu_addr, __gu_size, __gu_err); \
} \
(x) = (__force __typeof__(*(ptr)))__gu_val; \
\
__gu_err; \
})

Expand All @@ -315,10 +333,13 @@ do { \
long __gu_err; \
__long_type(*(ptr)) __gu_val; \
__typeof__(*(ptr)) __user *__gu_addr = (ptr); \
__chk_user_ptr(ptr); \
__typeof__(size) __gu_size = (size); \
\
__chk_user_ptr(__gu_addr); \
barrier_nospec(); \
__get_user_size(__gu_val, __gu_addr, (size), __gu_err); \
__get_user_size(__gu_val, __gu_addr, __gu_size, __gu_err); \
(x) = (__force __typeof__(*(ptr)))__gu_val; \
\
__gu_err; \
})

Expand Down

0 comments on commit 6b36099

Please sign in to comment.