[patch V3 08/12] uaccess: Provide put/get_user_masked()

Thomas Gleixner posted 12 patches 2 months ago
[patch V3 08/12] uaccess: Provide put/get_user_masked()
Posted by Thomas Gleixner 2 months ago
Provide conveniance wrappers around scoped masked user access similiar to
put/get_user(), which reduce the usage sites to:

       if (!get_user_masked(val, ptr))
       		return -EFAULT;

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 include/linux/uaccess.h |   44 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

--- a/include/linux/uaccess.h
+++ b/include/linux/uaccess.h
@@ -830,6 +830,50 @@ for (bool ____stop = false; !____stop; _
 #define scoped_masked_user_rw_access(_uptr, _elbl)				\
 	scoped_masked_user_rw_access_size((_uptr), sizeof(*(_uptr)), _elbl)
 
+/**
+ * get_user_masked - Read user data with masked access
+ * @_val:	The variable to store the value read from user memory
+ * @_usrc:	Pointer to the user space memory to read from
+ *
+ * Return: true if successful, false when faulted
+ */
+#define get_user_masked(_val, _usrc)				\
+({								\
+	__label__ efault;					\
+	typeof((_usrc)) _tmpsrc	= (_usrc);			\
+	bool ____ret = true;					\
+								\
+	scoped_masked_user_read_access(_tmpsrc, efault)		\
+		unsafe_get_user(_val, _tmpsrc, efault);		\
+	if (0) {						\
+	efault:							\
+		____ret = false;				\
+	}							\
+	____ret;						\
+})
+
+/**
+ * put_user_masked - Write to user memory with masked access
+ * @_val:	The value to write
+ * @_udst:	Pointer to the user space memory to write to
+ *
+ * Return: true if successful, false when faulted
+ */
+#define put_user_masked(_val, _udst)				\
+({								\
+	__label__ efault;					\
+	typeof((_udst)) _tmpdst	= (_udst);			\
+	bool ____ret = true;					\
+								\
+	scoped_masked_user_write_access(_tmpdst, efault)	\
+		unsafe_put_user(_val, _tmpdst, efault);		\
+	if (0) {						\
+	efault:							\
+		____ret = false;				\
+	}							\
+	____ret;						\
+})
+
 #ifdef CONFIG_HARDENED_USERCOPY
 void __noreturn usercopy_abort(const char *name, const char *detail,
 			       bool to_user, unsigned long offset,
Re: [patch V3 08/12] uaccess: Provide put/get_user_masked()
Posted by Mathieu Desnoyers 2 months ago
On 2025-10-17 06:09, Thomas Gleixner wrote:
> Provide conveniance wrappers around scoped masked user access similiar to

convenience
similar

> +/**
> + * get_user_masked - Read user data with masked access
> + * @_val:	The variable to store the value read from user memory
> + * @_usrc:	Pointer to the user space memory to read from
> + *
> + * Return: true if successful, false when faulted

^ '.' (or not) across sentences. Your choice, but it's irregular across
the series.

> + */
> +#define get_user_masked(_val, _usrc)				\
> +({								\
> +	__label__ efault;					\
> +	typeof((_usrc)) _tmpsrc	= (_usrc);			\

Remove extra () around _usrc in typeof.

UNIQUE_ID for _tmpsrc ?

> +	bool ____ret = true;					\

Why so many underscores ? It there are nesting concerns,
it may be a sign that UNIQUE_ID is needed.


> + */
> +#define put_user_masked(_val, _udst)				\
> +({								\
> +	__label__ efault;					\
> +	typeof((_udst)) _tmpdst	= (_udst);			\

Remove extra () around _udst in typeof.

UNIQUE_ID for _tmpsrc ?

> +	bool ____ret = true;	

UNIQUE_ID for ____ret ?

Thanks,

Mathieu

-- 
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com
Re: [patch V3 08/12] uaccess: Provide put/get_user_masked()
Posted by Mathieu Desnoyers 2 months ago
On 2025-10-17 09:41, Mathieu Desnoyers wrote:
[...]
>> +/**
>> + * get_user_masked - Read user data with masked access
[...]
>> + * Return: true if successful, false when faulted

I notice that __get_user() and get_user() return -EFAULT
on error, 0 on success. I suspect the reversed logic
will be rather confusing.

Thanks,

Mathieu

-- 
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com
Re: [patch V3 08/12] uaccess: Provide put/get_user_masked()
Posted by Thomas Gleixner 1 month, 4 weeks ago
On Fri, Oct 17 2025 at 09:45, Mathieu Desnoyers wrote:
> On 2025-10-17 09:41, Mathieu Desnoyers wrote:
> [...]
>>> +/**
>>> + * get_user_masked - Read user data with masked access
> [...]
>>> + * Return: true if successful, false when faulted
>
> I notice that __get_user() and get_user() return -EFAULT
> on error, 0 on success. I suspect the reversed logic
> will be rather confusing.

In most cases the return value of those is treated as a boolean
success/fail indicator. It's pretty confusing when a boolean return
value indicates success with 'false'. It's way more intuitive to read:

       if (!do_something())
       		goto fail;

Just because we have this historic return '0' on success and error code
on fail concept does not mean we have to proliferate it forever.

In fact a lot of that 'hand an error code through the callchain' is just
pointless as in many cases that error code is completely ignored and
just used for a boolean decision at the end. So there is no point to
pretend that those error codes are meaningful except for places where
they need to be returned to user space.

Thanks,

        tglx