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

[Kieran Bingham]

Hi Thomas,

On 15/01/2021 09:30, 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 already led to a memory
> leak 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.
> 
> v2:
> 	* support (vm_ops == NULL) if mmap is given; required by VRAM
> 	  helpers
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
> Fixes: f5ca8eb6f9bd ("drm/cma-helper: Implement mmap as GEM CMA object functions")
> Reported-by: Kieran Bingham <kieran.bingham+renesas at ideasonboard.com>
> Tested-by: Kieran Bingham <kieran.bingham+renesas at ideasonboard.com>

Re-tested just fine this side ;-)
 - https://paste.ubuntu.com/p/Jgz6xMKNJX/

Thanks

Kieran

> Cc: Maxime Ripard <mripard at kernel.org>
> Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> Cc: David Airlie <airlied at linux.ie>
> Cc: Daniel Vetter <daniel at ffwll.ch>
> Cc: Eric Anholt <eric at anholt.net>
> Cc: dri-devel at lists.freedesktop.org
> ---
>  drivers/gpu/drm/drm_gem.c   | 19 ++++++++++---------
>  drivers/gpu/drm/drm_prime.c |  2 ++
>  2 files changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index 34b2f111c01c..c2ce78c4edc3 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -1068,20 +1068,17 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size,
>  	drm_gem_object_get(obj);
>  
>  	vma->vm_private_data = obj;
> +	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;
> +		if (!vma->vm_ops) {
> +			ret = -EINVAL;
> +			goto err_drm_gem_object_put;
>  		}
>  
>  		vma->vm_flags |= VM_IO | VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP;
> @@ -1090,6 +1087,10 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size,
>  	}
>  
>  	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..2a54f86856af 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -717,6 +717,8 @@ 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) {
> +		vma->vm_ops = obj->funcs->vm_ops;
> +
>  		ret = obj->funcs->mmap(obj, vma);
>  		if (ret)
>  			return ret;
>