dri-devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel@ffwll.ch>
To: Rob Clark <rob.clark@linaro.org>
Cc: dri-devel@lists.freedesktop.org, patches@linaro.org
Subject: Re: [PATCH] drm/prime: drop reference on imported dma-buf
Date: Wed, 5 Sep 2012 11:02:20 +0200	[thread overview]
Message-ID: <20120905090220.GA5357@phenom.ffwll.local> (raw)
In-Reply-To: <CAF6AEGvn=0q8gbv9-MBom2eYdsPUz5Ngfa-0EZTMKgqZz2+-0w@mail.gmail.com>

On Tue, Sep 04, 2012 at 05:32:21PM -0500, Rob Clark wrote:
> Dave, I just noticed that I still have this patch locally, but don't
> see it in drm-next..  so just checking that it didn't get forgotten

My locking fixes blew up :(
-Daniel

> 
> BR,
> -R
> 
> On Tue, Jul 24, 2012 at 3:05 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Tue, Jul 24, 2012 at 1:07 AM, Rob Clark <rob.clark@linaro.org> wrote:
> >> From: Rob Clark <rob@ti.com>
> >>
> >> 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 <rob@ti.com>
> > [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 <daniel.vetter@ffwll.ch>
> >> ---
> >>  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
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

  reply	other threads:[~2012-09-05  9:01 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-07-23 23:07 [PATCH] drm/prime: drop reference on imported dma-buf Rob Clark
2012-07-24  6:17 ` Paul Menzel
2012-07-24  8:05 ` Daniel Vetter
2012-09-04 22:32   ` Rob Clark
2012-09-05  9:02     ` Daniel Vetter [this message]
2012-09-05 12:01       ` Rob Clark

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120905090220.GA5357@phenom.ffwll.local \
    --to=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=patches@linaro.org \
    --cc=rob.clark@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox