From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Michael S. Tsirkin" Subject: Re: [PULL] vhost: cleanups and fixes Date: Fri, 2 Nov 2018 12:59:40 -0400 Message-ID: <20181102122937-mutt-send-email-mst@kernel.org> References: <20181101171938-mutt-send-email-mst@kernel.org> <20181102114635.hi3q53kzmz4qljsf@lakrids.cambridge.arm.com> <20181102083018-mutt-send-email-mst@kernel.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: mark.rutland@arm.com, Kees Cook , kvm@vger.kernel.org, virtualization@lists.linux-foundation.org, netdev@vger.kernel.org, Linux Kernel Mailing List , Andrew Morton , bijan.mottahedeh@oracle.com, gedwards@ddn.com, joe@perches.com, lenaic@lhuard.fr, liang.z.li@intel.com, mhocko@kernel.org, mhocko@suse.com, stefanha@redhat.com, wei.w.wang@intel.com, Jason Wang To: Linus Torvalds Return-path: Content-Disposition: inline In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org List-Id: netdev.vger.kernel.org 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 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