From: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
> Sent: 07 October 2019 19:11
...
> I've been very close to just removing __get_user/__put_user several
> times, exactly because people do completely the wrong thing with them
> - not speeding code up, but making it unsafe and buggy.
They could do the very simple check that 'user_ptr+size < kernel_base'
rather than the full window check under the assumption that access_ok()
has been called and that the likely errors are just overruns.
> The new "user_access_begin/end()" model is much better, but it also
> has actual STATIC checking that there are no function calls etc inside
> the region, so it forces you to do the loop properly and tightly, and
> not the incorrect "I checked the range somewhere else, now I'm doing
> an unsafe copy".
>
> And it actually speeds things up, unlike the access_ok() games.
I've code that does:
if (!access_ok(...))
return -EFAULT;
...
for (...) {
if (__get_user(tmp_u64, user_ptr++))
return -EFAULT;
writeq(tmp_u64, io_ptr++);
}
(Although the code is more complex because not all transfers are multiples of 8 bytes.)
With user_access_begin/end() I'd probably want to put the copy loop
inside a function (which will probably get inlined) to avoid convoluted
error processing.
So you end up with:
if (!user_access_ok())
return _EFAULT;
user_access_begin();
rval = do_copy_code(...);
user_access_end();
return rval;
Which, at the source level (at least) breaks your 'no function calls' rule.
The writeq() might also break it as well.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
