On Fri, Nov 02, 2018 at 09:14:51AM -0700, Linus Torvalds wrote: > On Fri, Nov 2, 2018 at 6:04 AM Michael S. Tsirkin <mst@xxxxxxxxxx> wrote: > > > > I've tried making access_ok mask the parameter it gets. > > PLEASE don't do this. Okay. > Just use "copy_to/from_user()". Just for completeness I'd like to point out for vhost the copies are done from the kernel thread. So yes we can switch to copy_to/from_user but for e.g. 32-bit userspace running on top of a 64 bit kernel it is IIUC not sufficient - we must *also* do access_ok checks on control path when addresses are passed to the kernel and when current points to the correct task struct. > We have had lots of bugs because code bitrots. Yes, I wish we did not need these access_ok checks and could just rely on copy_to/from_user. > And no, the access_ok() checks aren't expensive, not even in a loop. > They *used* to be somewhat expensive compared to the access, but that > simply isn't true any more. The real expense in copy_to_user and > friends are in the user access bit setting (STAC and CLAC on x86), > which easily an order of magnitude more expensive than access_ok(). > > So just get rid of the double-underscore version. It's basically > always a mis-optimization due to entirely historical reasons. I can > pretty much guarantee that it's not visible in profiles. > > Linus OK. So maybe we should focus on switching to user_access_begin/end + unsafe_get_user/unsafe_put_user in a loop which does seem to be measureable. That moves the barrier out of the loop, which seems to be consistent with what you would expect. -- MST
