https://gitlab.freedesktop.org/mesa/mesa/merge_requests/605
Can you try that please and maybe discuss there as well? I cleaned up
the code a bit and realigned it with the radeon code.
Dave.
On Tue, 9 Apr 2019 at 04:44, Chia-I Wu <olva...@gmail.com> wrote:
>
>
>
> On Mon, Apr 8, 2019 at 11:24 AM Lepton Wu <lep...@chromium.org> wrote:
>>
>> On Mon, Apr 8, 2019 at 11:10 AM Chia-I Wu <olva...@gmail.com> wrote:
>> >
>> >
>> >
>> > On Mon, Apr 8, 2019 at 9:34 AM Lepton Wu <lep...@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 <lep...@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.
>> But it seems there is no way to fix it at user space, right? Every
>> time we got a flink name for
>> the first time, kernel will give a new handle and no way to reuse any
>> res from hash table.
>
> Yeah, I think you are right... but that appears to be a kernel bug. Once
> kernel is fixed to return the same handle for the same flink name / prime fd,
> we should be ready for that. Let's see what Dave thinks.
>
>> >
>> >> 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
>> >>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
