| Message ID | 20200427214405.13069-1-ezequiel@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/vkms: Hold gem object while still in-use | expand |
Thanks for your patch! Reviewed-by: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com> On 04/27, Ezequiel Garcia wrote: > We need to keep the reference to the drm_gem_object > until the last access by vkms_dumb_create. > > Therefore, the put the object after it is used. > > This fixes a use-after-free issue reported by syzbot. > > While here, change vkms_gem_create() symbol to static. > > Reported-and-tested-by: syzbot+e3372a2afe1e7ef04bc7@syzkaller.appspotmail.com > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com> > --- > drivers/gpu/drm/vkms/vkms_drv.h | 5 ----- > drivers/gpu/drm/vkms/vkms_gem.c | 11 ++++++----- > 2 files changed, 6 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h > index eda04ffba7b1..f4036bb0b9a8 100644 > --- a/drivers/gpu/drm/vkms/vkms_drv.h > +++ b/drivers/gpu/drm/vkms/vkms_drv.h > @@ -117,11 +117,6 @@ struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev, > enum drm_plane_type type, int index); > > /* Gem stuff */ > -struct drm_gem_object *vkms_gem_create(struct drm_device *dev, > - struct drm_file *file, > - u32 *handle, > - u64 size); > - > vm_fault_t vkms_gem_fault(struct vm_fault *vmf); > > int vkms_dumb_create(struct drm_file *file, struct drm_device *dev, > diff --git a/drivers/gpu/drm/vkms/vkms_gem.c b/drivers/gpu/drm/vkms/vkms_gem.c > index 2e01186fb943..c541fec57566 100644 > --- a/drivers/gpu/drm/vkms/vkms_gem.c > +++ b/drivers/gpu/drm/vkms/vkms_gem.c > @@ -97,10 +97,10 @@ vm_fault_t vkms_gem_fault(struct vm_fault *vmf) > return ret; > } > > -struct drm_gem_object *vkms_gem_create(struct drm_device *dev, > - struct drm_file *file, > - u32 *handle, > - u64 size) > +static struct drm_gem_object *vkms_gem_create(struct drm_device *dev, > + struct drm_file *file, > + u32 *handle, > + u64 size) > { > struct vkms_gem_object *obj; > int ret; > @@ -113,7 +113,6 @@ struct drm_gem_object *vkms_gem_create(struct drm_device *dev, > return ERR_CAST(obj); > > ret = drm_gem_handle_create(file, &obj->gem, handle); > - drm_gem_object_put_unlocked(&obj->gem); > if (ret) > return ERR_PTR(ret); > > @@ -142,6 +141,8 @@ int vkms_dumb_create(struct drm_file *file, struct drm_device *dev, > args->size = gem_obj->size; > args->pitch = pitch; > > + drm_gem_object_put_unlocked(gem_obj); > + > DRM_DEBUG_DRIVER("Created object of size %lld\n", size); > > return 0; > -- > 2.26.0.rc2 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org
Applied to drm-misc-next. On 05/06, Rodrigo Siqueira wrote: > Thanks for your patch! > > Reviewed-by: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com> > > On 04/27, Ezequiel Garcia wrote: > > We need to keep the reference to the drm_gem_object > > until the last access by vkms_dumb_create. > > > > Therefore, the put the object after it is used. > > > > This fixes a use-after-free issue reported by syzbot. > > > > While here, change vkms_gem_create() symbol to static. > > > > Reported-and-tested-by: syzbot+e3372a2afe1e7ef04bc7@syzkaller.appspotmail.com > > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com> > > --- > > drivers/gpu/drm/vkms/vkms_drv.h | 5 ----- > > drivers/gpu/drm/vkms/vkms_gem.c | 11 ++++++----- > > 2 files changed, 6 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h > > index eda04ffba7b1..f4036bb0b9a8 100644 > > --- a/drivers/gpu/drm/vkms/vkms_drv.h > > +++ b/drivers/gpu/drm/vkms/vkms_drv.h > > @@ -117,11 +117,6 @@ struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev, > > enum drm_plane_type type, int index); > > > > /* Gem stuff */ > > -struct drm_gem_object *vkms_gem_create(struct drm_device *dev, > > - struct drm_file *file, > > - u32 *handle, > > - u64 size); > > - > > vm_fault_t vkms_gem_fault(struct vm_fault *vmf); > > > > int vkms_dumb_create(struct drm_file *file, struct drm_device *dev, > > diff --git a/drivers/gpu/drm/vkms/vkms_gem.c b/drivers/gpu/drm/vkms/vkms_gem.c > > index 2e01186fb943..c541fec57566 100644 > > --- a/drivers/gpu/drm/vkms/vkms_gem.c > > +++ b/drivers/gpu/drm/vkms/vkms_gem.c > > @@ -97,10 +97,10 @@ vm_fault_t vkms_gem_fault(struct vm_fault *vmf) > > return ret; > > } > > > > -struct drm_gem_object *vkms_gem_create(struct drm_device *dev, > > - struct drm_file *file, > > - u32 *handle, > > - u64 size) > > +static struct drm_gem_object *vkms_gem_create(struct drm_device *dev, > > + struct drm_file *file, > > + u32 *handle, > > + u64 size) > > { > > struct vkms_gem_object *obj; > > int ret; > > @@ -113,7 +113,6 @@ struct drm_gem_object *vkms_gem_create(struct drm_device *dev, > > return ERR_CAST(obj); > > > > ret = drm_gem_handle_create(file, &obj->gem, handle); > > - drm_gem_object_put_unlocked(&obj->gem); > > if (ret) > > return ERR_PTR(ret); > > > > @@ -142,6 +141,8 @@ int vkms_dumb_create(struct drm_file *file, struct drm_device *dev, > > args->size = gem_obj->size; > > args->pitch = pitch; > > > > + drm_gem_object_put_unlocked(gem_obj); > > + > > DRM_DEBUG_DRIVER("Created object of size %lld\n", size); > > > > return 0; > > -- > > 2.26.0.rc2 > > > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > -- > Rodrigo Siqueira > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsiqueira.tech%2F&data=02%7C01%7CRodrigo.Siqueira%40amd.com%7C087365fcd8794562fb4c08d7f22899e6%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637244129267902564&sdata=855ahjfgdqouFFFn98OKtmCHf0ESNINBsGHUoBz6fWg%3D&reserved=0 > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&data=02%7C01%7CRodrigo.Siqueira%40amd.com%7C087365fcd8794562fb4c08d7f22899e6%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637244129267912520&sdata=MqNXl25%2B6DYRTGgIOSit74dmAd%2Fkx42bw2e3Iut2rMQ%3D&reserved=0
diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h index eda04ffba7b1..f4036bb0b9a8 100644 --- a/drivers/gpu/drm/vkms/vkms_drv.h +++ b/drivers/gpu/drm/vkms/vkms_drv.h @@ -117,11 +117,6 @@ struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev, enum drm_plane_type type, int index); /* Gem stuff */ -struct drm_gem_object *vkms_gem_create(struct drm_device *dev, - struct drm_file *file, - u32 *handle, - u64 size); - vm_fault_t vkms_gem_fault(struct vm_fault *vmf); int vkms_dumb_create(struct drm_file *file, struct drm_device *dev, diff --git a/drivers/gpu/drm/vkms/vkms_gem.c b/drivers/gpu/drm/vkms/vkms_gem.c index 2e01186fb943..c541fec57566 100644 --- a/drivers/gpu/drm/vkms/vkms_gem.c +++ b/drivers/gpu/drm/vkms/vkms_gem.c @@ -97,10 +97,10 @@ vm_fault_t vkms_gem_fault(struct vm_fault *vmf) return ret; } -struct drm_gem_object *vkms_gem_create(struct drm_device *dev, - struct drm_file *file, - u32 *handle, - u64 size) +static struct drm_gem_object *vkms_gem_create(struct drm_device *dev, + struct drm_file *file, + u32 *handle, + u64 size) { struct vkms_gem_object *obj; int ret; @@ -113,7 +113,6 @@ struct drm_gem_object *vkms_gem_create(struct drm_device *dev, return ERR_CAST(obj); ret = drm_gem_handle_create(file, &obj->gem, handle); - drm_gem_object_put_unlocked(&obj->gem); if (ret) return ERR_PTR(ret); @@ -142,6 +141,8 @@ int vkms_dumb_create(struct drm_file *file, struct drm_device *dev, args->size = gem_obj->size; args->pitch = pitch; + drm_gem_object_put_unlocked(gem_obj); + DRM_DEBUG_DRIVER("Created object of size %lld\n", size); return 0;
We need to keep the reference to the drm_gem_object until the last access by vkms_dumb_create. Therefore, the put the object after it is used. This fixes a use-after-free issue reported by syzbot. While here, change vkms_gem_create() symbol to static. Reported-and-tested-by: syzbot+e3372a2afe1e7ef04bc7@syzkaller.appspotmail.com Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com> --- drivers/gpu/drm/vkms/vkms_drv.h | 5 ----- drivers/gpu/drm/vkms/vkms_gem.c | 11 ++++++----- 2 files changed, 6 insertions(+), 10 deletions(-)