Commit ea2aa97c authored by Jeffy Chen's avatar Jeffy Chen Committed by Christian König

drm/gem: Fix GEM handle release errors

Currently we are assuming a one to one mapping between dmabuf and
GEM handle when releasing GEM handles.

But that is not always true, since we would create extra handles for the
GEM obj in cases like gem_open() and getfb{,2}().

A similar issue was reported at:
https://lore.kernel.org/all/20211105083308.392156-1-jay.xu@rock-chips.com/

Another problem is that the imported dmabuf might not always have
gem_obj->dma_buf set, which would cause leaks in
drm_gem_remove_prime_handles().

Let's fix these for now by using handle to find the exact map to remove.
Signed-off-by: default avatarJeffy Chen <jeffy.chen@rock-chips.com>
Reviewed-by: default avatarChristian König <christian.koenig@amd.com>
Signed-off-by: default avatarChristian König <christian.koenig@amd.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20220819072834.17888-1-jeffy.chen@rock-chips.com
parent 258e483a
...@@ -168,21 +168,6 @@ void drm_gem_private_object_init(struct drm_device *dev, ...@@ -168,21 +168,6 @@ void drm_gem_private_object_init(struct drm_device *dev,
} }
EXPORT_SYMBOL(drm_gem_private_object_init); EXPORT_SYMBOL(drm_gem_private_object_init);
static void
drm_gem_remove_prime_handles(struct drm_gem_object *obj, struct drm_file *filp)
{
/*
* Note: obj->dma_buf can't disappear as long as we still hold a
* handle reference in obj->handle_count.
*/
mutex_lock(&filp->prime.lock);
if (obj->dma_buf) {
drm_prime_remove_buf_handle_locked(&filp->prime,
obj->dma_buf);
}
mutex_unlock(&filp->prime.lock);
}
/** /**
* drm_gem_object_handle_free - release resources bound to userspace handles * drm_gem_object_handle_free - release resources bound to userspace handles
* @obj: GEM object to clean up. * @obj: GEM object to clean up.
...@@ -253,7 +238,7 @@ drm_gem_object_release_handle(int id, void *ptr, void *data) ...@@ -253,7 +238,7 @@ drm_gem_object_release_handle(int id, void *ptr, void *data)
if (obj->funcs->close) if (obj->funcs->close)
obj->funcs->close(obj, file_priv); obj->funcs->close(obj, file_priv);
drm_gem_remove_prime_handles(obj, file_priv); drm_prime_remove_buf_handle(&file_priv->prime, id);
drm_vma_node_revoke(&obj->vma_node, file_priv); drm_vma_node_revoke(&obj->vma_node, file_priv);
drm_gem_object_handle_put_unlocked(obj); drm_gem_object_handle_put_unlocked(obj);
......
...@@ -74,8 +74,8 @@ int drm_prime_fd_to_handle_ioctl(struct drm_device *dev, void *data, ...@@ -74,8 +74,8 @@ int drm_prime_fd_to_handle_ioctl(struct drm_device *dev, void *data,
void drm_prime_init_file_private(struct drm_prime_file_private *prime_fpriv); void drm_prime_init_file_private(struct drm_prime_file_private *prime_fpriv);
void drm_prime_destroy_file_private(struct drm_prime_file_private *prime_fpriv); void drm_prime_destroy_file_private(struct drm_prime_file_private *prime_fpriv);
void drm_prime_remove_buf_handle_locked(struct drm_prime_file_private *prime_fpriv, void drm_prime_remove_buf_handle(struct drm_prime_file_private *prime_fpriv,
struct dma_buf *dma_buf); uint32_t handle);
/* drm_drv.c */ /* drm_drv.c */
struct drm_minor *drm_minor_acquire(unsigned int minor_id); struct drm_minor *drm_minor_acquire(unsigned int minor_id);
......
...@@ -190,29 +190,33 @@ static int drm_prime_lookup_buf_handle(struct drm_prime_file_private *prime_fpri ...@@ -190,29 +190,33 @@ static int drm_prime_lookup_buf_handle(struct drm_prime_file_private *prime_fpri
return -ENOENT; return -ENOENT;
} }
void drm_prime_remove_buf_handle_locked(struct drm_prime_file_private *prime_fpriv, void drm_prime_remove_buf_handle(struct drm_prime_file_private *prime_fpriv,
struct dma_buf *dma_buf) uint32_t handle)
{ {
struct rb_node *rb; struct rb_node *rb;
rb = prime_fpriv->dmabufs.rb_node; mutex_lock(&prime_fpriv->lock);
rb = prime_fpriv->handles.rb_node;
while (rb) { while (rb) {
struct drm_prime_member *member; struct drm_prime_member *member;
member = rb_entry(rb, struct drm_prime_member, dmabuf_rb); member = rb_entry(rb, struct drm_prime_member, handle_rb);
if (member->dma_buf == dma_buf) { if (member->handle == handle) {
rb_erase(&member->handle_rb, &prime_fpriv->handles); rb_erase(&member->handle_rb, &prime_fpriv->handles);
rb_erase(&member->dmabuf_rb, &prime_fpriv->dmabufs); rb_erase(&member->dmabuf_rb, &prime_fpriv->dmabufs);
dma_buf_put(dma_buf); dma_buf_put(member->dma_buf);
kfree(member); kfree(member);
return; break;
} else if (member->dma_buf < dma_buf) { } else if (member->handle < handle) {
rb = rb->rb_right; rb = rb->rb_right;
} else { } else {
rb = rb->rb_left; rb = rb->rb_left;
} }
} }
mutex_unlock(&prime_fpriv->lock);
} }
void drm_prime_init_file_private(struct drm_prime_file_private *prime_fpriv) void drm_prime_init_file_private(struct drm_prime_file_private *prime_fpriv)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment