Messages in this thread | | | From | Andy Lutomirski <> | Date | Wed, 19 Aug 2020 09:39:10 -0700 | Subject | Re: [PATCH] x86/uaccess: Use pointer masking to limit uaccess speculation |
| |
On Wed, Aug 19, 2020 at 7:50 AM Josh Poimboeuf <jpoimboe@redhat.com> wrote: > +/* > + * Sanitize a uaccess pointer such that it becomes NULL if it's not a valid > + * user pointer. This blocks speculative dereferences of user-controlled > + * pointers. > + */ > +#define uaccess_mask_ptr(ptr) \ > + (__typeof__(ptr)) array_index_nospec((__force unsigned long)ptr, user_addr_max()) > +
If I dug through all the macros correctly, this is generating a fairly complex pile of math to account for the fact that user_addr_max() is variable and that it's a nasty number.
But I don't think there's any particular need to use the real maximum user address here. Allowing a mis-speculated user access to a non-canonical address or to the top guard page of the lower canonical region is harmless. With current kernels, a sequence like:
if (likely((long)addr > 0) { masked_addr = addr & 0x7FFFFFFFFFFFFFFFUL; } else { if (kernel fs) { masked_addr = addr; } else { EFAULT; } }
could plausibly be better. But Christoph's series fixes this whole mess, and I think that this should be:
#define uaccess_mask_ptr(ptr) ((__typeof___(ptr)) (__force unsigned long)ptr & USER_ADDR_MASK))
where USER_ADDR_MASK is the appropriate value for 32-bit or 64-bit.
| |