From f91a9dd35715b1ec0dc5bbc7c8a80dc89083d8d0 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Tue, 26 Feb 2019 10:53:18 +0100 Subject: [PATCH] Fix unlinking resources from hash table. Virglrenderer sometimes tries to remove resources from the hash table twice. Which will mess up the ressource hash table and reference counts and therefore and leads to qemu/virglrenderer crashes. Reproducer: (a) guest creates resource foo, id 42. (b) guest creates an object bar referencing resource foo. (c) guest unreferences resource foo. -> resource id 42 is removed from hash. (d) guest creates a new resource baz, re-using id 42. (e) guest destroys object bar. -> resource foo refcount goes down to zero. -> resource id 42 gets removed from hash the second time, but id 42 entry points to resource baz not foo now. Oops. Note that most linux kernel drivers will never ever re-use resource ids due to a bug in the virtio-gpu kms driver, in which case this bug doesn't cause any harm. Root cause is that vrend_renderer_resource_destroy() may call vrend_resource_remove(), depending on the call chain. This is wrong. Only vrend_renderer_resource_unref() which is called when the guest unreferences a resource should remove the id from the hash table by calling vrend_resource_remove(). Signed-off-by: Gerd Hoffmann Reviewed-by: --- src/vrend_renderer.c | 10 ++++------ src/vrend_renderer.h | 4 ++-- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/vrend_renderer.c b/src/vrend_renderer.c index 9ff25f8..b0d89c8 100644 --- a/src/vrend_renderer.c +++ b/src/vrend_renderer.c @@ -5997,13 +5997,13 @@ int vrend_renderer_resource_create(struct vrend_renderer_resource_create_args *a ret = vrend_resource_insert(gr, args->handle); if (ret == 0) { - vrend_renderer_resource_destroy(gr, true); + vrend_renderer_resource_destroy(gr); return ENOMEM; } return 0; } -void vrend_renderer_resource_destroy(struct vrend_resource *res, bool remove) +void vrend_renderer_resource_destroy(struct vrend_resource *res) { if (res->readback_fb_id) glDeleteFramebuffers(1, &res->readback_fb_id); @@ -6019,8 +6019,6 @@ void vrend_renderer_resource_destroy(struct vrend_resource *res, bool remove) glDeleteTextures(1, &res->id); } - if (res->handle && remove) - vrend_resource_remove(res->handle); free(res); } @@ -6029,7 +6027,7 @@ static void vrend_destroy_resource_object(void *obj_ptr) struct vrend_resource *res = obj_ptr; if (pipe_reference(&res->base.reference, NULL)) - vrend_renderer_resource_destroy(res, false); + vrend_renderer_resource_destroy(res); } void vrend_renderer_resource_unref(uint32_t res_handle) @@ -7593,7 +7591,7 @@ static void vrend_renderer_blit_int(struct vrend_context *ctx, glBindFramebuffer(GL_FRAMEBUFFER, ctx->sub->fb_id); if (make_intermediate_copy) { - vrend_renderer_resource_destroy(intermediate_copy, false); + vrend_renderer_resource_destroy(intermediate_copy); glDeleteFramebuffers(1, &intermediate_fbo); } diff --git a/src/vrend_renderer.h b/src/vrend_renderer.h index e6b26d9..4e34391 100644 --- a/src/vrend_renderer.h +++ b/src/vrend_renderer.h @@ -356,7 +356,7 @@ int vrend_renderer_resource_attach_iov(int res_handle, struct iovec *iov, void vrend_renderer_resource_detach_iov(int res_handle, struct iovec **iov_p, int *num_iovs_p); -void vrend_renderer_resource_destroy(struct vrend_resource *res, bool remove); +void vrend_renderer_resource_destroy(struct vrend_resource *res); static inline void vrend_resource_reference(struct vrend_resource **ptr, struct vrend_resource *tex) @@ -364,7 +364,7 @@ vrend_resource_reference(struct vrend_resource **ptr, struct vrend_resource *tex struct vrend_resource *old_tex = *ptr; if (pipe_reference(&(*ptr)->base.reference, &tex->base.reference)) - vrend_renderer_resource_destroy(old_tex, true); + vrend_renderer_resource_destroy(old_tex); *ptr = tex; }