[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20220510130734.108704531@linuxfoundation.org>
Date: Tue, 10 May 2022 15:07:39 +0200
From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To: linux-kernel@...r.kernel.org
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
stable@...r.kernel.org, Dan Carpenter <dan.carpenter@...cle.com>,
Hillf Danton <hdanton@...a.com>,
syzbot+0dc4444774d419e916c8@...kaller.appspotmail.com,
Emil Velikov <emil.velikov@...labora.com>,
Daniel Vetter <daniel.vetter@...ll.ch>,
Sean Paul <seanpaul@...omium.org>,
Chris Wilson <chris@...is-wilson.co.uk>,
Eric Anholt <eric@...olt.net>, Sam Ravnborg <sam@...nborg.org>,
Rob Clark <robdclark@...omium.org>,
Daniel Vetter <daniel.vetter@...el.com>,
Ovidiu Panait <ovidiu.panait@...driver.com>
Subject: [PATCH 4.14 53/78] drm/vgem: Close use-after-free race in vgem_gem_create
From: Daniel Vetter <daniel.vetter@...ll.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@...cle.com>
Cc: Hillf Danton <hdanton@...a.com>
Reported-by: syzbot+0dc4444774d419e916c8@...kaller.appspotmail.com
Cc: stable@...r.kernel.org
Cc: Emil Velikov <emil.velikov@...labora.com>
Cc: Daniel Vetter <daniel.vetter@...ll.ch>
Cc: Sean Paul <seanpaul@...omium.org>
Cc: Chris Wilson <chris@...is-wilson.co.uk>
Cc: Eric Anholt <eric@...olt.net>
Cc: Sam Ravnborg <sam@...nborg.org>
Cc: Rob Clark <robdclark@...omium.org>
Reviewed-by: Chris Wilson <chris@...is-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@...el.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@...driver.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
---
drivers/gpu/drm/vgem/vgem_drv.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
--- a/drivers/gpu/drm/vgem/vgem_drv.c
+++ b/drivers/gpu/drm/vgem/vgem_drv.c
@@ -190,9 +190,10 @@ static struct drm_gem_object *vgem_gem_c
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;
}
@@ -215,7 +216,9 @@ static int vgem_gem_dumb_create(struct d
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;
}
Powered by blists - more mailing lists