Christophe Leroy's on March 27, 2020 5:21 pm:
>
>
> Le 27/03/2020 à 08:02, Nicholas Piggin a écrit :
>> 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.
>>
>> Signed-off-by: Nicholas Piggin <npig...@gmail.com>
>> ---
>> arch/powerpc/include/asm/uaccess.h | 97 ++++++++++++++++++------------
>> 1 file changed, 59 insertions(+), 38 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/uaccess.h
>> b/arch/powerpc/include/asm/uaccess.h
>> index 670910df3cc7..1cf8595aeef1 100644
>> --- a/arch/powerpc/include/asm/uaccess.h
>> +++ b/arch/powerpc/include/asm/uaccess.h
>> @@ -162,36 +162,48 @@ do {
>> \
>> prevent_write_to_user(ptr, size); \
>> } while (0)
>>
>> -#define __put_user_nocheck(x, ptr, size, do_allow) \
>> +#define __put_user_nocheck(x, ptr, size, do_allow) \
>
> No need to touch this line. Anyway at the end, you still have several \
> which are not aligned.
>
>> ({ \
>> 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); \
>> - if (do_allow)
>> \
>
> No need to touch that line
>
>> - __put_user_size((x), __pu_addr, (size), __pu_err);
>> \
>> - else
>> \
>
> No need to touch that line
Okay fair point I can redo the patch.
Thanks,
Nick
