<div dir="ltr"><div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Apr 8, 2019 at 9:34 AM Lepton Wu <<a href="mailto:lepton@chromium.org" target="_blank">lepton@chromium.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">The old code could use gem name as key when inserting it to bo_handles<br>
hash table while trying to remove it from hash table with bo_handle as<br>
key in virgl_hw_res_destroy and then it fail to remove it from bo_handles<br>
hash table. This triggers use after free. Also, we should insert resource<br>
to bo_names hash table when handle type is SHARED.<br>
<br>
Signed-off-by: Lepton Wu <<a href="mailto:lepton@chromium.org" target="_blank">lepton@chromium.org</a>><br>
---<br>
 .../winsys/virgl/drm/virgl_drm_winsys.c       | 24 +++++++++++++------<br>
 1 file changed, 17 insertions(+), 7 deletions(-)<br>
<br>
diff --git a/src/gallium/winsys/virgl/drm/virgl_drm_winsys.c b/src/gallium/winsys/virgl/drm/virgl_drm_winsys.c<br>
index 2cf8b4ba076..af92b6a98fc 100644<br>
--- a/src/gallium/winsys/virgl/drm/virgl_drm_winsys.c<br>
+++ b/src/gallium/winsys/virgl/drm/virgl_drm_winsys.c<br>
@@ -406,6 +406,12 @@ virgl_drm_winsys_resource_create_handle(struct virgl_winsys *qws,<br>
       return NULL;<br>
    }<br>
<br>
+   if (whandle->type != WINSYS_HANDLE_TYPE_FD &&<br>
+       whandle->type != WINSYS_HANDLE_TYPE_SHARED) {<br>
+      fprintf(stderr, "Unexpected handle type: %d\n", whandle->type);<br>
+      return NULL;<br>
+   }<br>
+<br>
    mtx_lock(&qdws->bo_handles_mutex);<br>
<br>
    if (whandle->type == WINSYS_HANDLE_TYPE_SHARED) {<br>
@@ -424,13 +430,13 @@ virgl_drm_winsys_resource_create_handle(struct virgl_winsys *qws,<br>
          res = NULL;<br>
          goto done;<br>
       }<br>
-   }<br>
<br>
-   res = util_hash_table_get(qdws->bo_handles, (void*)(uintptr_t)handle);<br>
-   if (res) {<br>
-      struct virgl_hw_res *r = NULL;<br>
-      virgl_drm_resource_reference(qdws, &r, res);<br>
-      goto done;<br>
+      res = util_hash_table_get(qdws->bo_handles, (void*)(uintptr_t)handle);<br>
+      if (res) {<br>
+         struct virgl_hw_res *r = NULL;<br>
+         virgl_drm_resource_reference(qdws, &r, res);<br>
+         goto done;<br>
+      }<br>
    }<br>
<br></blockquote><div>You can still run into troubles when asked to import a buffer by both its prime fd and its flink name.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">    res = CALLOC_STRUCT(virgl_hw_res);<br>
@@ -448,6 +454,8 @@ virgl_drm_winsys_resource_create_handle(struct virgl_winsys *qws,<br>
          goto done;<br>
       }<br>
       res->bo_handle = open_arg.handle;<br>
+      res->flinked = true;<br>
+      res->flink = whandle->handle;<br>
    }<br>
    res->name = handle;<br></blockquote><div>res->name has no user.  Remove it.</div><div><br></div><div>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.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
@@ -469,7 +477,9 @@ virgl_drm_winsys_resource_create_handle(struct virgl_winsys *qws,<br>
    res->num_cs_references = 0;<br>
    res->fence_fd = -1;<br>
<br>
-   util_hash_table_set(qdws->bo_handles, (void *)(uintptr_t)handle, res);<br>
+   util_hash_table_set(qdws->bo_handles, (void *)(uintptr_t)res->bo_handle, res);<br>
+   if (whandle->type == WINSYS_HANDLE_TYPE_SHARED)<br>
+      util_hash_table_set(qdws->bo_names, (void *)(uintptr_t)res->flink, res);<br>
<br>
 done:<br>
    mtx_unlock(&qdws->bo_handles_mutex);<br>
-- <br>
2.21.0.392.gf8f6787159e-goog<br>
<br>
</blockquote></div></div></div>