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

[Thomas Weißschuh]

On 2025-08-18 22:21:06+0100, David Laight wrote:
> On Sun, 17 Aug 2025 14:49:43 +0100
> David Laight <david.laight.linux@xxxxxxxxx> wrote:

(...)

> 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().

To enforce some assignment, but not to the exact same variable as the argument,
you can wrap user_read_begin() in a function marked as __must_check.


#define __user_read_begin(uaddr, size, error_code) ({	\
	/* See above */
})

static __always_inline void __must_check __user *__user_read_check(void __user *val)
{
	return val;
}

#define user_read_begin(uaddr, size, error_code)	\
	((typeof(uaddr))__user_read_check(__user_read_begin(uaddr, size, error_code)))


Ignoring the return value gives:

error: ignoring return value of ‘__user_read_check’ declared with attribute ‘warn_unused_result’ [-Werror=unused-result]
 1629 |         ((typeof(uaddr))__user_read_check(__user_read_begin(uaddr, size, error_code)))
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
note: in expansion of macro ‘user_read_begin’
 1635 |         user_read_begin(uaddr, sizeof (*uaddr), return -EFAULT);
      |         ^~~~~~~~~~~~~~~


Thomas