From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jesse Barnes Subject: Re: [PATCH] drm/i915: Introduce mapping of user pages into video memory (userptr) ioctl Date: Wed, 5 Feb 2014 15:55:04 +0000 Message-ID: <20140205155504.75ba32fb@jbarnes-t420> References: <1390905261-5410-4-git-send-email-chris@chris-wilson.co.uk> <1390915006-16007-1-git-send-email-chris@chris-wilson.co.uk> <52EFB5A5.3080502@linux.intel.com> <20140204105647.GG17001@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from alt-proxy64.mail.unifiedlayer.com (alt-proxy64.mail.unifiedlayer.com [173.254.70.10]) by gabe.freedesktop.org (Postfix) with SMTP id 4029241CB for ; Wed, 5 Feb 2014 08:02:14 -0800 (PST) In-Reply-To: <20140204105647.GG17001@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org To: Daniel Vetter Cc: intel-gfx , Akash Goel List-Id: intel-gfx@lists.freedesktop.org On Tue, 4 Feb 2014 11:56:47 +0100 Daniel Vetter wrote: > On Mon, Feb 03, 2014 at 03:28:37PM +0000, Tvrtko Ursulin wrote: > > > > On 01/29/2014 08:34 PM, Daniel Vetter wrote: > > >Actually I've found something else to complain about: > > > > > >On Tue, Jan 28, 2014 at 2:16 PM, Chris Wilson wrote: > > >>+#define I915_USERPTR_READ_ONLY 0x1 > > > > > >This smells like an insta-root-exploit: > > >1. mmap /lib/ld-linux.so as read-only > > >2. userptr bind that mmap'ed area as READ_ONLY > > >3. blit exploit code over it > > >4. profit > > > > > >I also don't see a way we could fix this, at least without the > > >hardware providing read-only modes in the ptes. Which also requires us > > >to actually trust it to follow them, even when they exists ... > > > > Would disallowing mapping of shared pages help and be acceptable > > considering intended use cases? > > The above exploit is the simplest one I could come up with, but I expect > the vm in general won't be too happy if we write to pages it never expects > are written to. We could do fun stuff like corrupt pagecache or swap > cache. Which in conjunction with stable kernel pages (which some I/O paths > needed) is rather likely to result in havoc. > > Essentially I'm no vm expert, and this definitely needs a full vm audit > even before considering it at all. So I'd like to drop support for it in > the initial version ... Yeah I think we'd need to only allow this usage for root (i.e. you get to keep both pieces) or for platforms where we actually have RW[X] GTT control (e.g. BDW). A shared mapping restriction *might* be sufficient, but like Daniel said, the real fix is to properly handle the PROT_* bits... Seems like it could be kind of a cool feature though, so we should try to enable it on BDW+. Jesse Jesse