From 5c59fead4610a13acb9165fe53b27ac7f067e1e6 Mon Sep 17 00:00:00 2001 From: Ketan Patil Date: Tue, 10 Oct 2023 12:17:39 +0000 Subject: [PATCH] video: tegra: nvmap: Fix data race Consider the following execution scenario, it can lead to a data race Thread 1 Thread 2 ------------------------------------------------------- NvRmMemHandleAllocAttr NvRmMemGetFd NvRmMemHandleFromFd close(fd), NvRmMemHandleFree When thread 1 is executing NvRmMemHandleFromFd while thread 2 is closing the fd and freeing the handle, then following sequence will lead to accessing a freed dma-buf and may lead to dereference issue. - TH1: Get handle from fd and increment handle's refcount. - TH2: Close the fd and start execution of NvRmMemHandleFree. - TH2: Decrement ref's dup count, it will become 0, hence ref would be freed and dma-buf as well, but as handle's refcount is incremented in step 1, handle won't be freed. - TH1: Resume HandleFromFd part, call to nvmap_duplicate_handle. Ref is already freed, so generate new ref and increment dma-buf's count but as dma-buf is freed already, accessing dma-buf will lead to dereferene issue. Hence, we need to add a null check here and return error value in such scenario. Also, add check for return value of nvmap_handle_get, at missing places. Bug 4214453 Change-Id: Ib6ef66b4a7126bef2ed1dbb48643445a4ded1bab Signed-off-by: Ketan Patil Reviewed-on: https://git-master.nvidia.com/r/c/linux-nv-oot/+/2994962 (cherry picked from commit 7ba6a3abfe65f7cffeeb4004cf0868468121fc32) Reviewed-on: https://git-master.nvidia.com/r/c/linux-nv-oot/+/2994440 Reviewed-by: Sachin Nikam GVS: Gerrit_Virtual_Submit --- drivers/video/tegra/nvmap/nvmap_dmabuf.c | 6 +++++- drivers/video/tegra/nvmap/nvmap_handle.c | 9 +++++++++ drivers/video/tegra/nvmap/nvmap_sci_ipc.c | 6 ++++-- 3 files changed, 18 insertions(+), 3 deletions(-) diff --git a/drivers/video/tegra/nvmap/nvmap_dmabuf.c b/drivers/video/tegra/nvmap/nvmap_dmabuf.c index 31232523..f6c209f5 100644 --- a/drivers/video/tegra/nvmap/nvmap_dmabuf.c +++ b/drivers/video/tegra/nvmap/nvmap_dmabuf.c @@ -624,7 +624,11 @@ struct dma_buf *__nvmap_make_dmabuf(struct nvmap_client *client, err = PTR_ERR(dmabuf); goto err_export; } - nvmap_handle_get(handle); + + if (!nvmap_handle_get(handle)) { + err = -EINVAL; + goto err_export; + } trace_nvmap_make_dmabuf(client->name, handle, dmabuf); return dmabuf; diff --git a/drivers/video/tegra/nvmap/nvmap_handle.c b/drivers/video/tegra/nvmap/nvmap_handle.c index 6eabdfec..9953b78c 100644 --- a/drivers/video/tegra/nvmap/nvmap_handle.c +++ b/drivers/video/tegra/nvmap/nvmap_handle.c @@ -403,9 +403,13 @@ struct nvmap_handle_ref *nvmap_duplicate_handle(struct nvmap_client *client, if (is_ro) { ref->is_ro = true; + if (!h->dmabuf_ro) + goto exit; get_dma_buf(h->dmabuf_ro); } else { ref->is_ro = false; + if (!h->dmabuf) + goto exit; get_dma_buf(h->dmabuf); } @@ -413,6 +417,11 @@ out: NVMAP_TAG_TRACE(trace_nvmap_duplicate_handle, NVMAP_TP_ARGS_CHR(client, h, ref)); return ref; + +exit: + pr_err("dmabuf is NULL\n"); + kfree(ref); + return ERR_PTR(-EINVAL); } struct nvmap_handle_ref *nvmap_create_handle_from_id( diff --git a/drivers/video/tegra/nvmap/nvmap_sci_ipc.c b/drivers/video/tegra/nvmap/nvmap_sci_ipc.c index 84bb3600..a808a2de 100644 --- a/drivers/video/tegra/nvmap/nvmap_sci_ipc.c +++ b/drivers/video/tegra/nvmap/nvmap_sci_ipc.c @@ -186,8 +186,10 @@ int nvmap_create_sci_ipc_id(struct nvmap_client *client, } unlock: mutex_unlock(&nvmapsciipc->mlock); - if (!ret) - (void)nvmap_handle_get(h); + if (!ret) { + if (!nvmap_handle_get(h)) + return -EINVAL; + } return ret; }