Re: [PATCH] drm/i915: Introduce mapping of user pages into video memory (userptr) ioctl

[Jesse Barnes <jbarnes@virtuousgeek.org>]

On Tue, 4 Feb 2014 11:56:47 +0100
Daniel Vetter <daniel@ffwll.ch> 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 <chris@chris-wilson.co.uk> 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