[PATCH] drm/vkms: Hold gem object while still in-use

[PATCH] drm/vkms: Hold gem object while still in-use

[Ezequiel Garcia]

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
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/vkms: Hold gem object while still in-use

[Rodrigo Siqueira]

[-- Attachment #1.1: Type: text/plain, Size: 2791 bytes --]

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://siqueira.tech

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/vkms: Hold gem object while still in-use

[Rodrigo Siqueira]

[-- Attachment #1.1: Type: text/plain, Size: 3812 bytes --]

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&amp;data=02%7C01%7CRodrigo.Siqueira%40amd.com%7C087365fcd8794562fb4c08d7f22899e6%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637244129267902564&amp;sdata=855ahjfgdqouFFFn98OKtmCHf0ESNINBsGHUoBz6fWg%3D&amp;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&amp;data=02%7C01%7CRodrigo.Siqueira%40amd.com%7C087365fcd8794562fb4c08d7f22899e6%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637244129267912520&amp;sdata=MqNXl25%2B6DYRTGgIOSit74dmAd%2Fkx42bw2e3Iut2rMQ%3D&amp;reserved=0


-- 
Rodrigo Siqueira
https://siqueira.tech

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel