Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

vk: Use-after-free coverage #8273

Merged
merged 3 commits into from
Dec 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 29 additions & 4 deletions filament/backend/src/vulkan/memory/Resource.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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;
Expand All @@ -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;

Expand All @@ -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 {
Expand All @@ -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;
Expand All @@ -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;

Expand Down
7 changes: 7 additions & 0 deletions filament/backend/src/vulkan/memory/ResourcePointer.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "vulkan/memory/ResourceManager.h"

#include <backend/Handle.h>
#include <utils/compiler.h>

#include <utility>

Expand Down Expand Up @@ -59,6 +60,10 @@ struct resource_ptr {
static enabled_resource_ptr<B> cast(ResourceManager* resManager,
Handle<B> const& handle) noexcept {
D* ptr = resManager->handle_cast<D*, B>(handle);
FILAMENT_CHECK_PRECONDITION(!ptr->isHandleConsideredDestroyed())
<< "Handle id=" << ptr->id << " (" << getTypeStr(ptr->restype)
<< ") is being used after it has been freed";

return {ptr};
}

Expand Down Expand Up @@ -156,6 +161,8 @@ struct resource_ptr {
// only be used from VulkanDriver.
inline void dec() {
assert_invariant(mRef);
assert_invariant(!mRef->isHandleConsiderDestroyed());
mRef->setHandleConsiderDestroyed();
pixelflinger marked this conversation as resolved.
Show resolved Hide resolved
mRef->dec();
}

Expand Down
Loading