Forwarded: [PATCH] drm/gem: fix use-after-free in drm_gem_release

[syzbot <syzbot+b2e951687503f32f74ce@syzkaller.appspotmail.com>]

For archival purposes, forwarding an incoming command email to
linux-kernel@vger.kernel.org, syzkaller-bugs@googlegroups.com.

***

Subject: [PATCH] drm/gem: fix use-after-free in drm_gem_release
Author: kartikey406@gmail.com

#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master


KASAN reported a use-after-free in drm_gem_object_release_handle
when drm_gem_release() races with drm_gem_handle_delete().

drm_gem_release() calls idr_for_each() which gives out object
pointers, but a concurrent drm_gem_handle_delete() can free the
object before drm_gem_object_release_handle() gets to use it.

Fix this by splitting the operation into two phases:

Phase 1: Walk the IDR under table_lock. For each object, atomically
replace its IDR slot with NULL using idr_replace() so that a
concurrent drm_gem_handle_delete() sees NULL and bails out with
-EINVAL. Grab a reference on the object and collect it into a local
list. Only atomic operations are performed here so holding a
spinlock is safe.

Phase 2: Outside the lock, iterate the local list and call
drm_gem_object_release_handle() on each object safely. Drop our
reference afterwards. Destroy the IDR only after all objects have
been properly released.

The idr_replace(NULL) in phase 1 is the key: it mirrors what
drm_gem_handle_delete() itself does to atomically claim ownership
of an object, ensuring that each object is processed by exactly
one path.

Reported-by: syzbot+b2e951687503f32f74ce@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=b2e951687503f32f74ce
Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
---
 drivers/gpu/drm/drm_gem.c | 49 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 48 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 891c3bff5ae0..9ec1f03c6383 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -1073,6 +1073,37 @@ drm_gem_open(struct drm_device *dev, struct drm_file *file_private)
 	spin_lock_init(&file_private->table_lock);
 }
 
+struct drm_gem_object_entry {
+	struct drm_gem_object *obj;
+	struct list_head head;
+};
+
+struct drm_gem_collect_data {
+	struct drm_file    *file_priv;
+	struct list_head   list;
+};
+
+static int drm_gem_object_collect(int id, void *ptr, void *data)
+{
+	struct drm_gem_collect_data *cd = data;
+	struct drm_gem_object_entry *entry;
+	struct drm_gem_object *obj;
+
+	/* Atomically claim the slot → handle_delete sees NULL, bails */
+	obj = idr_replace(&cd->file_priv->object_idr, NULL, id);
+	if (IS_ERR_OR_NULL(obj))
+		return 0;
+
+	entry = kmalloc(sizeof(*entry), GFP_ATOMIC);
+	if (!entry)
+		return -ENOMEM;
+
+	drm_gem_object_get(obj);
+	entry->obj = obj;
+	list_add(&entry->head, &cd->list);
+	return 0;
+}
+
 /**
  * drm_gem_release - release file-private GEM resources
  * @dev: drm_device which is being closed by userspace
@@ -1085,8 +1116,24 @@ drm_gem_open(struct drm_device *dev, struct drm_file *file_private)
 void
 drm_gem_release(struct drm_device *dev, struct drm_file *file_private)
 {
+	struct drm_gem_object_entry *entry, *tmp;
+	struct drm_gem_collect_data cd = {
+		.file_priv = file_private,
+	};
+	INIT_LIST_HEAD(&cd.list);
+
+	spin_lock(&file_private->table_lock);
 	idr_for_each(&file_private->object_idr,
-		     &drm_gem_object_release_handle, file_private);
+		     &drm_gem_object_collect, &cd);
+	spin_unlock(&file_private->table_lock);
+
+	list_for_each_entry_safe(entry, tmp, &cd.list, head) {
+		list_del(&entry->head);
+		drm_gem_object_release_handle(0, entry->obj, file_private);
+		drm_gem_object_put(entry->obj);
+		kfree(entry);
+	}
+
 	idr_destroy(&file_private->object_idr);
 }
 
-- 
2.43.0