Re: [PATCH] drm/msm: Ensure mmap offset is initialized

[Dmitry Baryshkov]

On Tue, 31 May 2022 at 23:08, Rob Clark <robdclark@xxxxxxxxx> wrote:
>
> From: Rob Clark <robdclark@xxxxxxxxxxxx>
>
> If a GEM object is allocated, and then exported as a dma-buf fd which is
> mmap'd before or without the GEM buffer being directly mmap'd, the
> vma_node could be unitialized.  This leads to a situation where the CPU
> mapping is not correctly torn down in drm_vma_node_unmap().
>
> Fixes: e5516553999f ("drm: call drm_gem_object_funcs.mmap with fake offset")
> Signed-off-by: Rob Clark <robdclark@xxxxxxxxxxxx>
> ---
>  drivers/gpu/drm/msm/msm_drv.c       |  2 +-
>  drivers/gpu/drm/msm/msm_drv.h       |  1 +
>  drivers/gpu/drm/msm/msm_gem_prime.c | 15 +++++++++++++++
>  3 files changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index 44485363f37a..14ab9a627d8b 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -964,7 +964,7 @@ static const struct drm_driver msm_driver = {
>         .prime_handle_to_fd = drm_gem_prime_handle_to_fd,
>         .prime_fd_to_handle = drm_gem_prime_fd_to_handle,
>         .gem_prime_import_sg_table = msm_gem_prime_import_sg_table,
> -       .gem_prime_mmap     = drm_gem_prime_mmap,
> +       .gem_prime_mmap     = msm_gem_prime_mmap,
>  #ifdef CONFIG_DEBUG_FS
>         .debugfs_init       = msm_debugfs_init,
>  #endif
> diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
> index bb052071b16d..090b8074fec7 100644
> --- a/drivers/gpu/drm/msm/msm_drv.h
> +++ b/drivers/gpu/drm/msm/msm_drv.h
> @@ -275,6 +275,7 @@ unsigned long msm_gem_shrinker_shrink(struct drm_device *dev, unsigned long nr_t
>  void msm_gem_shrinker_init(struct drm_device *dev);
>  void msm_gem_shrinker_cleanup(struct drm_device *dev);
>
> +int msm_gem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma);
>  struct sg_table *msm_gem_prime_get_sg_table(struct drm_gem_object *obj);
>  int msm_gem_prime_vmap(struct drm_gem_object *obj, struct iosys_map *map);
>  void msm_gem_prime_vunmap(struct drm_gem_object *obj, struct iosys_map *map);
> diff --git a/drivers/gpu/drm/msm/msm_gem_prime.c b/drivers/gpu/drm/msm/msm_gem_prime.c
> index 94ab705e9b8a..dcc8a573bc76 100644
> --- a/drivers/gpu/drm/msm/msm_gem_prime.c
> +++ b/drivers/gpu/drm/msm/msm_gem_prime.c
> @@ -11,6 +11,21 @@
>  #include "msm_drv.h"
>  #include "msm_gem.h"
>
> +int msm_gem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma)
> +{
> +       int ret;
> +
> +       /* Ensure the mmap offset is initialized.  We lazily initialize it,
> +        * so if it has not been first mmap'd directly as a GEM object, the
> +        * mmap offset will not be already initialized.
> +        */
> +       ret = drm_gem_create_mmap_offset(obj);
> +       if (ret)
> +               return ret;

Wouldn't it be better to have this call directly in the
drm_gem_prime_mmap() ? This way all drivers can be lazy.


> +
> +       return drm_gem_prime_mmap(obj, vma);
> +}
> +
>  struct sg_table *msm_gem_prime_get_sg_table(struct drm_gem_object *obj)
>  {
>         struct msm_gem_object *msm_obj = to_msm_bo(obj);
> --
> 2.36.1
>


-- 
With best wishes
Dmitry