From fe743523bf9ce15010f7cbf60d8ede152c8cf217 Mon Sep 17 00:00:00 2001
From: Mathias Agopian <mathias@google.com>
Date: Fri, 21 Jun 2024 22:04:46 -0700
Subject: [PATCH] fix use-after-free dereference risk

Texture handles were resolved to pointers when updating a SamplerGroup,
as that point the handle was checked for use-after-free. However, the
texture could be destroyed later while still active in the SamplerGroup,
this would result in using the pointer which now contains garbage.

We now keep the handle and resolve the texture when binding samplers
to the program; which will also perform the use-after-free check.
---
 .../include/private/backend/HandleAllocator.h      | 12 ++++++++++++
 filament/backend/src/opengl/OpenGLDriver.cpp       |  8 ++++----
 filament/backend/src/opengl/OpenGLDriver.h         |  7 ++++++-
 filament/backend/src/opengl/OpenGLProgram.cpp      | 14 +++++++++-----
 4 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/filament/backend/include/private/backend/HandleAllocator.h b/filament/backend/include/private/backend/HandleAllocator.h
index afc6dc924e..d083fe3da9 100644
--- a/filament/backend/include/private/backend/HandleAllocator.h
+++ b/filament/backend/include/private/backend/HandleAllocator.h
@@ -181,6 +181,18 @@ public:
         return static_cast<Dp>(p);
     }
 
+    template<typename B>
+    bool is_valid(Handle<B>& handle) {
+        if (handle && isPoolHandle(handle.getId())) {
+            auto [p, tag] = handleToPointer(handle.getId());
+            uint8_t const age = (tag & HANDLE_AGE_MASK) >> HANDLE_AGE_SHIFT;
+            auto const pNode = static_cast<typename Allocator::Node*>(p);
+            uint8_t const expectedAge = pNode[-1].age;
+            return expectedAge == age;
+        }
+        return true;
+    }
+
     template<typename Dp, typename B>
     inline typename std::enable_if_t<
             std::is_pointer_v<Dp> &&
diff --git a/filament/backend/src/opengl/OpenGLDriver.cpp b/filament/backend/src/opengl/OpenGLDriver.cpp
index 4f88c9717f..5b011ee625 100644
--- a/filament/backend/src/opengl/OpenGLDriver.cpp
+++ b/filament/backend/src/opengl/OpenGLDriver.cpp
@@ -2336,9 +2336,9 @@ void OpenGLDriver::updateSamplerGroup(Handle<HwSamplerGroup> sbh,
     auto const* const pSamplers = (SamplerDescriptor const*)data.buffer;
     for (size_t i = 0, c = sb->textureUnitEntries.size(); i < c; i++) {
         GLuint samplerId = 0u;
-        const GLTexture* t = nullptr;
-        if (UTILS_LIKELY(pSamplers[i].t)) {
-            t = handle_cast<const GLTexture*>(pSamplers[i].t);
+        Handle<HwTexture> th = pSamplers[i].t;
+        if (UTILS_LIKELY(th)) {
+            GLTexture const* const t = handle_cast<const GLTexture*>(th);
             assert_invariant(t);
 
             SamplerParams params = pSamplers[i].s;
@@ -2394,7 +2394,7 @@ void OpenGLDriver::updateSamplerGroup(Handle<HwSamplerGroup> sbh,
             // which is not an error.
         }
 
-        sb->textureUnitEntries[i] = { t, samplerId };
+        sb->textureUnitEntries[i] = { th, samplerId };
     }
     scheduleDestroy(std::move(data));
 }
diff --git a/filament/backend/src/opengl/OpenGLDriver.h b/filament/backend/src/opengl/OpenGLDriver.h
index d5e1b797b6..5345d474bf 100644
--- a/filament/backend/src/opengl/OpenGLDriver.h
+++ b/filament/backend/src/opengl/OpenGLDriver.h
@@ -126,7 +126,7 @@ public:
     struct GLSamplerGroup : public HwSamplerGroup {
         using HwSamplerGroup::HwSamplerGroup;
         struct Entry {
-            GLTexture const* texture = nullptr;
+            Handle<HwTexture> th;
             GLuint sampler = 0u;
         };
         utils::FixedCapacityVector<Entry> textureUnitEntries;
@@ -256,6 +256,11 @@ private:
         return mHandleAllocator.handle_cast<Dp, B>(handle);
     }
 
+    template<typename B>
+    bool is_valid(Handle<B>& handle) {
+        return mHandleAllocator.is_valid(handle);
+    }
+
     template<typename Dp, typename B>
     inline typename std::enable_if_t<
             std::is_pointer_v<Dp> &&
diff --git a/filament/backend/src/opengl/OpenGLProgram.cpp b/filament/backend/src/opengl/OpenGLProgram.cpp
index 6c5eab9a44..7a85e3c63c 100644
--- a/filament/backend/src/opengl/OpenGLProgram.cpp
+++ b/filament/backend/src/opengl/OpenGLProgram.cpp
@@ -20,21 +20,24 @@
 #include "OpenGLDriver.h"
 #include "ShaderCompilerService.h"
 
+#include <backend/DriverEnums.h>
 #include <backend/Program.h>
+#include <backend/Handle.h>
 
-#include <private/backend/BackendUtils.h>
-
-#include <utils/debug.h>
 #include <utils/compiler.h>
+#include <utils/debug.h>
+#include <utils/FixedCapacityVector.h>
 #include <utils/Log.h>
 #include <utils/Systrace.h>
 
+#include <algorithm>
 #include <array>
 #include <string_view>
 #include <utility>
 #include <new>
 
 #include <stddef.h>
+#include <stdint.h>
 
 namespace filament::backend {
 
@@ -241,8 +244,9 @@ void OpenGLProgram::updateSamplers(OpenGLDriver* const gld) const noexcept {
         assert_invariant(sb);
         if (!sb) continue; // should never happen, this would be a user error.
         for (uint8_t j = 0, m = sb->textureUnitEntries.size(); j < m; ++j, ++tmu) { // "<=" on purpose here
-            const GLTexture* const t = sb->textureUnitEntries[j].texture;
-            if (t) { // program may not use all samplers of sampler group
+            Handle<HwTexture> th = sb->textureUnitEntries[j].th;
+            if (th) { // program may not use all samplers of sampler group
+                GLTexture const* const t = gld->handle_cast<GLTexture const*>(th);
                 gld->bindTexture(tmu, t);
 #ifndef FILAMENT_SILENCE_NOT_SUPPORTED_BY_ES2
                 if (UTILS_LIKELY(!es2)) {