Re: [PATCH RFC] uaccess: user_access_begin_after_access_ok()

[Linus Torvalds]

On Tue, Jun 2, 2020 at 9:33 AM Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
>
> >
> > It's not clear whether we need a new API, I think __uaccess_being() has the
> > assumption that the address has been validated by access_ok().
>
> __uaccess_begin() is a stopgap, not a public API.

Correct. It's just an x86 implementation detail.

> The problem is real, but "let's add a public API that would do user_access_begin()
> with access_ok() already done" is no-go.

Yeah, it's completely pointless.

The solution to this is easy: remove the incorrect and useless early
"access_ok()". Boom, done.

Then use user_access_begin() and the appropriate unsage_get/put_user()
sequence, and user_access_end().

The range test that user-access-begin does is not just part of the
ABI, it's just required in general. We have almost thirty years of
history of trying to avoid it, AND IT WAS ALL BOGUS.

The fact is, the range check is pretty damn cheap, and not doing the
range check has always been a complete and utter disaster.

You have exactly two cases:

 (a) the access_ok() would be right above the code and can't be missed

 (b) not

and in (a) the solution is: remove the access_ok() and let
user_access_begin() do the range check.

In (b), the solution is literally "DON'T DO THAT!"

Because EVERY SINGLE TIME people have eventually noticed (possibly
after code movement) that "oops, we never did the access_ok at all,
and now we can be fooled into kernel corruption and a security issue".

And even if that didn't happen, the worry was there.

End result: use user_access_begin() and stop trying to remove the two
cycles or whatever of the limit checking cost. The "upside" of
removing that limit check just isn't. It's a downside.

                 Linus