| From d02f6b7dab8228487268298ea1f21081c0b4b3eb Mon Sep 17 00:00:00 2001 |
| From: Nicholas Piggin <npiggin@gmail.com> |
| Date: Tue, 7 Apr 2020 14:12:45 +1000 |
| Subject: powerpc/uaccess: Evaluate macro arguments once, before user access is allowed |
| |
| From: Nicholas Piggin <npiggin@gmail.com> |
| |
| commit d02f6b7dab8228487268298ea1f21081c0b4b3eb 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: de78a9c42a79 ("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: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| arch/powerpc/include/asm/uaccess.h | 49 ++++++++++++++++++++++++++----------- |
| 1 file changed, 35 insertions(+), 14 deletions(-) |
| |
| --- a/arch/powerpc/include/asm/uaccess.h |
| +++ b/arch/powerpc/include/asm/uaccess.h |
| @@ -166,13 +166,17 @@ 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; \ |
| }) |
| |
| @@ -180,9 +184,13 @@ do { \ |
| ({ \ |
| 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(__pu_addr, size)) \ |
| - __put_user_size((x), __pu_addr, (size), __pu_err); \ |
| + if (access_ok(__pu_addr, __pu_size)) \ |
| + __put_user_size(__pu_val, __pu_addr, __pu_size, __pu_err); \ |
| + \ |
| __pu_err; \ |
| }) |
| |
| @@ -190,8 +198,12 @@ do { \ |
| ({ \ |
| 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; \ |
| }) |
| |
| @@ -283,15 +295,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; \ |
| }) |
| |
| @@ -300,12 +315,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(__gu_addr, (size))) { \ |
| + if (access_ok(__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; \ |
| }) |
| |
| @@ -314,10 +332,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; \ |
| }) |
| |