RE: [PATCH RFC] uaccess: user_access_begin_after_access_ok()

From: David Laight
Date: Tue Jun 02 2020 - 16:41:45 EST

From: Michael S. Tsirkin
> Sent: 02 June 2020 21:33
> On Tue, Jun 02, 2020 at 10:18:09AM -0700, Linus Torvalds wrote:
> > 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.
> Hmm are you sure we can drop it? access_ok is done in the context
> of the process. Access itself in the context of a kernel thread
> that borrows the same mm. IIUC if the process can be 32 bit
> while the kernel is 64 bit, access_ok in the context of the
> kernel thread will not DTRT.

In which case you need a 'user_access_begin' that takes the mm
as an additional parameter.

I found an 'interesting' acccess_ok() call in the code that copies
iov[] into kernel (eg for readv()).

a) It is a long way from any copies.
b) It can be conditionally ignored - and is so for one call.
The oddball is code that reads from a different process.
I didn't spot an equivalent check, but it all worked by
mapping in the required page - so I'm not sure what happens.

Are there really just 2 limits for access_ok().
One for 64bit programs and one for 32bit?
With the limit being just below the 'dso' page??
So checking the current processes limit is never going
to restrict access.


Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)