diff mbox

[2/2] drm: prime: fix lookup of existing imports for self imported buffers

Message ID 1365509289-11796-2-git-send-email-imre.deak@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Imre Deak April 9, 2013, 12:08 p.m. UTC
Since atm we don't take a reference on the dma buf pointer when we add
it to the import lookup table the dma buf can vanish leaving the stale
pointer behind. This can in turn lead to returning stale GEM handles
when userspace imports a newly exported buffer.

Fix this by keeping a reference on the dma buffer whenever we have a
pointer to it in the lookup table.

Reference: https://bugs.freedesktop.org/show_bug.cgi?id=59229

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/drm_prime.c |    2 ++
 1 file changed, 2 insertions(+)

Comments

Dave Airlie April 9, 2013, 9:52 p.m. UTC | #1
> Since atm we don't take a reference on the dma buf pointer when we add
> it to the import lookup table the dma buf can vanish leaving the stale
> pointer behind. This can in turn lead to returning stale GEM handles
> when userspace imports a newly exported buffer.


I sent a bunch of patches to prime months ago, maybe go back and dig them out

they might fix some of these issues,

I think danvet bikeshedded my will to care at the time due to lack of
proper locking,
the fact is the patches didn't change the locking, but I could
probably go back and find them.

Hopefully you haven't gone and reinvented that work.

Dave.
Imre Deak April 11, 2013, 9:24 a.m. UTC | #2
On Wed, 2013-04-10 at 07:52 +1000, Dave Airlie wrote:
> > Since atm we don't take a reference on the dma buf pointer when we add
> > it to the import lookup table the dma buf can vanish leaving the stale
> > pointer behind. This can in turn lead to returning stale GEM handles
> > when userspace imports a newly exported buffer.
> 
> 
> I sent a bunch of patches to prime months ago, maybe go back and dig them out
> 
> they might fix some of these issues,
> 
> I think danvet bikeshedded my will to care at the time due to lack of
> proper locking,
> the fact is the patches didn't change the locking, but I could
> probably go back and find them.
> 
> Hopefully you haven't gone and reinvented that work.

Yes, I checked it with the i-g-t/prime_self_import test case and it's
passing with your

"drm/prime: keep a reference from the handle to exported dma-buf (v2.1)"

patch, so we can drop this one.

--Imre
diff mbox

Patch

diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index bba45f6..e4e1a69 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -501,6 +501,7 @@  int drm_prime_add_imported_buf_handle(struct drm_prime_file_private *prime_fpriv
 	if (!member)
 		return -ENOMEM;
 
+	get_dma_buf(dma_buf);
 	member->dma_buf = dma_buf;
 	member->handle = handle;
 	list_add(&member->entry, &prime_fpriv->head);
@@ -529,6 +530,7 @@  void drm_prime_remove_imported_buf_handle(struct drm_prime_file_private *prime_f
 	mutex_lock(&prime_fpriv->lock);
 	list_for_each_entry_safe(member, safe, &prime_fpriv->head, entry) {
 		if (member->dma_buf == dma_buf) {
+			dma_buf_put(dma_buf);
 			list_del(&member->entry);
 			kfree(member);
 		}