diff mbox series

[4.19,1/1] drm/vgem: Close use-after-free race in vgem_gem_create

Message ID 20220502113857.2126299-1-ovidiu.panait@windriver.com
State New
Headers show
Series [4.19,1/1] drm/vgem: Close use-after-free race in vgem_gem_create | expand

Commit Message

Ovidiu Panait May 2, 2022, 11:38 a.m. UTC 
From: Daniel Vetter <daniel.vetter@ffwll.ch>

commit 4b848f20eda5974020f043ca14bacf7a7e634fc8 upstream.

There's two references floating around here (for the object reference,
not the handle_count reference, that's a different thing):

- The temporary reference held by vgem_gem_create, acquired by
  creating the object and released by calling
  drm_gem_object_put_unlocked.

- The reference held by the object handle, created by
  drm_gem_handle_create. This one generally outlives the function,
  except if a 2nd thread races with a GEM_CLOSE ioctl call.

So usually everything is correct, except in that race case, where the
access to gem_object->size could be looking at freed data already.
Which again isn't a real problem (userspace shot its feet off already
with the race, we could return garbage), but maybe someone can exploit
this as an information leak.

Cc: Dan Carpenter <dan.carpenter@oracle.com>
Cc: Hillf Danton <hdanton@sina.com>
Reported-by: syzbot+0dc4444774d419e916c8@syzkaller.appspotmail.com
Cc: stable@vger.kernel.org
Cc: Emil Velikov <emil.velikov@collabora.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Sean Paul <seanpaul@chromium.org>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Eric Anholt <eric@anholt.net>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Rob Clark <robdclark@chromium.org>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200202132133.1891846-1-daniel.vetter@ffwll.ch
[OP: backport to 4.19: adjusted DRM_DEBUG() -> DRM_DEBUG_DRIVER()]
Signed-off-by: Ovidiu Panait <ovidiu.panait@windriver.com>
---
This is the fix for CVE-2022-1419.

The testcase from [1] adjusted to run in a while(1); loop generates the
following KASAN trace currently:
root@intel-x86-64:~# ./poc
[   37.663495] ==================================================================
[   37.664441] BUG: KASAN: use-after-free in vgem_gem_dumb_create+0x22c/0x240
[   37.665266] Read of size 8 at addr ffff88810b27b380 by task poc/5993
[   37.666047] 
[   37.666257] CPU: 1 PID: 5993 Comm: poc Not tainted 4.19.241 #7
[   37.666937] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014
[   37.668034] Call Trace:
[   37.668360]  dump_stack+0x188/0x1ff
[   37.668804]  ? vgem_gem_dumb_create+0x22c/0x240
[   37.669376]  print_address_description.cold+0x7c/0x212
[   37.670004]  ? vgem_gem_dumb_create+0x22c/0x240
[   37.670553]  kasan_report.cold+0x88/0x2af
[   37.671059]  vgem_gem_dumb_create+0x22c/0x240
[   37.671602]  drm_mode_create_dumb+0x27e/0x310
[   37.672153]  drm_ioctl_kernel+0x1f1/0x2a0
[   37.672647]  ? drm_mode_create_dumb+0x310/0x310
[   37.673199]  ? drm_setversion+0x880/0x880
[   37.673695]  ? __might_fault+0x192/0x1d0
[   37.674173]  drm_ioctl+0x4e4/0x9a0
[   37.674631]  ? drm_mode_create_dumb+0x310/0x310
[   37.675199]  ? drm_version+0x3d0/0x3d0
[   37.675659]  ? pud_val+0xf0/0xf0
[   37.676084]  ? find_held_lock+0x2d/0x110
[   37.676557]  ? __fget+0x341/0x500
[   37.676975]  ? drm_version+0x3d0/0x3d0
[   37.677443]  do_vfs_ioctl+0xcdb/0x12e0
[   37.677907]  ? check_preemption_disabled+0x41/0x280
[   37.678508]  ? ioctl_preallocate+0x200/0x200
[   37.679021]  ? __fget+0x368/0x500
[   37.679420]  ? ksys_dup3+0x3c0/0x3c0
[   37.679859]  ? syscall_trace_enter+0x3b8/0xd50
[   37.680387]  ? syscall_get_arguments.part.0+0x10/0x10
[   37.680980]  ksys_ioctl+0x9b/0xc0
[   37.681375]  __x64_sys_ioctl+0x6f/0xb0
[   37.681826]  do_syscall_64+0xf0/0x180
[   37.682275]  entry_SYSCALL_64_after_hwframe+0x49/0xbe
[   37.682876] RIP: 0033:0x7fb3d3c0ac87

With the fix applied, the testcase is OK.

[1] https://www.openwall.com/lists/oss-security/2022/04/21/1

 drivers/gpu/drm/vgem/vgem_drv.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)
diff mbox series

Patch

diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
index 1c1a435d354b..56ed771032c2 100644
--- a/drivers/gpu/drm/vgem/vgem_drv.c
+++ b/drivers/gpu/drm/vgem/vgem_drv.c
@@ -189,9 +189,10 @@  static struct drm_gem_object *vgem_gem_create(struct drm_device *dev,
 		return ERR_CAST(obj);
 
 	ret = drm_gem_handle_create(file, &obj->base, handle);
-	drm_gem_object_put_unlocked(&obj->base);
-	if (ret)
+	if (ret) {
+		drm_gem_object_put_unlocked(&obj->base);
 		return ERR_PTR(ret);
+	}
 
 	return &obj->base;
 }
@@ -214,7 +215,9 @@  static int vgem_gem_dumb_create(struct drm_file *file, struct drm_device *dev,
 	args->size = gem_object->size;
 	args->pitch = pitch;
 
-	DRM_DEBUG_DRIVER("Created object of size %lld\n", size);
+	drm_gem_object_put_unlocked(gem_object);
+
+	DRM_DEBUG_DRIVER("Created object of size %llu\n", args->size);
 
 	return 0;
 }