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

[Chia-I Wu]

On Mon, Apr 8, 2019 at 9:34 AM 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 and then it fail to remove it from bo_handles
> hash table. This triggers use after free. Also, we should insert resource
> to bo_names hash table when handle type is SHARED.
>
> Signed-off-by: Lepton Wu <lepton at chromium.org>
> ---
>  .../winsys/virgl/drm/virgl_drm_winsys.c       | 24 +++++++++++++------
>  1 file changed, 17 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 2cf8b4ba076..af92b6a98fc 100644
> --- a/src/gallium/winsys/virgl/drm/virgl_drm_winsys.c
> +++ b/src/gallium/winsys/virgl/drm/virgl_drm_winsys.c
> @@ -406,6 +406,12 @@ virgl_drm_winsys_resource_create_handle(struct
> virgl_winsys *qws,
>        return NULL;
>     }
>
> +   if (whandle->type != WINSYS_HANDLE_TYPE_FD &&
> +       whandle->type != WINSYS_HANDLE_TYPE_SHARED) {
> +      fprintf(stderr, "Unexpected handle type: %d\n", whandle->type);
> +      return NULL;
> +   }
> +
>     mtx_lock(&qdws->bo_handles_mutex);
>
>     if (whandle->type == WINSYS_HANDLE_TYPE_SHARED) {
> @@ -424,13 +430,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;
> +      }
>     }
>
> You can still run into troubles when asked to import a buffer by both its
prime fd and its flink name.

    res = CALLOC_STRUCT(virgl_hw_res);
> @@ -448,6 +454,8 @@ virgl_drm_winsys_resource_create_handle(struct
> virgl_winsys *qws,
>           goto done;
>        }
>        res->bo_handle = open_arg.handle;
> +      res->flinked = true;
> +      res->flink = whandle->handle;
>     }
>     res->name = handle;
>
res->name has no user.  Remove it.

It is also less confusing to make sure `handle' means GEM handle at this
point, by calling either GEM_OPEN or drmPrimeFDToHandle depending on the
handle type.


> @@ -469,7 +477,9 @@ 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);
> +   if (whandle->type == WINSYS_HANDLE_TYPE_SHARED)
> +      util_hash_table_set(qdws->bo_names, (void *)(uintptr_t)res->flink,
> res);
>
>  done:
>     mtx_unlock(&qdws->bo_handles_mutex);
> --
> 2.21.0.392.gf8f6787159e-goog
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20190408/1a8f7e5d/attachment-0001.html>