Re: [PATCH] drm/i915: Add OACONTROL to the command parser register whitelist.

[Jesse Barnes <jbarnes@virtuousgeek.org>]

On Fri, 16 May 2014 13:12:27 -0700
"Volkin, Bradley D" <bradley.d.volkin@intel.com> wrote:

> On Fri, May 16, 2014 at 12:53:30PM -0700, Jesse Barnes wrote:
> > On Fri, 16 May 2014 12:34:08 -0700
> > Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> > 
> > > On Fri, 16 May 2014 20:20:50 +0100
> > > Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > > > Yes, X only sets the secure bit when it pokes the display registers, and
> > > > those registers should be privileged even with a cmd parser in place
> > > > (which they are).
> > > > 
> > > > Daniel's argument presumes that we haven't been patching out the
> > > > cmd parser all this time anyway.
> > > 
> > > Yeah I know we have some perf issues as it is; it would be nice if the
> > > overhead were so minimal that it didn't matter.  But just on principle,
> > > scanning secure buffers seems wrong, and I'm trying to understand why
> > > Daniel would want it.
> > 
> > Ok Daniel explained on IRC that we actually have a special whitelist
> > for the secure batch case.  The idea is to allow a DRM_MASTER to submit
> > secure batches, but still prevent a local root exploit.  I suppose that
> > means preventing access to most commands and registers, but allowing a
> > few extra things like wait events and display register updates.
> 
> Just to clarify further: the additional register whitelist and commands
> are only based on DRM_MASTER. Setting I915_EXEC_SECURE is not required. So
> I suppose we could stop scanning batches that have I915_EXEC_SECURE and
> userspace could stop sending such batches when the parser is fully enabled.

Ah ok, yeah that's another option, but now I understand where Daniel is
coming from with testing, since that's not how the current X driver
behaves.

-- 
Jesse Barnes, Intel Open Source Technology Center