video: tegra: nvmap: fix use-after-free race condition
Incremented nvmap_handle ref count in utility function
nvmap_get_id_from_dmabuf_fd() before the function release reference
to dma buffer. This is required to avoid race conditions in nvmap
code where nvmap_handle returned by this function could be freed
concurrently while the caller is still using it.
As a side effect of above change, every caller of this utility
function must decrement nvmap_handle ref count after using the
returned nvmap_handle.
Bug: 17453812
Change-Id: Iffc2e5819f8b493d5ed95a9d0c422ccd52438965
Signed-off-by: Maneet Singh <mmaneetsingh@nvidia.com>
(cherry picked from commit 0b4d06100bf603c80f29f8f2b19537929fce1807)
diff --git a/drivers/video/tegra/nvmap/nvmap_dmabuf.c b/drivers/video/tegra/nvmap/nvmap_dmabuf.c
index 6f47c49..e0ade75 100644
--- a/drivers/video/tegra/nvmap/nvmap_dmabuf.c
+++ b/drivers/video/tegra/nvmap/nvmap_dmabuf.c
@@ -684,6 +684,11 @@
/*
* Returns the nvmap handle ID associated with the passed dma_buf's fd. This
* does not affect the ref count of the dma_buf.
+ * NOTE: Callers of this utility function must invoke nvmap_handle_put after
+ * using the returned nvmap_handle. Call to nvmap_handle_get is required in
+ * this utility function to avoid race conditions in code where nvmap_handle
+ * returned by this function is freed concurrently while the caller is still
+ * using it.
*/
struct nvmap_handle *nvmap_get_id_from_dmabuf_fd(struct nvmap_client *client,
int fd)
@@ -698,6 +703,8 @@
if (dmabuf->ops == &nvmap_dma_buf_ops) {
info = dmabuf->priv;
handle = info->handle;
+ if (!nvmap_handle_get(handle))
+ handle = ERR_PTR(-EINVAL);
}
dma_buf_put(dmabuf);
return handle;
@@ -719,6 +726,7 @@
return -EINVAL;
op.fd = nvmap_get_dmabuf_fd(client, handle);
+ nvmap_handle_put(handle);
if (op.fd < 0)
return op.fd;
diff --git a/drivers/video/tegra/nvmap/nvmap_handle.c b/drivers/video/tegra/nvmap/nvmap_handle.c
index a1eb641..f7646ae 100644
--- a/drivers/video/tegra/nvmap/nvmap_handle.c
+++ b/drivers/video/tegra/nvmap/nvmap_handle.c
@@ -428,7 +428,11 @@
void nvmap_free_handle_user_id(struct nvmap_client *client,
unsigned long user_id)
{
- nvmap_free_handle(client, unmarshal_user_handle(user_id));
+ struct nvmap_handle *handle = unmarshal_user_handle(user_id);
+ if (handle) {
+ nvmap_free_handle(client, handle);
+ nvmap_handle_put(handle);
+ }
}
static void add_handle_ref(struct nvmap_client *client,
@@ -599,6 +603,7 @@
if (IS_ERR(handle))
return ERR_CAST(handle);
ref = nvmap_duplicate_handle(client, handle, 1);
+ nvmap_handle_put(handle);
return ref;
}
diff --git a/drivers/video/tegra/nvmap/nvmap_ioctl.c b/drivers/video/tegra/nvmap/nvmap_ioctl.c
index 7a60fb6..f942920 100644
--- a/drivers/video/tegra/nvmap/nvmap_ioctl.c
+++ b/drivers/video/tegra/nvmap/nvmap_ioctl.c
@@ -48,6 +48,9 @@
unsigned long sys_stride, unsigned long elem_size,
unsigned long count);
+/* NOTE: Callers of this utility function must invoke nvmap_handle_put after
+ * using the returned nvmap_handle.
+ */
struct nvmap_handle *unmarshal_user_handle(__u32 handle)
{
struct nvmap_handle *h;
@@ -105,8 +108,8 @@
struct nvmap_handle *on_stack[16];
struct nvmap_handle **refs;
unsigned long __user *output = NULL;
- unsigned int i;
int err = 0;
+ u32 i, n_unmarshal_handles = 0;
#ifdef CONFIG_COMPAT
if (is32) {
@@ -149,6 +152,7 @@
err = -EINVAL;
goto out;
}
+ n_unmarshal_handles++;
}
} else {
refs = on_stack;
@@ -158,6 +162,7 @@
err = -EINVAL;
goto out;
}
+ n_unmarshal_handles++;
}
trace_nvmap_ioctl_pinop(filp->private_data, is_pin, op.count, refs);
@@ -220,6 +225,9 @@
nvmap_unpin_ids(filp->private_data, op.count, refs);
out:
+ for (i = 0; i < n_unmarshal_handles; i++)
+ nvmap_handle_put(refs[i]);
+
if (refs != on_stack)
kfree(refs);
@@ -238,11 +246,6 @@
if (!h)
return -EINVAL;
- h = nvmap_handle_get(h);
-
- if (!h)
- return -EPERM;
-
op.id = marshal_id(h);
nvmap_handle_put(h);
@@ -284,6 +287,7 @@
return -EINVAL;
op.fd = nvmap_get_dmabuf_fd(client, handle);
+ nvmap_handle_put(handle);
if (op.fd < 0)
return op.fd;
@@ -299,24 +303,27 @@
struct nvmap_alloc_handle op;
struct nvmap_client *client = filp->private_data;
struct nvmap_handle *handle;
+ int err;
if (copy_from_user(&op, arg, sizeof(op)))
return -EFAULT;
- handle = unmarshal_user_handle(op.handle);
- if (!handle)
+ if (op.align & (op.align - 1))
return -EINVAL;
- if (op.align & (op.align - 1))
+ handle = unmarshal_user_handle(op.handle);
+ if (!handle)
return -EINVAL;
/* user-space handles are aligned to page boundaries, to prevent
* data leakage. */
op.align = max_t(size_t, op.align, PAGE_SIZE);
- return nvmap_alloc_handle(client, handle, op.heap_mask, op.align,
+ err = nvmap_alloc_handle(client, handle, op.heap_mask, op.align,
0, /* no kind */
op.flags & (~NVMAP_HANDLE_KIND_SPECIFIED));
+ nvmap_handle_put(handle);
+ return err;
}
int nvmap_ioctl_alloc_kind(struct file *filp, void __user *arg)
@@ -324,26 +331,29 @@
struct nvmap_alloc_kind_handle op;
struct nvmap_client *client = filp->private_data;
struct nvmap_handle *handle;
+ int err;
if (copy_from_user(&op, arg, sizeof(op)))
return -EFAULT;
- handle = unmarshal_user_handle(op.handle);
- if (!handle)
+ if (op.align & (op.align - 1))
return -EINVAL;
- if (op.align & (op.align - 1))
+ handle = unmarshal_user_handle(op.handle);
+ if (!handle)
return -EINVAL;
/* user-space handles are aligned to page boundaries, to prevent
* data leakage. */
op.align = max_t(size_t, op.align, PAGE_SIZE);
- return nvmap_alloc_handle(client, handle,
+ err = nvmap_alloc_handle(client, handle,
op.heap_mask,
op.align,
op.kind,
op.flags);
+ nvmap_handle_put(handle);
+ return err;
}
int nvmap_create_fd(struct nvmap_client *client, struct nvmap_handle *h)
@@ -436,15 +446,9 @@
return -EFAULT;
h = unmarshal_user_handle(op.handle);
-
if (!h)
return -EINVAL;
- h = nvmap_handle_get(h);
-
- if (!h)
- return -EPERM;
-
if(!h->alloc) {
nvmap_handle_put(h);
return -EFAULT;
@@ -537,10 +541,6 @@
if (!h)
return -EINVAL;
- h = nvmap_handle_get(h);
- if (!h)
- return -EINVAL;
-
nvmap_ref_lock(client);
ref = __nvmap_validate_locked(client, h);
if (IS_ERR_OR_NULL(ref)) {
@@ -593,13 +593,12 @@
if (copy_from_user(&op, arg, sizeof(op)))
return -EFAULT;
- h = unmarshal_user_handle(op.handle);
- if (!h || !op.addr || !op.count || !op.elem_size)
+ if (!op.addr || !op.count || !op.elem_size)
return -EINVAL;
- h = nvmap_handle_get(h);
+ h = unmarshal_user_handle(op.handle);
if (!h)
- return -EPERM;
+ return -EINVAL;
trace_nvmap_ioctl_rw_handle(client, h, is_read, op.offset,
op.addr, op.hmem_stride,
@@ -636,11 +635,14 @@
unsigned long end;
int err = 0;
- handle = unmarshal_user_handle(op->handle);
- if (!handle || !op->addr || op->op < NVMAP_CACHE_OP_WB ||
+ if (!op->addr || op->op < NVMAP_CACHE_OP_WB ||
op->op > NVMAP_CACHE_OP_WB_INV)
return -EINVAL;
+ handle = unmarshal_user_handle(op->handle);
+ if (!handle)
+ return -EINVAL;
+
down_read(¤t->mm->mmap_sem);
vma = find_vma(current->active_mm, (unsigned long)op->addr);
@@ -667,6 +669,7 @@
false);
out:
up_read(¤t->mm->mmap_sem);
+ nvmap_handle_put(handle);
return err;
}
@@ -1127,7 +1130,8 @@
u32 *offset_ptr;
u32 *size_ptr;
struct nvmap_handle **refs;
- int i, err = 0;
+ int err = 0;
+ u32 i, n_unmarshal_handles = 0;
if (copy_from_user(&op, arg, sizeof(op)))
return -EFAULT;
@@ -1169,6 +1173,7 @@
err = -EINVAL;
goto free_mem;
}
+ n_unmarshal_handles++;
}
if (is_reserve_ioctl)
@@ -1179,6 +1184,8 @@
op.op, op.nr);
free_mem:
+ for (i = 0; i < n_unmarshal_handles; i++)
+ nvmap_handle_put(refs[i]);
kfree(refs);
return err;
}
