video: tegra: nvmap: Fix data race for RO dma-buf

When one process is trying to duplicate RO handle while other process is
trying to free the same RO handle, then race can occur and second
process can decrement the dma-buf's refcount and it may reach to 0. The
first process can then call get_dma_buf on it, leading to NULL pointer
dereference and ultimately to kernel panic. Fix this by taking an extra
dma-buf refcount before duplicating the handle and then decrease it once
duplication is completed.

Bug 3991243

Change-Id: I99901ce19d8a5d23c5192cb10a17efd2ebaf9d3a
Signed-off-by: Ketan Patil <ketanp@nvidia.com>
Reviewed-on: https://git-master.nvidia.com/r/c/linux-nvidia/+/2865519
Reviewed-by: svc_kernel_abi <svc_kernel_abi@nvidia.com>
Reviewed-by: svcacv <svcacv@nvidia.com>
Reviewed-by: Krishna Reddy <vdumpa@nvidia.com>
GVS: Gerrit_Virtual_Submit <buildbot_gerritrpt@nvidia.com>
This commit is contained in:
Ketan Patil
2023-03-01 15:23:38 +00:00
committed by Laxman Dewangan
parent eb15c0f8cf
commit 678dc695bb
2 changed files with 15 additions and 28 deletions

View File

@@ -475,7 +475,6 @@ struct nvmap_handle_ref *nvmap_dup_handle_ro(struct nvmap_client *client,
{ {
struct nvmap_handle *h; struct nvmap_handle *h;
struct nvmap_handle_ref *ref = NULL; struct nvmap_handle_ref *ref = NULL;
bool dmabuf_created = false;
long remain; long remain;
if (!client) if (!client)
@@ -499,7 +498,6 @@ struct nvmap_handle_ref *nvmap_dup_handle_ro(struct nvmap_client *client,
mutex_unlock(&h->lock); mutex_unlock(&h->lock);
return ERR_CAST(h->dmabuf_ro); return ERR_CAST(h->dmabuf_ro);
} }
dmabuf_created = true;
} else { } else {
if (!get_file_rcu(h->dmabuf_ro->file)) { if (!get_file_rcu(h->dmabuf_ro->file)) {
mutex_unlock(&h->lock); mutex_unlock(&h->lock);
@@ -513,30 +511,21 @@ struct nvmap_handle_ref *nvmap_dup_handle_ro(struct nvmap_client *client,
mutex_unlock(&h->lock); mutex_unlock(&h->lock);
return ERR_CAST(h->dmabuf_ro); return ERR_CAST(h->dmabuf_ro);
} }
dmabuf_created = true;
} else { } else {
nvmap_handle_put(h); nvmap_handle_put(h);
return ERR_PTR(-EINVAL); return ERR_PTR(-EINVAL);
} }
} else {
dma_buf_put(h->dmabuf_ro);
} }
} }
mutex_unlock(&h->lock); mutex_unlock(&h->lock);
ref = nvmap_duplicate_handle(client, h, false, true); ref = nvmap_duplicate_handle(client, h, false, true);
if (!ref) {
nvmap_handle_put(h);
return ref;
}
/* /*
* When new dmabuf created (only RO dmabuf is getting created in this function) * When new RO dmabuf created or duplicated, one extra dma_buf refcount is taken so to
* it's counter is incremented one extra time in nvmap_duplicate_handle. Hence * avoid getting it freed by another process, until duplication completes. Decrement that
* decrement it by one. * extra refcount here.
*/ */
if (dmabuf_created)
dma_buf_put(h->dmabuf_ro); dma_buf_put(h->dmabuf_ro);
nvmap_handle_put(h); nvmap_handle_put(h);
return ref; return ref;

View File

@@ -220,7 +220,7 @@ found:
int nvmap_get_handle_from_sci_ipc_id(struct nvmap_client *client, u32 flags, int nvmap_get_handle_from_sci_ipc_id(struct nvmap_client *client, u32 flags,
u64 sci_ipc_id, NvSciIpcEndpointVuid localu_vuid, u32 *handle) u64 sci_ipc_id, NvSciIpcEndpointVuid localu_vuid, u32 *handle)
{ {
bool is_ro = (flags == PROT_READ) ? true : false, dmabuf_created = false; bool is_ro = (flags == PROT_READ) ? true : false;
struct nvmap_handle_ref *ref = NULL; struct nvmap_handle_ref *ref = NULL;
struct nvmap_sci_ipc_entry *entry; struct nvmap_sci_ipc_entry *entry;
struct dma_buf *dmabuf = NULL; struct dma_buf *dmabuf = NULL;
@@ -256,7 +256,6 @@ int nvmap_get_handle_from_sci_ipc_id(struct nvmap_client *client, u32 flags,
mutex_unlock(&h->lock); mutex_unlock(&h->lock);
goto unlock; goto unlock;
} }
dmabuf_created = true;
} else { } else {
if (!get_file_rcu(h->dmabuf_ro->file)) { if (!get_file_rcu(h->dmabuf_ro->file)) {
mutex_unlock(&h->lock); mutex_unlock(&h->lock);
@@ -270,31 +269,30 @@ int nvmap_get_handle_from_sci_ipc_id(struct nvmap_client *client, u32 flags,
mutex_unlock(&h->lock); mutex_unlock(&h->lock);
goto unlock; goto unlock;
} }
dmabuf_created = true;
} else { } else {
ret = -EINVAL; ret = -EINVAL;
goto unlock; goto unlock;
} }
} else {
dma_buf_put(h->dmabuf_ro);
} }
} }
} }
mutex_unlock(&h->lock); mutex_unlock(&h->lock);
ref = nvmap_duplicate_handle(client, h, false, is_ro); ref = nvmap_duplicate_handle(client, h, false, is_ro);
if (!ref) { /*
* When new RO dmabuf created or duplicated, one extra dma_buf refcount is taken so to
* avoid getting it freed by another process, until duplication completes. Decrement that
* extra refcount here.
*/
if (is_ro)
dma_buf_put(h->dmabuf_ro);
if (IS_ERR(ref)) {
ret = -EINVAL; ret = -EINVAL;
goto unlock; goto unlock;
} }
nvmap_handle_put(h); nvmap_handle_put(h);
/*
* When new dmabuf created (only RO dmabuf is getting created in this function)
* it's counter is incremented one extra time in nvmap_duplicate_handle. Hence
* decrement it by one.
*/
if (dmabuf_created)
dma_buf_put(h->dmabuf_ro);
if (!IS_ERR(ref)) { if (!IS_ERR(ref)) {
u32 id = 0; u32 id = 0;