drm/gem: Make drm_gem_objects_lookup() self-cleaning on failure v6

drm_gem_objects_lookup() can allocate the output array and take
references on GEM objects before it fails.

If an error happens part-way through, callers previously had to clean up
partially created results themselves. This relied on subtle and
undocumented behavior and was easy to get wrong.

Make drm_gem_objects_lookup() clean up on failure. The function now
drops any references it already took, frees the array, and sets
*objs_out to NULL before returning an error.

On success, behavior is unchanged. Existing callers remain correct and
their error cleanup paths simply do nothing when *objs_out is NULL.

v2/v3: Move partial-lookup cleanup into objects_lookup(), perform
reference dropping outside the lock, and remove reliance on __GFP_ZERO
or implicit NULL handling. (Christian)

v4: Use goto-style error handling in objects_lookup(), drop partial
references outside the lock, and simplify drm_gem_objects_lookup()
cleanup by routing failures through err_free_handles as suggested.
(Christian)

v5: Rebase on drm-misc-next, drop the ret local variable. (Christian)

v6: Drop superfluous initialization of handles. (Christian/Tvrtko)

Cc: Alex Deucher <alexander.deucher@amd.com>
Suggested-by: Christian König <christian.koenig@amd.com>
Suggested-by: Tvrtko Ursulin <tursulin@ursulin.net>
Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
Link: https://patch.msgid.link/20260206132141.1474191-1-srinivasan.shanmugam@amd.com

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 7ff6b7bbeb73b804532a6989acde39e679127a4a..c4a3de3b920e6ba2d46959d7688cef13e1dd18f2 100644 (file)
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -783,7 +783,7 @@ EXPORT_SYMBOL(drm_gem_put_pages);
 static int objects_lookup(struct drm_file *filp, u32 *handle, int count,
                          struct drm_gem_object **objs)
 {
-       int i, ret = 0;
+       int i;
        struct drm_gem_object *obj;
 
        spin_lock(&filp->table_lock);
@@ -791,16 +791,23 @@ static int objects_lookup(struct drm_file *filp, u32 *handle, int count,
        for (i = 0; i < count; i++) {
                /* Check if we currently have a reference on the object */
                obj = idr_find(&filp->object_idr, handle[i]);
-               if (!obj) {
-                       ret = -ENOENT;
-                       break;
-               }
+               if (!obj)
+                       goto err;
+
                drm_gem_object_get(obj);
                objs[i] = obj;
        }
+
        spin_unlock(&filp->table_lock);
+       return 0;
 
-       return ret;
+err:
+       spin_unlock(&filp->table_lock);
+
+       while (i--)
+               drm_gem_object_put(objs[i]);
+
+       return -ENOENT;
 }
 
 /**
@@ -828,24 +835,34 @@ int drm_gem_objects_lookup(struct drm_file *filp, void __user *bo_handles,
        u32 *handles;
        int ret;
 
+       *objs_out = NULL;
+
        if (!count)
                return 0;
 
-       objs = kvmalloc_array(count, sizeof(struct drm_gem_object *),
-                            GFP_KERNEL | __GFP_ZERO);
+       objs = kvmalloc_array(count, sizeof(*objs), GFP_KERNEL);
        if (!objs)
                return -ENOMEM;
 
-       *objs_out = objs;
-
        handles = vmemdup_array_user(bo_handles, count, sizeof(u32));
-       if (IS_ERR(handles))
-               return PTR_ERR(handles);
+       if (IS_ERR(handles)) {
+               ret = PTR_ERR(handles);
+               goto err_free_objs;
+       }
 
        ret = objects_lookup(filp, handles, count, objs);
+       if (ret)
+               goto err_free_handles;
+
        kvfree(handles);
-       return ret;
+       *objs_out = objs;
+       return 0;
 
+err_free_handles:
+       kvfree(handles);
+err_free_objs:
+       kvfree(objs);
+       return ret;
 }
 EXPORT_SYMBOL(drm_gem_objects_lookup);