vk: Use-after-free coverage

To allow for use-after-free discovery with ref-counting, we keep a bit to indicate whether the handle is considered destroyed by Filament. We assert against "cast"-ing a destroyed handle.
google · Nov 14, 2024 · 3e6ceaa · 3e6ceaa
1 parent 5dfd285
commit 3e6ceaa
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 4 deletions.
diff --git a/filament/backend/src/vulkan/memory/Resource.h b/filament/backend/src/vulkan/memory/Resource.h
@@ -66,7 +66,8 @@ struct Resource {
         : resManager(nullptr),
           id(HandleBase::nullid),
           mCount(0),
-          restype(ResourceType::UNDEFINED_TYPE) {}
+          restype(ResourceType::UNDEFINED_TYPE),
+          mHandleConsideredDestroyed(false) {}
 
 private:
     inline void inc() noexcept {
@@ -80,6 +81,16 @@ struct Resource {
         }
     }
 
+    // To be able to detect use-after-free, we need a bit to signify if the handle should be
+    // consider destroyed (from Filament's perspective).
+    inline void setHandleConsiderDestroyed() noexcept {
+        mHandleConsideredDestroyed = true;
+    }
+
+    inline bool isHandleConsideredDestroyed() const {
+        return mHandleConsideredDestroyed;
+    }
+
     template <typename T>
     inline void init(HandleId id, ResourceManager* resManager) {
         this->id = id;
@@ -92,7 +103,9 @@ struct Resource {
     ResourceManager* resManager; // 8
     HandleId id;                 // 4
     uint32_t mCount      : 24;
-    ResourceType restype : 6;    // restype + mCount is 4 bytes.
+    ResourceType restype : 7;
+    bool mHandleConsideredDestroyed : 1;  // restype + mCount + mHandleConsideredDestroyed
+                                          //   is 4 bytes.
 
     friend class ResourceManager;
 
@@ -105,7 +118,8 @@ struct ThreadSafeResource {
         : resManager(nullptr),
           id(HandleBase::nullid),
           mCount(0),
-          restype(ResourceType::UNDEFINED_TYPE) {}
+          restype(ResourceType::UNDEFINED_TYPE),
+          mHandleConsideredDestroyed(false) {}
 
 private:
     inline void inc() noexcept {
@@ -118,6 +132,16 @@ struct ThreadSafeResource {
         }
     }
 
+    // To be able to detect use-after-free, we need a bit to signify if the handle should be
+    // consider destroyed (from Filament's perspective).
+    inline void setHandleConsiderDestroyed() noexcept {
+        mHandleConsideredDestroyed = true;
+    }
+
+    inline bool isHandleConsideredDestroyed() const {
+        return mHandleConsideredDestroyed;
+    }
+
     template <typename T>
     inline void init(HandleId id, ResourceManager* resManager) {
         this->id = id;
@@ -130,7 +154,8 @@ struct ThreadSafeResource {
     ResourceManager* resManager;  // 8
     HandleId id;                  // 4
     std::atomic<uint32_t> mCount; // 4
-    ResourceType restype;         // 1
+    ResourceType restype : 7;
+    bool mHandleConsideredDestroyed : 1;  // restype + mHandleConsideredDestroyed is 1 byte
 
     friend class ResourceManager;
 

diff --git a/filament/backend/src/vulkan/memory/ResourcePointer.h b/filament/backend/src/vulkan/memory/ResourcePointer.h
@@ -21,6 +21,7 @@
 #include "vulkan/memory/ResourceManager.h"
 
 #include <backend/Handle.h>
+#include <utils/compiler.h>
 
 #include <utility>
 
@@ -59,6 +60,11 @@ struct resource_ptr {
     static enabled_resource_ptr<B> cast(ResourceManager* resManager,
             Handle<B> const& handle) noexcept {
         D* ptr = resManager->handle_cast<D*, B>(handle);
+        if (UTILS_UNLIKELY(ptr->isHandleConsideredDestroyed())) {
+            FILAMENT_CHECK_PRECONDITION(!ptr->isHandleConsideredDestroyed())
+                    << "Handle id=" << ptr->id << " (" << getTypeStr(ptr->restype)
+                    << ") is being used after it has been freed";
+        }
         return {ptr};
     }
 
@@ -156,6 +162,8 @@ struct resource_ptr {
     // only be used from VulkanDriver.
     inline void dec() {
         assert_invariant(mRef);
+        assert_invariant(!mRef->isHandleConsiderDestroyed());
+        mRef->setHandleConsiderDestroyed();
         mRef->dec();
     }