Re: [PATCH] drm: Set vm_ops to GEM object's values during mmap

[Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>]

Hi Thomas,

On 15/01/2021 08:39, Thomas Zimmermann wrote:
> The GEM mmap code relies on the GEM object's mmap callback to set the
> VMA's vm_ops field. This is easily forgotten and lead to a memory leak

 s/lead/can lead/

> in the CMA helpers. Instead set the vm_ops field in the DRM core code
> to the GEM object's value. Drivers with different needs can override
> this in their mmap callback.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Fixes: f5ca8eb6f9bd ("drm/cma-helper: Implement mmap as GEM CMA object functions")
> Reported-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

This also works here.
 - https://paste.ubuntu.com/p/S2Q586rgwT/

Tested-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Eric Anholt <eric@anholt.net>
> Cc: dri-devel@lists.freedesktop.org
> ---
>  drivers/gpu/drm/drm_gem.c   | 23 ++++++++++++-----------
>  drivers/gpu/drm/drm_prime.c |  4 ++++
>  2 files changed, 16 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index 34b2f111c01c..54d95621fcbb 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -1069,27 +1069,28 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size,
>  
>  	vma->vm_private_data = obj;
>  
> +	if (!obj->funcs->vm_ops) {
> +		ret = -EINVAL;
> +		goto err_drm_gem_object_put;
> +	}
> +	vma->vm_ops = obj->funcs->vm_ops;
> +
>  	if (obj->funcs->mmap) {
>  		ret = obj->funcs->mmap(obj, vma);
> -		if (ret) {
> -			drm_gem_object_put(obj);
> -			return ret;
> -		}
> +		if (ret)
> +			goto err_drm_gem_object_put;
>  		WARN_ON(!(vma->vm_flags & VM_DONTEXPAND));
>  	} else {
> -		if (obj->funcs->vm_ops)
> -			vma->vm_ops = obj->funcs->vm_ops;
> -		else {
> -			drm_gem_object_put(obj);
> -			return -EINVAL;
> -		}
> -
>  		vma->vm_flags |= VM_IO | VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP;
>  		vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
>  		vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
>  	}
>  
>  	return 0;
> +
> +err_drm_gem_object_put:
> +	drm_gem_object_put(obj);
> +	return ret;
>  }
>  EXPORT_SYMBOL(drm_gem_mmap_obj);
>  
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index 683aa29ecd3b..0549ec190583 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -717,6 +717,10 @@ int drm_gem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma)
>  	vma->vm_pgoff += drm_vma_node_start(&obj->vma_node);
>  
>  	if (obj->funcs && obj->funcs->mmap) {
> +		if (!obj->funcs->vm_ops)
> +			return -EINVAL;
> +		vma->vm_ops = obj->funcs->vm_ops;
> +
>  		ret = obj->funcs->mmap(obj, vma);
>  		if (ret)
>  			return ret;
> 

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