gpu: ion: Fix race condition with import
Consider two threads. Thread 1 has an fd linked to an ion buffer
and Thread 2 has a handle to the same buffer. The two came from
the same client
Thread 1 Thread 2
-----------------------------------------
ion_import_fd
ion_import
mutex_lock(&client->lock)
ion_free
ion_handle_put
ion_handle_destroy
mutex_lock(&client->lock) <--- currently locked
ion_handle_lookup
<return reference to same handle>
mutex_unlock(&client->lock)
acquire client lock
free(handle)
Thread 1 is now holding a reference to an already freed handle.
The issue arises because thread 2 is attempting to destroy the
handle but the handle still exists on the clients list of handles.
This needs to be atomic. Fix this by taking the client lock
around ion_handle_put.
CRs-Fixed: 328348
Change-Id: I3ff5e6c50b5268fd42092bc1f2b99403e5fcd3cd
Signed-off-by: Laura Abbott <lauraa@codeaurora.org>
diff --git a/drivers/gpu/ion/ion.c b/drivers/gpu/ion/ion.c
index 8847a8d..a6c36c8 100644
--- a/drivers/gpu/ion/ion.c
+++ b/drivers/gpu/ion/ion.c
@@ -274,6 +274,7 @@
return handle;
}
+/* Client lock must be locked when calling */
static void ion_handle_destroy(struct kref *kref)
{
struct ion_handle *handle = container_of(kref, struct ion_handle, ref);
@@ -282,10 +283,8 @@
*/
WARN_ON(handle->kmap_cnt || handle->dmap_cnt || handle->usermap_cnt);
ion_buffer_put(handle->buffer);
- mutex_lock(&handle->client->lock);
if (!RB_EMPTY_NODE(&handle->node))
rb_erase(&handle->node, &handle->client->handles);
- mutex_unlock(&handle->client->lock);
kfree(handle);
}
@@ -448,13 +447,13 @@
mutex_lock(&client->lock);
valid_handle = ion_handle_validate(client, handle);
- mutex_unlock(&client->lock);
-
if (!valid_handle) {
+ mutex_unlock(&client->lock);
WARN("%s: invalid handle passed to free.\n", __func__);
return;
}
ion_handle_put(handle);
+ mutex_unlock(&client->lock);
}
EXPORT_SYMBOL(ion_free);
@@ -1254,7 +1253,9 @@
atomic_read(&client->ref.refcount),
atomic_read(&handle->ref.refcount),
atomic_read(&buffer->ref.refcount));
+ mutex_lock(&client->lock);
ion_handle_put(handle);
+ mutex_unlock(&client->lock);
ion_client_put(client);
pr_debug("%s: %d client_cnt %d handle_cnt %d alloc_cnt %d\n",
__func__, __LINE__,
@@ -1349,7 +1350,9 @@
mutex_unlock(&buffer->lock);
/* drop the reference to the handle */
err1:
+ mutex_lock(&client->lock);
ion_handle_put(handle);
+ mutex_unlock(&client->lock);
err:
/* drop the reference to the client */
ion_client_put(client);