Re: [RFC] dma-buf: Import/export the implicit fences on the dma-buf

From: Daniel Vetter <daniel@ffwll.ch>
To: "Chris Wilson" <chris@chris-wilson.co.uk>,
	"Christian König" <christian.koenig@amd.com>,
	dri-devel@lists.freedesktop.org,
	"Daniel Vetter" <daniel.vetter@ffwll.ch>,
	"Gustavo Padovan" <gustavo.padovan@collabora.co.uk>
Subject: Re: [RFC] dma-buf: Import/export the implicit fences on the dma-buf
Date: Tue, 12 Jul 2016 16:31:02 +0200	[thread overview]
Message-ID: <20160712143102.GS23520@phenom.ffwll.local> (raw)
In-Reply-To: <20160712121441.GS6157@nuc-i3427.alporthouse.com>

On Tue, Jul 12, 2016 at 01:14:41PM +0100, Chris Wilson wrote:
> On Tue, Jul 12, 2016 at 01:53:56PM +0200, Christian König wrote:
> > Am 11.07.2016 um 23:59 schrieb Chris Wilson:
> > >When dealing with user interfaces that utilize explicit fences, it is
> > >convenient to sometimes create those fences after the fact, i.e. to
> > >query the dma-buf for the current set of implicit fences, encapsulate
> > >those into a sync_file and hand that fd back to userspace.
> > >Correspondingly, being able to add an explicit fence back into the mix
> > >of fences being tracked by the dma-buf allows that userspace fence to be
> > >included in any implicit tracking.
> > 
> > Well I think that this is a rather questionable interface.
> > 
> > For example how do you deal with race conditions? E.g. between
> > querying the fences from the reservation object and adding a new one
> > we could gain new fences because of the kernel swapping things out
> > or another thread making some submission with this buffer.
> > 
> > Additional to that IIRC reservation_object_add_excl_fence()
> > currently replaces all shared fences with the exclusive one. A
> > malicious application could use this to trick the kernel driver into
> > thinking that this buffer object is idle while it is still accessed
> > by some operation. At least with GPU operations you can easily take
> > over the system when you manage to get access to a page table with
> > this.
> 
> The only difference here is that we believe the GPU drivers to enforce
> the ordering between each other. So we can either insert a wait before
> adding the exclusive fence, or we can just not expose an import ioctl.
> Extracting the set of fences isn't an issue? (That's the part that has a
> more legitimate usecase.)

If we change the kernel to just merge everything together when importing a
new fence I think it should be perfectly save. I.e.
1) grab reservation lock
2) assemble a fence_array with the current fences + the new one passed in
through sync_file.
3) put that into the right slot
4) unlock

If we switch reservations over to fence_array it might even be somewhat
pretty.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel