Re: [patch 0/4] uaccess: Provide and use helpers for user masked access

[David Laight <david.laight.linux@gmail.com>]

On Sun, 17 Aug 2025 14:49:43 +0100
David Laight <david.laight.linux@gmail.com> wrote:

> On Wed, 13 Aug 2025 17:57:00 +0200 (CEST)
> Thomas Gleixner <tglx@linutronix.de> wrote:
> 
> > commit 2865baf54077 ("x86: support user address masking instead of
> > non-speculative conditional") provided an optimization for
> > unsafe_get/put_user(), which optimizes the Spectre-V1 mitigation in an
> > architecture specific way. Currently only x86_64 supports that.
> > 
> > The required code pattern screams for helper functions before it is copied
> > all over the kernel. So far the exposure is limited to futex, x86 and
> > fs/select.
> > 
> > Provide a set of helpers for common single size access patterns:  
> 
> (gmail hasn't decided to accept 1/4 yet - I need to find a better
> mail relay...)
> 
> +/*
> + * Conveniance macros to avoid spreading this pattern all over the place
>     ^ spelling...
> + */
> +#define user_read_masked_begin(src) ({					\
> +	bool __ret = true;						\
> +									\
> +	if (can_do_masked_user_access())				\
> +		src = masked_user_access_begin(src);			\
> +	else if (!user_read_access_begin(src, sizeof(*src)))		\
> +		__ret = false;						\
> +	__ret;								\
> +})

Would something like this work (to avoid the hidden update)?

#define user_read_begin(uaddr, size, error_code) ({	\
	typeof(uaddr) __uaddr;				\
	if (can_do_masked_user_access())		\
		__uaddr = masked_user_access_begin(uaddr);\
	else if (user_read_access_begin(uaddr, size))	\
		__uaddr = uaddr;			\
	else {						\
		error_code;				\
	}						\
	__uaddr;					\
})

With typical use being either:
	uaddr = user_read_begin(uaddr, sizeof (*uaddr), return -EFAULT);
or:
	uaddr = user_read_begin(uaddr, sizeof (*uaddr), goto bad_uaddr);

One problem is I don't think you can easily enforce the assignment.
Ideally you'd want something that made the compiler think that 'uaddr' was unset.
It could be done for in a debug/diagnostic compile by adding 'uaddr = NULL'
at the bottom of the #define and COMPILE_ASSERT(!staticically_true(uaddr == NULL))
inside unsafe_get/put_user().

	David