Re: [PATCH] drm/prime: drop reference on imported dma-buf

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Sep 5, 2012 at 4:02 AM, Daniel Vetter <daniel@xxxxxxxx> wrote:
> 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 :(

oh, ok.. I guess there was a thread that I missed..

BR,
-R

> -Daniel
>
>>
>> BR,
>> -R
>>
>> On Tue, Jul 24, 2012 at 3:05 AM, Daniel Vetter <daniel@xxxxxxxx> wrote:
>> > On Tue, Jul 24, 2012 at 1:07 AM, Rob Clark <rob.clark@xxxxxxxxxx> wrote:
>> >> From: Rob Clark <rob@xxxxxx>
>> >>
>> >> 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@xxxxxx>
>> > [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@xxxxxxxx>
>> >> ---
>> >>  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@xxxxxxxxxxxxxxxxxxxxx
>> >> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>> >
>> >
>> >
>> > --
>> > Daniel Vetter
>> > daniel.vetter@xxxxxxxx - +41 (0) 79 364 57 48 - http://blog.ffwll.ch
>> > _______________________________________________
>> > dri-devel mailing list
>> > dri-devel@xxxxxxxxxxxxxxxxxxxxx
>> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@xxxxxxxxxxxxxxxxxxxxx
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Mail: daniel@xxxxxxxx
> Mobile: +41 (0)79 365 57 48
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel


[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux