From: Dhabaleshwar Das <[email protected]>
rocket_ioctl_create_bo() inserts a GEM handle into the file's IDR via
drm_gem_handle_create() early on, then performs several operations that
can fail (sgt allocation, drm_mm insert, iommu_map). If any fail after
the handle is live, the error path calls drm_gem_shmem_object_free()
which kfree's the object without removing the handle from the IDR.
This leaves a dangling handle pointing to freed slab memory. Any
subsequent ioctl using that handle (PREP_BO, FINI_BO, SUBMIT) calls
drm_gem_object_lookup() and dereferences freed memory (UAF).
Fix by moving drm_gem_handle_create() to after all fallible operations
succeed, matching the pattern used by panfrost, lima, and etnaviv.
Also fix drm_mm_insert_node_generic() whose return value was silently
overwritten by iommu_map_sgtable() on the next line. Add the missing
error check.
[tomeu: Move handle creation to the very end]
Fixes: 658ebeac3351 ("accel/rocket: Add IOCTL for BO creation")
Reported-by: Dhabaleshwar Das <[email protected]>
Signed-off-by: Dhabaleshwar Das <[email protected]>
Reviewed-by: Tomeu Vizoso <[email protected]>
Signed-off-by: Tomeu Vizoso <[email protected]>
---
drivers/accel/rocket/rocket_gem.c | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/accel/rocket/rocket_gem.c
b/drivers/accel/rocket/rocket_gem.c
index c8084719208a..a5fffa51ff35 100644
--- a/drivers/accel/rocket/rocket_gem.c
+++ b/drivers/accel/rocket/rocket_gem.c
@@ -79,11 +79,6 @@ int rocket_ioctl_create_bo(struct drm_device *dev, void
*data, struct drm_file *
rkt_obj->size = args->size;
rkt_obj->offset = 0;
- ret = drm_gem_handle_create(file, gem_obj, &args->handle);
- drm_gem_object_put(gem_obj);
- if (ret)
- goto err;
-
sgt = drm_gem_shmem_get_pages_sgt(shmem_obj);
if (IS_ERR(sgt)) {
ret = PTR_ERR(sgt);
@@ -95,6 +90,8 @@ int rocket_ioctl_create_bo(struct drm_device *dev, void
*data, struct drm_file *
rkt_obj->size, PAGE_SIZE,
0, 0);
mutex_unlock(&rocket_priv->mm_lock);
+ if (ret)
+ goto err;
ret = iommu_map_sgtable(rocket_priv->domain->domain,
rkt_obj->mm.start,
@@ -112,8 +109,18 @@ int rocket_ioctl_create_bo(struct drm_device *dev, void
*data, struct drm_file *
args->offset = drm_vma_node_offset_addr(&gem_obj->vma_node);
args->dma_address = rkt_obj->mm.start;
+ ret = drm_gem_handle_create(file, gem_obj, &args->handle);
+ if (ret)
+ goto err_unmap;
+
+ drm_gem_object_put(gem_obj);
+
return 0;
+err_unmap:
+ iommu_unmap(rocket_priv->domain->domain,
+ rkt_obj->mm.start, rkt_obj->size);
+
err_remove_node:
mutex_lock(&rocket_priv->mm_lock);
drm_mm_remove_node(&rkt_obj->mm);
--
2.54.0
