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 <[email protected]>
> Fixes: f5ca8eb6f9bd ("drm/cma-helper: Implement mmap as GEM CMA object 
> functions")
> Reported-by: Kieran Bingham <[email protected]>

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

Tested-by: Kieran Bingham <[email protected]>

> Cc: Maxime Ripard <[email protected]>
> Cc: Maarten Lankhorst <[email protected]>
> Cc: David Airlie <[email protected]>
> Cc: Daniel Vetter <[email protected]>
> Cc: Eric Anholt <[email protected]>
> Cc: [email protected]
> ---
>  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
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to