[PATCH v3 1/2] drm/gem: drm_gem_dumb_map_offset(): reject dma-buf

[Daniel Vetter]

On Fri, Aug 18, 2017 at 01:34:45PM -0700, Eric Anholt wrote:
> Daniel Vetter <daniel at ffwll.ch> writes:
> 
> > On Fri, Aug 18, 2017 at 10:41:21AM -0700, Eric Anholt wrote:
> >> Noralf Trønnes <noralf at tronnes.org> writes:
> >> > Den 18.08.2017 09.46, skrev Daniel Vetter:
> >> >> On Thu, Aug 17, 2017 at 06:21:30PM +0200, Noralf Trønnes wrote:
> >> >>> Reject mapping an imported dma-buf since is's an invalid use-case.
> >> >>>
> >> >>> Cc: Philipp Zabel <p.zabel at pengutronix.de>
> >> >>> Cc: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> >> >>> Cc: Sean Paul <seanpaul at chromium.org>
> >> >>> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> >> >>> Signed-off-by: Noralf Trønnes <noralf at tronnes.org>
> >> >> I think acks from someone using mali would be good too. amdgpu already has
> >> >> such checks, so I think on the desktop side we're ok.
> >> >>
> >> >> Acked-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> >> >>
> >> >> But I think this one here definitely needs a few more acks. I could break
> >> >> uabi if we're unlucky, so let's not rush it.
> >> >
> >> > Ok, I've CC'ed the affected parties to increase the odds that they look
> >> > at this. These are the drivers using drm_gem_dumb_map_offset()
> >> > (hopefully I got the list right):
> >> 
> >> If I understand the affected path right, this would break the PL111+VC4
> >> combination: PL111 makes (dumb) buffers for scanout, and VC4 imports
> >> them and uses them for rendering.  A vc4 glReadPixels of the window
> >> system buffer would map it and fail.
> >
> > It only rejects the map call on dumb buffers, and mmap on imported dma-buf
> > tends to not really work well, or at least break a few abstractions. Are
> > you sure this works currently?
> 
> OK, that's right -- vc4 would be doing its "native" map call (the same
> code), not dumb map.
> 
> Furthermore, I had it backwards (I had written things both ways at
> different points, iirc).  We have VC4 making the buffers and PL111
> dma-buf importing them.  I don't see X11 mapping those buffers if glamor
> is enabled, so this should be OK for vc4.
> 
> GBM's dumb mapping looks safe to me.  X11 does some dumb maps, but I
> don't think any of those would be on imports.

Yeah the idea is only to lock down the dumb mmap and make sure abi abuse
(which might work on a specific combo of exporters/kms drivers) is caught
for this generic interface. dumb really should only be used for
unaccelarated rendering on exactly that driver.

So ack from you?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch