Messages in this thread | | | From | David Laight <> | Subject | RE: [PATCH v4 3/4] x86/uaccess: Use pointer masking to limit uaccess speculation | Date | Wed, 5 May 2021 08:48:48 +0000 |
| |
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)
| |