[Mesa-dev] [PATCH] virgl: Use right key to insert resource to hash.

[Chia-I Wu]

On Mon, Mar 18, 2019 at 2:22 PM Lepton Wu <lepton at chromium.org> wrote:

> The old code could use gem name as key when inserting it to bo_handles
> hash table while trying to remove it from hash table with bo_handle as
> key in virgl_hw_res_destroy. This triggers use after free. Also, we
> should only reuse resource from bo_handle hash when the handle type is
> FD.
>
Reuse is not very accurate.  Opening a shared handle (flink name) twice
gives two GEM handles.  Importing an fd handle (prime fd) twice gives the
same GEM handle.  In all cases, within a virgl_winsys, we want only one GEM
handle and only one virgl_resource for each kernel GEM object.

I think the logic should go like:

  if (HANDLE_TYPE_SHARED) {
    if (bo_names.has(flink_name))
      return bo_names[flink_name];
    gem_handle = gem_open(flink_name);
  } else {
    gem_handle = drmPrimeFDToHandle(prime_fd);
  }

  if (bo_handles.has(gem_handle))
    return bo_handles[gem_handle];
  bo_handles[gem_handle] = create_new_resource();


> Signed-off-by: Lepton Wu <lepton at chromium.org>
> ---
>  src/gallium/winsys/virgl/drm/virgl_drm_winsys.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/src/gallium/winsys/virgl/drm/virgl_drm_winsys.c
> b/src/gallium/winsys/virgl/drm/virgl_drm_winsys.c
> index 120e8eda2cd..01811a0e997 100644
> --- a/src/gallium/winsys/virgl/drm/virgl_drm_winsys.c
> +++ b/src/gallium/winsys/virgl/drm/virgl_drm_winsys.c
> @@ -424,13 +424,13 @@ virgl_drm_winsys_resource_create_handle(struct
> virgl_winsys *qws,
>           res = NULL;
>           goto done;
>        }
> -   }
>
> -   res = util_hash_table_get(qdws->bo_handles, (void*)(uintptr_t)handle);
> -   if (res) {
> -      struct virgl_hw_res *r = NULL;
> -      virgl_drm_resource_reference(qdws, &r, res);
> -      goto done;
> +      res = util_hash_table_get(qdws->bo_handles,
> (void*)(uintptr_t)handle);
> +      if (res) {
> +        struct virgl_hw_res *r = NULL;
> +        virgl_drm_resource_reference(qdws, &r, res);
> +        goto done;
> +      }
>     }
>
>     res = CALLOC_STRUCT(virgl_hw_res);
> @@ -469,7 +469,8 @@ virgl_drm_winsys_resource_create_handle(struct
> virgl_winsys *qws,
>     res->num_cs_references = 0;
>     res->fence_fd = -1;
>
> -   util_hash_table_set(qdws->bo_handles, (void *)(uintptr_t)handle, res);
> +   util_hash_table_set(qdws->bo_handles, (void
> *)(uintptr_t)res->bo_handle,
> +                       res);
>
>  done:
>     mtx_unlock(&qdws->bo_handles_mutex);
> --
> 2.21.0.225.g810b269d1ac-goog
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20190320/2d0fdfbb/attachment.html>