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

[Andy Lutomirski]

On Tue, May 4, 2021 at 8:55 PM Josh Poimboeuf <jpoimboe@xxxxxxxxxx> wrote:
> +/*
> + * 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);                                           \
> +})

Is there an equally efficient sequence that squishes the pointer value
to something noncanonical or something like -1 instead of 0?  I'm not
sure this matters, but it opens up the possibility of combining the
access_ok check with the masking without any branches at all.

Also, why are you doing mask &= _ptr; mask instead of just
((typeof(ptr)) (_ptr & mask))?  or _ptr &= mask, for that matter?