layers: Fix command buffer allocation ABA bug

In core validation layer's DestroyCommandPool API entry function, the
ext layer's API call is executed outside of the lock, and before the
command pool's command buffers are erased from the layer's command
buffer map. That means that between next layer's call and the time the
lock is regained, any other thread can allocate other command buffers.
In particular, it may reuse the memory freed the command, and then the
core validation layer will not insert a new entry in the command buffer
map. So after the lock in regained and the command pool's command
buffers are erased from the map, leaving those allocated with no state,
leading to either crashes or assertions later on. The fix is to move the
release of command buffers before releasing the lock the first, before
calling the next layer's function, such that they will be removed from
the map before any other thread might reuse those addresses.

1 files changed, 4 insertions, 5 deletions

diff --git a/layers/core_validation.cpp b/layers/core_validation.cpp
index 3484389f..fc735772 100644
--- a/layers/core_validation.cpp
+++ b/layers/core_validation.cpp
@@ -4500,7 +4500,7 @@ static bool PreCallValidateDestroyCommandPool(layer_data *dev_data, VkCommandPoo
     return skip;
 }
 
-static void PostCallRecordDestroyCommandPool(layer_data *dev_data, VkCommandPool pool) {
+static void PreCallRecordDestroyCommandPool(layer_data *dev_data, VkCommandPool pool) {
     COMMAND_POOL_NODE *cp_state = GetCommandPoolNode(dev_data, pool);
     // Remove cmdpool from cmdpoolmap, after freeing layer data for the command buffers
     // "When a pool is destroyed, all command buffers allocated from the pool are freed."
@@ -4518,12 +4518,11 @@ VKAPI_ATTR void VKAPI_CALL DestroyCommandPool(VkDevice device, VkCommandPool com
     unique_lock_t lock(global_lock);
     bool skip = PreCallValidateDestroyCommandPool(dev_data, commandPool);
     if (!skip) {
-        lock.unlock();
-        dev_data->dispatch_table.DestroyCommandPool(device, commandPool, pAllocator);
-        lock.lock();
         if (commandPool != VK_NULL_HANDLE) {
-            PostCallRecordDestroyCommandPool(dev_data, commandPool);
+            PreCallRecordDestroyCommandPool(dev_data, commandPool);
         }
+        lock.unlock();
+        dev_data->dispatch_table.DestroyCommandPool(device, commandPool, pAllocator);
     }
 }