Re: [PATCH 1/2] uaccess: Simplify code pattern for masked user copies

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

On Sun, 9 Feb 2025 09:40:05 -0800
Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Sun, 9 Feb 2025 at 02:56, David Laight <david.laight.linux@gmail.com> wrote:
> >
> > Code can then be changed:
> > -               if (!user_read_access_begin(from, sizeof(*from)))
> > +               if (!masked_user_read_access_begin(&from, sizeof(*from)))
> >                         return -EFAULT;  
> 
> I really dislike the use of "pass pointer to simple variable you are
> going to change" interfaces which is why I didn't do it this way.

I'm not sure the 'goto' model works here.
The issue is that the calling code mustn't use the unmasked address.
You really want to make that as hard as possible.
So the 'function' really does need to do an in-situ update.

I did do a test compile without the &, it exploded but I didn't
check whether it always would.
IIRC there is a sparse check for 'user' pointers that would help.

Even with the current functions, someone is bound to write:
	if (!masked_user_access_begin(uaddr))
		return -EFAULT;
	unsafe_get_user(kaddr, uaddr, label);
and it will all appear to be fine...
(objtool might detect something because of the NULL pointer path.)

You almost need it to be 'void masked_user_access_begin(&uaddr)'.

	David