Re: [PATCH 00/03] Preventing zero GPU virtual address allocation

Daniel Vetter
2015-05-21T09:42:15Z

On Thu, May 21, 2015 at 09:43:00AM +0100, Chris Wilson wrote:
> On Thu, May 21, 2015 at 11:08:42AM +0300, David Weinehall wrote:
> > On Wed, May 20, 2015 at 05:10:58PM +0100, Chris Wilson wrote:
> > > On Wed, May 20, 2015 at 06:00:43PM +0200, Daniel Vetter wrote:
> > > > On Wed, May 20, 2015 at 03:14:06PM +0100, Chris Wilson wrote:
> > > > > On Wed, May 20, 2015 at 03:09:43PM +0100, Chris Wilson wrote:
> > > > > > On Wed, May 20, 2015 at 04:54:19PM +0300, David Weinehall wrote:
> > > > > > > This patch series (one patch each for libdrm, the kernel, and beignet)
> > > > > > > aims to provide a means to add a context-specific means to prevent
> > > > > > > a mapping to GPU virtual address zero. This is needed at least by
> > > > > > > Beignet (possibly in other use-cases too, though I don't know of any
> > > > > > > other) to allow use of address zero to represent NULL.
> > > > > >
> > > > > > Urm, you cannot allow absolute addressing period. What happens to the
> > > > > > object at 0 when the user reads from it or writes to it? You have to
> > > > > > have an object at 0 for the user's NULL pointer access.
> > > > >
> > > > > I'll mollify that: outside of full-ppgtt where you need to share the VM.
> > > >
> > > > The description is misleading, the new flag doesn't prevent anything from
> > > > getting mapped at 0 but only prevents any bo submitted through execbuf on
> > > > the given context from being bound at address 0. If that would happen
> > > > compute kernels using NULL checks for some things would fall over.
> >
> > Any suggestion for a better description?
> >
> > > That does not address the issue that *0 is an untrapped runtime bug that
> > > allows writing to other objects or reading from them.
> >
> > Not exactly sure what you suggest here?
>
> That you have an unmitigated security hole in your design.

There's no security issue afaics, there's only a correctness issue. opencl
kernels must be able to assume that NULL never points to a valid buffer
address of their own. It might point at other intersting stuff like batch
buffers or what else, but never about anything created by the ocl client
itself. This patch achieves that.

Ofc there's still the issue that anything else mapped can be stomped upon,
but that's just undefined behaviour in C-spec speak. And there's still the
isolation issues, but ocl doesn't make any new guarantees there afaik.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation