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
