Hi Daniel. On Sun, Feb 02, 2020 at 02:21:33PM +0100, Daniel Vetter wrote: > 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] ^^ Small typo
> 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 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
