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 <[email protected]>
Cc: Hillf Danton <[email protected]>
Cc: Reported-by: [email protected]
Cc: [email protected]
Cc: Emil Velikov <[email protected]>
Cc: Daniel Vetter <[email protected]>
Cc: Sean Paul <[email protected]>
Cc: Chris Wilson <[email protected]>
Cc: Eric Anholt <[email protected]>
Cc: Sam Ravnborg <[email protected]>
Cc: Rob Clark <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
---
 drivers/gpu/drm/vgem/vgem_drv.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
index 5bd60ded3d81..909eba43664a 100644
--- a/drivers/gpu/drm/vgem/vgem_drv.c
+++ b/drivers/gpu/drm/vgem/vgem_drv.c
@@ -196,9 +196,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;
 }
@@ -221,7 +222,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("Created object of size %lld\n", size);
+       drm_gem_object_put_unlocked(gem_object);
+
+       DRM_DEBUG("Created object of size %llu\n", args->size);
 
        return 0;
 }
-- 
2.24.1

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to