drm/panfrost: Fix GEM handle creation ref-counting

[ Upstream commit 4217c6ac817451d5116687f3cc6286220dc43d49 ]

panfrost_gem_create_with_handle() previously returned a BO but with the
only reference being from the handle, which user space could in theory
guess and release, causing a use-after-free. Additionally if the call to
panfrost_gem_mapping_get() in panfrost_ioctl_create_bo() failed then
a(nother) reference on the BO was dropped.

The _create_with_handle() is a problematic pattern, so ditch it and
instead create the handle in panfrost_ioctl_create_bo(). If the call to
panfrost_gem_mapping_get() fails then this means that user space has
indeed gone behind our back and freed the handle. In which case just
return an error code.

Reported-by: Rob Clark <robdclark@chromium.org>
Fixes: f3ba91228e8e ("drm/panfrost: Add initial panfrost driver")
Signed-off-by: Steven Price <steven.price@arm.com>
Reviewed-by: Rob Clark <robdclark@gmail.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20221219140130.410578-1-steven.price@arm.com
Signed-off-by: Sasha Levin <sashal@kernel.org>

diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c
index 2fa5afe..919e6cc 100644 (file)
--- a/drivers/gpu/drm/panfrost/panfrost_drv.c
+++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
@@ -82,6 +82,7 @@ static int panfrost_ioctl_create_bo(struct drm_device *dev, void *data,
        struct panfrost_gem_object *bo;
        struct drm_panfrost_create_bo *args = data;
        struct panfrost_gem_mapping *mapping;
+       int ret;
 
        if (!args->size || args->pad ||
            (args->flags & ~(PANFROST_BO_NOEXEC | PANFROST_BO_HEAP)))
@@ -92,21 +93,29 @@ static int panfrost_ioctl_create_bo(struct drm_device *dev, void *data,
            !(args->flags & PANFROST_BO_NOEXEC))
                return -EINVAL;
 
-       bo = panfrost_gem_create_with_handle(file, dev, args->size, args->flags,
-                                            &args->handle);
+       bo = panfrost_gem_create(dev, args->size, args->flags);
        if (IS_ERR(bo))
                return PTR_ERR(bo);
 
+       ret = drm_gem_handle_create(file, &bo->base.base, &args->handle);
+       if (ret)
+               goto out;
+
        mapping = panfrost_gem_mapping_get(bo, priv);
-       if (!mapping) {
-               drm_gem_object_put(&bo->base.base);
-               return -EINVAL;
+       if (mapping) {
+               args->offset = mapping->mmnode.start << PAGE_SHIFT;
+               panfrost_gem_mapping_put(mapping);
+       } else {
+               /* This can only happen if the handle from
+                * drm_gem_handle_create() has already been guessed and freed
+                * by user space
+                */
+               ret = -EINVAL;
        }
 
-       args->offset = mapping->mmnode.start << PAGE_SHIFT;
-       panfrost_gem_mapping_put(mapping);
-
-       return 0;
+out:
+       drm_gem_object_put(&bo->base.base);
+       return ret;
 }
 
 /**

diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c b/drivers/gpu/drm/panfrost/panfrost_gem.c
index 293e799..3c812fb 100644 (file)
--- a/drivers/gpu/drm/panfrost/panfrost_gem.c
+++ b/drivers/gpu/drm/panfrost/panfrost_gem.c
@@ -235,12 +235,8 @@ struct drm_gem_object *panfrost_gem_create_object(struct drm_device *dev, size_t
 }
 
 struct panfrost_gem_object *
-panfrost_gem_create_with_handle(struct drm_file *file_priv,
-                               struct drm_device *dev, size_t size,
-                               u32 flags,
-                               uint32_t *handle)
+panfrost_gem_create(struct drm_device *dev, size_t size, u32 flags)
 {
-       int ret;
        struct drm_gem_shmem_object *shmem;
        struct panfrost_gem_object *bo;
 
@@ -256,16 +252,6 @@ panfrost_gem_create_with_handle(struct drm_file *file_priv,
        bo->noexec = !!(flags & PANFROST_BO_NOEXEC);
        bo->is_heap = !!(flags & PANFROST_BO_HEAP);
 
-       /*
-        * Allocate an id of idr table where the obj is registered
-        * and handle has the id what user can see.
-        */
-       ret = drm_gem_handle_create(file_priv, &shmem->base, handle);
-       /* drop reference from allocate - handle holds it now. */
-       drm_gem_object_put(&shmem->base);
-       if (ret)
-               return ERR_PTR(ret);
-
        return bo;
 }
 

diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.h b/drivers/gpu/drm/panfrost/panfrost_gem.h
index 8088d5f..ad2877e 100644 (file)
--- a/drivers/gpu/drm/panfrost/panfrost_gem.h
+++ b/drivers/gpu/drm/panfrost/panfrost_gem.h
@@ -69,10 +69,7 @@ panfrost_gem_prime_import_sg_table(struct drm_device *dev,
                                   struct sg_table *sgt);
 
 struct panfrost_gem_object *
-panfrost_gem_create_with_handle(struct drm_file *file_priv,
-                               struct drm_device *dev, size_t size,
-                               u32 flags,
-                               uint32_t *handle);
+panfrost_gem_create(struct drm_device *dev, size_t size, u32 flags);
 
 int panfrost_gem_open(struct drm_gem_object *obj, struct drm_file *file_priv);
 void panfrost_gem_close(struct drm_gem_object *obj,