commit 74ace839e01801ee01cf841bf27762947a70d8b1
author Roman Kiryanov <rkir@google.com> Thu Mar 07 18:22:19 2019 -0800
committer Roman Kiryanov <rkir@google.com> Mon Mar 11 16:07:20 2019 -0700
tree bafd99335b7196f9892b2c4b1f368f56d94e2065
parent 6c1d1c32e3aefc0414eda1fdf1af7edf9ef6100a

Copy extension names to an owning container to prevent use-after-free

Bug: 127660235
Test: make -j
Change-Id: I3748b7b4b51f99acf59748675223cef02c22bee2
Signed-off-by: Roman Kiryanov <rkir@google.com>

libs/hwui/renderthread/VulkanManager.cpp

diff --git a/libs/hwui/renderthread/VulkanManager.cpp b/libs/hwui/renderthread/VulkanManager.cpp
index 5af660c..ce2dddc 100644
--- a/libs/hwui/renderthread/VulkanManager.cpp
+++ b/libs/hwui/renderthread/VulkanManager.cpp
@@ -79,7 +79,9 @@
     mDevice = VK_NULL_HANDLE;
     mPhysicalDevice = VK_NULL_HANDLE;
     mInstance = VK_NULL_HANDLE;
+    mInstanceExtensionsOwner.clear();
     mInstanceExtensions.clear();
+    mDeviceExtensionsOwner.clear();
     mDeviceExtensions.clear();
     free_features_extensions_structs(mPhysicalDeviceFeatures2);
     mPhysicalDeviceFeatures2 = {};
@@ -104,18 +106,18 @@
         uint32_t extensionCount = 0;
         err = mEnumerateInstanceExtensionProperties(nullptr, &extensionCount, nullptr);
         LOG_ALWAYS_FATAL_IF(VK_SUCCESS != err);
-        std::unique_ptr<VkExtensionProperties[]> extensions(
-                new VkExtensionProperties[extensionCount]);
-        err = mEnumerateInstanceExtensionProperties(nullptr, &extensionCount, extensions.get());
+        mInstanceExtensionsOwner.resize(extensionCount);
+        err = mEnumerateInstanceExtensionProperties(nullptr, &extensionCount,
+                                                    mInstanceExtensionsOwner.data());
         LOG_ALWAYS_FATAL_IF(VK_SUCCESS != err);
         bool hasKHRSurfaceExtension = false;
         bool hasKHRAndroidSurfaceExtension = false;
-        for (uint32_t i = 0; i < extensionCount; ++i) {
-            mInstanceExtensions.push_back(extensions[i].extensionName);
-            if (!strcmp(extensions[i].extensionName, VK_KHR_SURFACE_EXTENSION_NAME)) {
+        for (const VkExtensionProperties& extension : mInstanceExtensionsOwner) {
+            mInstanceExtensions.push_back(extension.extensionName);
+            if (!strcmp(extension.extensionName, VK_KHR_SURFACE_EXTENSION_NAME)) {
                 hasKHRSurfaceExtension = true;
             }
-            if (!strcmp(extensions[i].extensionName,VK_KHR_ANDROID_SURFACE_EXTENSION_NAME)) {
+            if (!strcmp(extension.extensionName, VK_KHR_ANDROID_SURFACE_EXTENSION_NAME)) {
                 hasKHRAndroidSurfaceExtension = true;
             }
         }
@@ -193,15 +195,14 @@
         err = mEnumerateDeviceExtensionProperties(mPhysicalDevice, nullptr, &extensionCount,
                 nullptr);
         LOG_ALWAYS_FATAL_IF(VK_SUCCESS != err);
-        std::unique_ptr<VkExtensionProperties[]> extensions(
-                new VkExtensionProperties[extensionCount]);
+        mDeviceExtensionsOwner.resize(extensionCount);
         err = mEnumerateDeviceExtensionProperties(mPhysicalDevice, nullptr, &extensionCount,
-                extensions.get());
+                mDeviceExtensionsOwner.data());
         LOG_ALWAYS_FATAL_IF(VK_SUCCESS != err);
         bool hasKHRSwapchainExtension = false;
-        for (uint32_t i = 0; i < extensionCount; ++i) {
-            mDeviceExtensions.push_back(extensions[i].extensionName);
-            if (!strcmp(extensions[i].extensionName, VK_KHR_SWAPCHAIN_EXTENSION_NAME)) {
+        for (const VkExtensionProperties& extension : mDeviceExtensionsOwner) {
+            mDeviceExtensions.push_back(extension.extensionName);
+            if (!strcmp(extension.extensionName, VK_KHR_SWAPCHAIN_EXTENSION_NAME)) {
                 hasKHRSwapchainExtension = true;
             }
         }
@@ -214,6 +215,7 @@
         }
         return vkGetInstanceProcAddr(instance, proc_name);
     };
+
     grExtensions.init(getProc, mInstance, mPhysicalDevice, mInstanceExtensions.size(),
             mInstanceExtensions.data(), mDeviceExtensions.size(), mDeviceExtensions.data());
 

libs/hwui/renderthread/VulkanManager.h

diff --git a/libs/hwui/renderthread/VulkanManager.h b/libs/hwui/renderthread/VulkanManager.h
index 95c9630..f5518e5 100644
--- a/libs/hwui/renderthread/VulkanManager.h
+++ b/libs/hwui/renderthread/VulkanManager.h
@@ -148,6 +148,8 @@
     status_t createReleaseFence(sp<Fence>& nativeFence);
 
     // Returned pointers are owned by VulkanManager.
+    // An instance of VkFunctorInitParams returned from getVkFunctorInitParams refers to
+    // the internal state of VulkanManager: VulkanManager must be alive to use the returned value.
     VkFunctorInitParams getVkFunctorInitParams() const;
 
     sk_sp<GrContext> createContext(const GrContextOptions& options);
@@ -249,7 +251,9 @@
 
     // Variables saved to populate VkFunctorInitParams.
     static const uint32_t mAPIVersion = VK_MAKE_VERSION(1, 1, 0);
+    std::vector<VkExtensionProperties> mInstanceExtensionsOwner;
     std::vector<const char*> mInstanceExtensions;
+    std::vector<VkExtensionProperties> mDeviceExtensionsOwner;
     std::vector<const char*> mDeviceExtensions;
     VkPhysicalDeviceFeatures2 mPhysicalDeviceFeatures2{};