Mesa (main): venus: free queues after vkDestroyDevice is emitted

[GitLab Mirror]

Module: Mesa
Branch: main
Commit: da000ea2efc794e26a7b24e51437b1b31822ea3b
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=da000ea2efc794e26a7b24e51437b1b31822ea3b

Author: Chia-I Wu <olvaffe at gmail.com>
Date:   Fri Aug  6 11:50:41 2021 -0700

venus: free queues after vkDestroyDevice is emitted

Otherwise, another thread might reuse their object ids for other
objects.  For example,

  T1: free queue with object id X
  T2: reuse id X
  T2: emit vkCreateFoo with id X
  T1: emit vkDestroyDevice

virglrenderer happily accepts that which leads to double frees of the
queue: once when X is updated to point to another object and once when
vkDestroyDevice is executed.  virglrenderer should be fixed to catch
such invalid object id reuse as well.

Fixes
dEQP-VK.api.object_management.multithreaded_shared_resources.device_group.

Fixes: ddd75330559 ("venus: initial support for queue/fence/semaphore")
Signed-off-by: Chia-I Wu <olvaffe at gmail.com>
Reviewed-by: Ryan Neph <ryanneph at google.com>
Reviewed-by: Yiwei Zhang <zzyiwei at chromium.org>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12252>

---

 src/virtio/vulkan/vn_command_buffer.c | 5 +++++
 src/virtio/vulkan/vn_descriptor_set.c | 4 ++++
 src/virtio/vulkan/vn_device.c         | 7 ++++++-
 3 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/src/virtio/vulkan/vn_command_buffer.c b/src/virtio/vulkan/vn_command_buffer.c
index afb2ed71299..de6bd7cac05 100644
--- a/src/virtio/vulkan/vn_command_buffer.c
+++ b/src/virtio/vulkan/vn_command_buffer.c
@@ -474,6 +474,11 @@ vn_DestroyCommandPool(VkDevice device,
 
    alloc = pAllocator ? pAllocator : &pool->allocator;
 
+   /* We must emit vkDestroyCommandPool before freeing the command buffers in
+    * pool->command_buffers.  Otherwise, another thread might reuse their
+    * object ids while they still refer to the command buffers in the
+    * renderer.
+    */
    vn_async_vkDestroyCommandPool(dev->instance, device, commandPool, NULL);
 
    list_for_each_entry_safe(struct vn_command_buffer, cmd,
diff --git a/src/virtio/vulkan/vn_descriptor_set.c b/src/virtio/vulkan/vn_descriptor_set.c
index 1f6376e8f32..d50c07056f9 100644
--- a/src/virtio/vulkan/vn_descriptor_set.c
+++ b/src/virtio/vulkan/vn_descriptor_set.c
@@ -189,6 +189,10 @@ vn_DestroyDescriptorPool(VkDevice device,
 
    alloc = pAllocator ? pAllocator : &pool->allocator;
 
+   /* We must emit vkDestroyDescriptorPool before freeing the sets in
+    * pool->descriptor_sets.  Otherwise, another thread might reuse their
+    * object ids while they still refer to the sets in the renderer.
+    */
    vn_async_vkDestroyDescriptorPool(dev->instance, device, descriptorPool,
                                     NULL);
 
diff --git a/src/virtio/vulkan/vn_device.c b/src/virtio/vulkan/vn_device.c
index e502a85315f..98f22013d33 100644
--- a/src/virtio/vulkan/vn_device.c
+++ b/src/virtio/vulkan/vn_device.c
@@ -358,10 +358,15 @@ vn_DestroyDevice(VkDevice device, const VkAllocationCallbacks *pAllocator)
 
    for (uint32_t i = 0; i < dev->queue_count; i++)
       vn_queue_fini(&dev->queues[i]);
-   vk_free(alloc, dev->queues);
 
+   /* We must emit vkDestroyDevice before freeing dev->queues.  Otherwise,
+    * another thread might reuse their object ids while they still refer to
+    * the queues in the renderer.
+    */
    vn_async_vkDestroyDevice(dev->instance, device, NULL);
 
+   vk_free(alloc, dev->queues);
+
    vn_device_base_fini(&dev->base);
    vk_free(alloc, dev);
 }