From 74ace839e01801ee01cf841bf27762947a70d8b1 Mon Sep 17 00:00:00 2001
From: Roman Kiryanov <rkir@google.com>
Date: Thu, 7 Mar 2019 18:22:19 -0800
Subject: [PATCH] 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 | 28 +++++++++++++-----------
 libs/hwui/renderthread/VulkanManager.h   |  4 ++++
 2 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/libs/hwui/renderthread/VulkanManager.cpp b/libs/hwui/renderthread/VulkanManager.cpp
index 5af660c8738a9..ce2dddc690491 100644
--- a/libs/hwui/renderthread/VulkanManager.cpp
+++ b/libs/hwui/renderthread/VulkanManager.cpp
@@ -79,7 +79,9 @@ void VulkanManager::destroy() {
     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 @@ void VulkanManager::setupDevice(GrVkExtensions& grExtensions, VkPhysicalDeviceFe
         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 @@ void VulkanManager::setupDevice(GrVkExtensions& grExtensions, VkPhysicalDeviceFe
         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 @@ void VulkanManager::setupDevice(GrVkExtensions& grExtensions, VkPhysicalDeviceFe
         }
         return vkGetInstanceProcAddr(instance, proc_name);
     };
+
     grExtensions.init(getProc, mInstance, mPhysicalDevice, mInstanceExtensions.size(),
             mInstanceExtensions.data(), mDeviceExtensions.size(), mDeviceExtensions.data());
 
diff --git a/libs/hwui/renderthread/VulkanManager.h b/libs/hwui/renderthread/VulkanManager.h
index 95c9630fb728b..f5518e50f17ec 100644
--- a/libs/hwui/renderthread/VulkanManager.h
+++ b/libs/hwui/renderthread/VulkanManager.h
@@ -148,6 +148,8 @@ public:
     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 @@ private:
 
     // 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{};