Fix corrupted pNext chain in vkCreateDevice

When creating a device, the loader looks for the VkDeviceGroupCreateInfo
structure and replaces it with its own. This allows the loader to edit the
struct. However, to do this required editing the pNext chain. Because the
edited chain contained pointers to structures whose lifetimes end when the
vkCreateDevice function returns, the pNext chain is now corrupted.

This commit fixes that by storing a pointer to the user's
VkDeviceGroupCreateInfo and fixing up the pNext chain to use that instead.

2 files changed, 153 insertions, 1 deletions

diff --git a/loader/loader.c b/loader/loader.c
index e89c75777..f4e8f3a7b 100644
--- a/loader/loader.c
+++ b/loader/loader.c
@@ -4732,6 +4732,7 @@ VkResult loader_create_device_chain(const VkPhysicalDevice pd, const VkDeviceCre
     VkLayerDeviceLink *layer_device_link_info;
     VkLayerDeviceCreateInfo chain_info;
     VkDeviceCreateInfo loader_create_info;
+    VkDeviceGroupDeviceCreateInfoKHR *original_device_group_create_info_struct = NULL;
     VkResult res;
 
     PFN_vkGetDeviceProcAddr fpGDPA = NULL, nextGDPA = loader_gpa_device_internal;
@@ -4770,6 +4771,8 @@ VkResult loader_create_device_chain(const VkPhysicalDevice pd, const VkDeviceCre
                     }
                     temp_struct->pPhysicalDevices = phys_dev_array;
 
+                    original_device_group_create_info_struct = (VkDeviceGroupDeviceCreateInfoKHR *)pPrev->pNext;
+
                     // Replace the old struct in the pNext chain with this one.
                     pPrev->pNext = (VkBaseOutStructure *)temp_struct;
                 }
@@ -4921,6 +4924,24 @@ VkResult loader_create_device_chain(const VkPhysicalDevice pd, const VkDeviceCre
             return res;
         }
         dev->chain_device = created_device;
+
+        // Because we changed the pNext chain to use our own VkDeviceGroupDeviceCreateInfoKHR, we need to fixup the chain to point
+        // back at the original VkDeviceGroupDeviceCreateInfoKHR.
+        VkBaseOutStructure *pNext = (VkBaseOutStructure *)loader_create_info.pNext;
+        VkBaseOutStructure *pPrev = (VkBaseOutStructure *)&loader_create_info;
+        while (NULL != pNext) {
+            if (VK_STRUCTURE_TYPE_DEVICE_GROUP_DEVICE_CREATE_INFO == pNext->sType) {
+                VkDeviceGroupDeviceCreateInfoKHR *cur_struct = (VkDeviceGroupDeviceCreateInfoKHR *)pNext;
+                if (0 < cur_struct->physicalDeviceCount && NULL != cur_struct->pPhysicalDevices) {
+                    pPrev->pNext = (VkBaseOutStructure *)original_device_group_create_info_struct;
+                }
+                break;
+            }
+
+            pPrev = pNext;
+            pNext = pNext->pNext;
+        }
+
     } else {
         loader_log(inst, VULKAN_LOADER_ERROR_BIT, 0,
                    "loader_create_device_chain: Failed to find \'vkCreateDevice\' in layers or ICD");
@@ -5459,7 +5480,7 @@ VKAPI_ATTR void VKAPI_CALL terminator_DestroyInstance(VkInstance instance, const
     loader_destroy_generic_list(ptr_instance, (struct loader_generic_list *)&ptr_instance->ext_list);
     if (NULL != ptr_instance->phys_devs_term) {
         for (uint32_t i = 0; i < ptr_instance->phys_dev_count_term; i++) {
-            for (uint32_t j = i+1; j < ptr_instance->phys_dev_count_term; j++) {
+            for (uint32_t j = i + 1; j < ptr_instance->phys_dev_count_term; j++) {
                 if (ptr_instance->phys_devs_term[i] == ptr_instance->phys_devs_term[j]) {
                     ptr_instance->phys_devs_term[j] = NULL;
                 }
diff --git a/tests/loader_regression_tests.cpp b/tests/loader_regression_tests.cpp
index ea71f10e5..da1b3721f 100644
--- a/tests/loader_regression_tests.cpp
+++ b/tests/loader_regression_tests.cpp
@@ -1389,6 +1389,35 @@ TEST(EnumeratePhysicalDeviceGroups, OneCall) {
         for (uint32_t dev = 0; dev < max_physical_device_count; ++dev) {
             ASSERT_EQ(true, found[dev]);
         }
+        for (auto& group : group_props) {
+            VkDeviceGroupDeviceCreateInfoKHR group_info{};
+            group_info.sType = VK_STRUCTURE_TYPE_DEVICE_GROUP_DEVICE_CREATE_INFO;
+            group_info.physicalDeviceCount = group.physicalDeviceCount;
+            group_info.pPhysicalDevices = &group.physicalDevices[0];
+            VkBaseInStructure spacer_structure{};
+            spacer_structure.sType = static_cast<VkStructureType>(100000);
+            spacer_structure.pNext = reinterpret_cast<const VkBaseInStructure*>(&group_info);
+            DeviceWrapper dev{inst};
+            dev.create_info.dev.pNext = &spacer_structure;
+            dev.CheckCreate(group.physicalDevices[0]);
+
+            // This convoluted logic makes sure that the pNext chain is unmolested after being passed into vkCreateDevice
+            // While not expected for applications to iterate over this chain, since it is const it is important to make sure
+            // that the chain didn't change somehow, and especially so that iterating it doesn't crash.
+            int count = 0;
+            const VkBaseInStructure* pNext = reinterpret_cast<const VkBaseInStructure*>(dev.create_info.dev.pNext);
+            while (pNext != nullptr) {
+                if (pNext->sType == VK_STRUCTURE_TYPE_DEVICE_GROUP_DEVICE_CREATE_INFO) {
+                    ASSERT_EQ(&group_info, reinterpret_cast<const VkDeviceGroupDeviceCreateInfoKHR*>(pNext));
+                }
+                if (pNext->sType == 100000) {
+                    ASSERT_EQ(&spacer_structure, pNext);
+                }
+                pNext = pNext->pNext;
+                count++;
+            }
+            ASSERT_EQ(count, 2);
+        }
     }
     driver.add_instance_extension({VK_KHR_DEVICE_GROUP_CREATION_EXTENSION_NAME});
     // Extension
@@ -1428,6 +1457,18 @@ TEST(EnumeratePhysicalDeviceGroups, OneCall) {
         for (uint32_t dev = 0; dev < max_physical_device_count; ++dev) {
             ASSERT_EQ(true, found[dev]);
         }
+        for (auto& group : group_props) {
+            VkDeviceGroupDeviceCreateInfoKHR group_info{};
+            group_info.sType = VK_STRUCTURE_TYPE_DEVICE_GROUP_DEVICE_CREATE_INFO;
+            group_info.physicalDeviceCount = group.physicalDeviceCount;
+            group_info.pPhysicalDevices = &group.physicalDevices[0];
+            VkBaseInStructure spacer_structure{};
+            spacer_structure.sType = static_cast<VkStructureType>(100000);
+            spacer_structure.pNext = reinterpret_cast<const VkBaseInStructure*>(&group_info);
+            DeviceWrapper dev{inst};
+            dev.create_info.dev.pNext = &spacer_structure;
+            dev.CheckCreate(group.physicalDevices[0]);
+        }
     }
 }
 
@@ -1484,6 +1525,15 @@ TEST(EnumeratePhysicalDeviceGroups, TwoCall) {
         for (uint32_t dev = 0; dev < max_physical_device_count; ++dev) {
             ASSERT_EQ(true, found[dev]);
         }
+        for (auto& group : group_props) {
+            VkDeviceGroupDeviceCreateInfoKHR group_info{};
+            group_info.sType = VK_STRUCTURE_TYPE_DEVICE_GROUP_DEVICE_CREATE_INFO;
+            group_info.physicalDeviceCount = group.physicalDeviceCount;
+            group_info.pPhysicalDevices = &group.physicalDevices[0];
+            DeviceWrapper dev{inst};
+            dev.create_info.dev.pNext = &group_info;
+            dev.CheckCreate(group.physicalDevices[0]);
+        }
     }
     driver.add_instance_extension({VK_KHR_DEVICE_GROUP_CREATION_EXTENSION_NAME});
     // Extension
@@ -1526,6 +1576,15 @@ TEST(EnumeratePhysicalDeviceGroups, TwoCall) {
         for (uint32_t dev = 0; dev < max_physical_device_count; ++dev) {
             ASSERT_EQ(true, found[dev]);
         }
+        for (auto& group : group_props) {
+            VkDeviceGroupDeviceCreateInfoKHR group_info{};
+            group_info.sType = VK_STRUCTURE_TYPE_DEVICE_GROUP_DEVICE_CREATE_INFO;
+            group_info.physicalDeviceCount = group.physicalDeviceCount;
+            group_info.pPhysicalDevices = &group.physicalDevices[0];
+            DeviceWrapper dev{inst};
+            dev.create_info.dev.pNext = &group_info;
+            dev.CheckCreate(group.physicalDevices[0]);
+        }
     }
 }
 
@@ -1581,6 +1640,15 @@ TEST(EnumeratePhysicalDeviceGroups, TwoCallIncomplete) {
             }
             ASSERT_EQ(true, found);
         }
+        for (auto& group : group_props) {
+            VkDeviceGroupDeviceCreateInfoKHR group_info{};
+            group_info.sType = VK_STRUCTURE_TYPE_DEVICE_GROUP_DEVICE_CREATE_INFO;
+            group_info.physicalDeviceCount = group.physicalDeviceCount;
+            group_info.pPhysicalDevices = &group.physicalDevices[0];
+            DeviceWrapper dev{inst};
+            dev.create_info.dev.pNext = &group_info;
+            dev.CheckCreate(group.physicalDevices[0]);
+        }
     }
     driver.add_instance_extension({VK_KHR_DEVICE_GROUP_CREATION_EXTENSION_NAME});
     // Extension
@@ -1623,6 +1691,15 @@ TEST(EnumeratePhysicalDeviceGroups, TwoCallIncomplete) {
             }
             ASSERT_EQ(true, found);
         }
+        for (auto& group : group_props) {
+            VkDeviceGroupDeviceCreateInfoKHR group_info{};
+            group_info.sType = VK_STRUCTURE_TYPE_DEVICE_GROUP_DEVICE_CREATE_INFO;
+            group_info.physicalDeviceCount = group.physicalDeviceCount;
+            group_info.pPhysicalDevices = &group.physicalDevices[0];
+            DeviceWrapper dev{inst};
+            dev.create_info.dev.pNext = &group_info;
+            dev.CheckCreate(group.physicalDevices[0]);
+        }
     }
 }
 
@@ -1704,6 +1781,15 @@ TEST(EnumeratePhysicalDeviceGroups, TestCoreVersusExtensionSameReturns) {
             }
         }
     }
+    for (auto& group : core_group_props) {
+        VkDeviceGroupDeviceCreateInfoKHR group_info{};
+        group_info.sType = VK_STRUCTURE_TYPE_DEVICE_GROUP_DEVICE_CREATE_INFO;
+        group_info.physicalDeviceCount = group.physicalDeviceCount;
+        group_info.pPhysicalDevices = &group.physicalDevices[0];
+        DeviceWrapper dev{inst};
+        dev.create_info.dev.pNext = &group_info;
+        dev.CheckCreate(group.physicalDevices[0]);
+    }
 }
 
 // Start with 6 devices in 3 different groups, and then add a group,
@@ -1788,6 +1874,15 @@ TEST(EnumeratePhysicalDeviceGroups, CallThriceAddGroupInBetween) {
             }
         }
     }
+    for (auto& group : group_props_after) {
+        VkDeviceGroupDeviceCreateInfoKHR group_info{};
+        group_info.sType = VK_STRUCTURE_TYPE_DEVICE_GROUP_DEVICE_CREATE_INFO;
+        group_info.physicalDeviceCount = group.physicalDeviceCount;
+        group_info.pPhysicalDevices = &group.physicalDevices[0];
+        DeviceWrapper dev{inst};
+        dev.create_info.dev.pNext = &group_info;
+        dev.CheckCreate(group.physicalDevices[0]);
+    }
 }
 
 // Start with 7 devices in 4 different groups, and then remove a group,
@@ -1864,6 +1959,15 @@ TEST(EnumeratePhysicalDeviceGroups, CallTwiceRemoveGroupInBetween) {
             }
         }
     }
+    for (auto& group : group_props_after) {
+        VkDeviceGroupDeviceCreateInfoKHR group_info{};
+        group_info.sType = VK_STRUCTURE_TYPE_DEVICE_GROUP_DEVICE_CREATE_INFO;
+        group_info.physicalDeviceCount = group.physicalDeviceCount;
+        group_info.pPhysicalDevices = &group.physicalDevices[0];
+        DeviceWrapper dev{inst};
+        dev.create_info.dev.pNext = &group_info;
+        dev.CheckCreate(group.physicalDevices[0]);
+    }
 }
 
 // Start with 6 devices in 3 different groups, and then add a device to the middle group,
@@ -1939,6 +2043,15 @@ TEST(EnumeratePhysicalDeviceGroups, CallTwiceAddDeviceInBetween) {
             }
         }
     }
+    for (auto& group : group_props_after) {
+        VkDeviceGroupDeviceCreateInfoKHR group_info{};
+        group_info.sType = VK_STRUCTURE_TYPE_DEVICE_GROUP_DEVICE_CREATE_INFO;
+        group_info.physicalDeviceCount = group.physicalDeviceCount;
+        group_info.pPhysicalDevices = &group.physicalDevices[0];
+        DeviceWrapper dev{inst};
+        dev.create_info.dev.pNext = &group_info;
+        dev.CheckCreate(group.physicalDevices[0]);
+    }
 }
 
 // Start with 6 devices in 3 different groups, and then remove a device to the middle group,
@@ -2025,6 +2138,15 @@ TEST(EnumeratePhysicalDeviceGroups, CallTwiceRemoveDeviceInBetween) {
             }
         }
     }
+    for (auto& group : group_props_after) {
+        VkDeviceGroupDeviceCreateInfoKHR group_info{};
+        group_info.sType = VK_STRUCTURE_TYPE_DEVICE_GROUP_DEVICE_CREATE_INFO;
+        group_info.physicalDeviceCount = group.physicalDeviceCount;
+        group_info.pPhysicalDevices = &group.physicalDevices[0];
+        DeviceWrapper dev{inst};
+        dev.create_info.dev.pNext = &group_info;
+        dev.CheckCreate(group.physicalDevices[0]);
+    }
 }
 
 // Start with 9 devices but only some in 3 different groups, add and remove
@@ -2132,6 +2254,15 @@ TEST(EnumeratePhysicalDeviceGroups, MultipleAddRemoves) {
     for (uint32_t group = 0; group < before_group_count; ++group) {
         ASSERT_EQ(group_props_after_add_device[group].physicalDeviceCount, after_add_dev_expected_counts[group]);
     }
+    for (auto& group : group_props_after_add_device) {
+        VkDeviceGroupDeviceCreateInfoKHR group_info{};
+        group_info.sType = VK_STRUCTURE_TYPE_DEVICE_GROUP_DEVICE_CREATE_INFO;
+        group_info.physicalDeviceCount = group.physicalDeviceCount;
+        group_info.pPhysicalDevices = &group.physicalDevices[0];
+        DeviceWrapper dev{inst};
+        dev.create_info.dev.pNext = &group_info;
+        dev.CheckCreate(group.physicalDevices[0]);
+    }
 }
 
 // Fill in random but valid data into the device properties struct for the current physical device