RE: [PATCH v4 3/4] x86/uaccess: Use pointer masking to limit uaccess speculation

David Laight
2021-05-05T08:48:58Z

From: Josh Poimboeuf
> Sent: 05 May 2021 04:55
>
> The x86 uaccess code uses barrier_nospec() in various places to prevent
> speculative dereferencing of user-controlled pointers (which might be
> combined with further gadgets or CPU bugs to leak data).
...
> Remove existing barrier_nospec() usage, and instead do user pointer
> masking, throughout the x86 uaccess code. This is similar to what arm64
> is already doing with uaccess_mask_ptr().
...
> diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
> index fb75657b5e56..ebe9ab46b183 100644
> --- a/arch/x86/include/asm/uaccess.h
> +++ b/arch/x86/include/asm/uaccess.h
> @@ -66,12 +66,35 @@ static inline bool pagefault_disabled(void);
> * Return: true (nonzero) if the memory block may be valid, false (zero)
> * if it is definitely invalid.
> */
> -#define access_ok(addr, size) \
> +#define access_ok(addr, size) \
> ({ \
> WARN_ON_IN_IRQ(); \
> likely(!__range_not_ok(addr, size, TASK_SIZE_MAX)); \
> })
>
> +/*
> + * Sanitize a user pointer such that it becomes NULL if it's not a valid user
> + * pointer. This prevents speculatively dereferencing a user-controlled
> + * pointer to kernel space if access_ok() speculatively returns true. This
> + * should be done *after* access_ok(), to avoid affecting error handling
> + * behavior.
> + */
> +#define mask_user_ptr(ptr) \
> +({ \
> + unsigned long _ptr = (__force unsigned long)ptr; \
> + unsigned long mask; \
> + \
> + asm volatile("cmp %[max], %[_ptr]\n\t" \
> + "sbb %[mask], %[mask]\n\t" \
> + : [mask] "=r" (mask) \
> + : [_ptr] "r" (_ptr), \
> + [max] "r" (TASK_SIZE_MAX) \
> + : "cc"); \
> + \
> + mask &= _ptr; \
> + ((typeof(ptr)) mask); \
> +})
> +

access_ok() and mask_user_ptr() are doing much the same check.
Is there scope for making access_ok() return the masked pointer?

So the canonical calling code would be:
uptr = access_ok(uptr, size);
if (!uptr)
return -EFAULT;

This would error requests for address 0 earlier - but I don't
believe they are ever valid in Linux.
(Some historic x86 a.out formats did load to address 0.)

Clearly for a follow up patch.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)