From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH] drm/prime: drop reference on imported dma-buf Date: Tue, 24 Jul 2012 10:05:22 +0200 Message-ID: <20120724080522.GA5506@phenom.ffwll.local> References: <1343084868-13427-1-git-send-email-rob.clark@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-wi0-f171.google.com (mail-wi0-f171.google.com [209.85.212.171]) by gabe.freedesktop.org (Postfix) with ESMTP id 92650A09FD for ; Tue, 24 Jul 2012 01:06:34 -0700 (PDT) Received: by wibhq4 with SMTP id hq4so3069892wib.12 for ; Tue, 24 Jul 2012 01:06:33 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1343084868-13427-1-git-send-email-rob.clark@linaro.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org To: Rob Clark Cc: Rob Clark , dri-devel@lists.freedesktop.org, patches@linaro.org List-Id: dri-devel@lists.freedesktop.org On Tue, Jul 24, 2012 at 1:07 AM, Rob Clark wrote: > From: Rob Clark > > The GEM handle takes the reference. If a driver is actually importing a > foreign dmabuf, rather than just re-importing it's own dmabuf, it needs > to do a get_dma_buf(). > > Signed-off-by: Rob Clark [Maybe mention that this is on top of my locking fixes.] We've discussed this a bit on irc and concluded that every function taking care of their own references to the dma buf works best in avoiding potential use-after-free issues when evil userspaces sneaks in a close(dma_buf_fd). Reviewed-by: Daniel Vetter > --- > drivers/gpu/drm/drm_prime.c | 7 +++++++ > drivers/gpu/drm/exynos/exynos_drm_dmabuf.c | 4 ++++ > drivers/gpu/drm/i915/i915_gem_dmabuf.c | 4 ++++ > drivers/gpu/drm/nouveau/nouveau_prime.c | 5 +++++ > drivers/gpu/drm/radeon/radeon_prime.c | 4 ++++ > drivers/gpu/drm/udl/udl_gem.c | 4 ++++ > 6 files changed, 28 insertions(+) > > diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c > index 4f80374..088e018 100644 > --- a/drivers/gpu/drm/drm_prime.c > +++ b/drivers/gpu/drm/drm_prime.c > @@ -185,6 +185,13 @@ int drm_gem_prime_fd_to_handle(struct drm_device *dev, > mutex_unlock(&file_priv->prime.lock); > drm_gem_object_unreference_unlocked(obj); > > + /* > + * Drop the ref we obtained w/ dma_buf_get() for the benefit of > + * drivers simply re-importing their own dma-buf.. if drivers > + * import a foreign dma-buf, they should get_dma_buf(). > + */ > + dma_buf_put(dma_buf); > + > return 0; > > fail: > diff --git a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c > index f270790..f8ac75b 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c > @@ -190,6 +190,9 @@ struct drm_gem_object *exynos_dmabuf_prime_import(struct drm_device *drm_dev, > } > } > > + /* since we are attaching, we need to hold a ref */ > + get_dma_buf(dma_buf); > + > attach = dma_buf_attach(dma_buf, drm_dev->dev); > if (IS_ERR(attach)) > return ERR_PTR(-EINVAL); > @@ -250,6 +253,7 @@ err_unmap_attach: > dma_buf_unmap_attachment(attach, sgt, DMA_BIDIRECTIONAL); > err_buf_detach: > dma_buf_detach(dma_buf, attach); > + dma_buf_put(dma_buf); > return ERR_PTR(ret); > } > > diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/i915_gem_dmabuf.c > index cf6bdec..9d2b2eb 100644 > --- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c > +++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c > @@ -189,6 +189,9 @@ struct drm_gem_object *i915_gem_prime_import(struct drm_device *dev, > } > } > > + /* since we are attaching, we need to hold a ref */ > + get_dma_buf(dma_buf); > + > /* need to attach */ > attach = dma_buf_attach(dma_buf, dev->dev); > if (IS_ERR(attach)) > @@ -224,5 +227,6 @@ fail_unmap: > dma_buf_unmap_attachment(attach, sg, DMA_BIDIRECTIONAL); > fail_detach: > dma_buf_detach(dma_buf, attach); > + dma_buf_put(dma_buf); > return ERR_PTR(ret); > } > diff --git a/drivers/gpu/drm/nouveau/nouveau_prime.c b/drivers/gpu/drm/nouveau/nouveau_prime.c > index 4c82801..fc30886 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_prime.c > +++ b/drivers/gpu/drm/nouveau/nouveau_prime.c > @@ -200,6 +200,10 @@ struct drm_gem_object *nouveau_gem_prime_import(struct drm_device *dev, > } > } > } > + > + /* since we are attaching, we need to hold a ref */ > + get_dma_buf(dma_buf); > + > /* need to attach */ > attach = dma_buf_attach(dma_buf, dev->dev); > if (IS_ERR(attach)) > @@ -223,6 +227,7 @@ fail_unmap: > dma_buf_unmap_attachment(attach, sg, DMA_BIDIRECTIONAL); > fail_detach: > dma_buf_detach(dma_buf, attach); > + dma_buf_put(dma_buf); > return ERR_PTR(ret); > } > > diff --git a/drivers/gpu/drm/radeon/radeon_prime.c b/drivers/gpu/drm/radeon/radeon_prime.c > index 28f79ac..073a8d3 100644 > --- a/drivers/gpu/drm/radeon/radeon_prime.c > +++ b/drivers/gpu/drm/radeon/radeon_prime.c > @@ -195,6 +195,9 @@ struct drm_gem_object *radeon_gem_prime_import(struct drm_device *dev, > } > } > > + /* since we are attaching, we need to hold a ref */ > + get_dma_buf(dma_buf); > + > /* need to attach */ > attach = dma_buf_attach(dma_buf, dev->dev); > if (IS_ERR(attach)) > @@ -218,5 +221,6 @@ fail_unmap: > dma_buf_unmap_attachment(attach, sg, DMA_BIDIRECTIONAL); > fail_detach: > dma_buf_detach(dma_buf, attach); > + dma_buf_put(dma_buf); > return ERR_PTR(ret); > } > diff --git a/drivers/gpu/drm/udl/udl_gem.c b/drivers/gpu/drm/udl/udl_gem.c > index 7bd65bd..47f25d5 100644 > --- a/drivers/gpu/drm/udl/udl_gem.c > +++ b/drivers/gpu/drm/udl/udl_gem.c > @@ -305,6 +305,9 @@ struct drm_gem_object *udl_gem_prime_import(struct drm_device *dev, > struct udl_gem_object *uobj; > int ret; > > + /* since we are attaching, we need to hold a ref */ > + get_dma_buf(dma_buf); > + > /* need to attach */ > attach = dma_buf_attach(dma_buf, dev->dev); > if (IS_ERR(attach)) > @@ -329,5 +332,6 @@ fail_unmap: > dma_buf_unmap_attachment(attach, sg, DMA_BIDIRECTIONAL); > fail_detach: > dma_buf_detach(dma_buf, attach); > + dma_buf_put(dma_buf); > return ERR_PTR(ret); > } > -- > 1.7.9.5 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter daniel.vetter@ffwll.ch - +41 (0) 79 364 57 48 - http://blog.ffwll.ch