Messages in this thread | | | From | David Laight <> | Subject | RE: [PATCH] Convert filldir[64]() from __put_user() to unsafe_put_user() | Date | Tue, 8 Oct 2019 09:58:24 +0000 |
| |
From: Linus Torvalds <torvalds@linux-foundation.org> > 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)
| |