From b9ec043b5a48f95dc124a5458630f5ed582a17a0 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Thu, 29 Jan 2026 10:07:39 +0100 Subject: [PATCH] LibGC: Add live HeapBlock registry to fix weak container UAF Maintain a HashTable of live heap blocks in the Heap, updated on block creation and destruction. Weak containers (WeakMap, WeakSet, WeakRef, FinalizationRegistry) now check block liveness before accessing cell memory in their remove_dead_cells() methods. This prevents use-after-free when blocks have been freed during incremental sweeping. --- Libraries/LibGC/CellAllocator.cpp | 1 + Libraries/LibGC/Heap.h | 7 +++++++ Libraries/LibGC/HeapBlock.cpp | 2 ++ Libraries/LibJS/Runtime/FinalizationRegistry.cpp | 6 +++++- Libraries/LibJS/Runtime/WeakMap.cpp | 6 ++++-- Libraries/LibJS/Runtime/WeakRef.cpp | 9 ++++++++- Libraries/LibJS/Runtime/WeakSet.cpp | 6 ++++-- 7 files changed, 31 insertions(+), 6 deletions(-) diff --git a/Libraries/LibGC/CellAllocator.cpp b/Libraries/LibGC/CellAllocator.cpp index 7d5bbcbaec..7e37102f8d 100644 --- a/Libraries/LibGC/CellAllocator.cpp +++ b/Libraries/LibGC/CellAllocator.cpp @@ -56,6 +56,7 @@ Cell* CellAllocator::allocate_cell(Heap& heap) void CellAllocator::block_did_become_empty(Badge, HeapBlock& block) { block.m_list_node.remove(); + block.heap().m_live_heap_blocks.remove(&block); // NOTE: HeapBlocks are managed by the BlockAllocator, so we don't want to `delete` the block here. block.~HeapBlock(); m_block_allocator.deallocate_block(&block); diff --git a/Libraries/LibGC/Heap.h b/Libraries/LibGC/Heap.h index c81926da5a..0e71a313a0 100644 --- a/Libraries/LibGC/Heap.h +++ b/Libraries/LibGC/Heap.h @@ -8,6 +8,7 @@ #include #include +#include #include #include #include @@ -98,6 +99,8 @@ public: void sweep_block(HeapBlock&); + bool is_live_heap_block(HeapBlock* block) const { return m_live_heap_blocks.contains(block); } + void enqueue_post_gc_task(AK::Function); WeakImpl* create_weak_impl(void*); @@ -106,6 +109,8 @@ public: void did_free_external_memory(size_t); private: + friend class CellAllocator; + friend class HeapBlock; friend class MarkingVisitor; friend class GraphConstructorVisitor; friend class DeferGC; @@ -181,6 +186,8 @@ private: Vector> m_post_gc_tasks; Vector> m_sweep_callbacks; + HashTable m_live_heap_blocks; + WeakBlock::List m_usable_weak_blocks; WeakBlock::List m_full_weak_blocks; diff --git a/Libraries/LibGC/HeapBlock.cpp b/Libraries/LibGC/HeapBlock.cpp index 4ed84b7ef3..74e362636e 100644 --- a/Libraries/LibGC/HeapBlock.cpp +++ b/Libraries/LibGC/HeapBlock.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #ifdef HAS_ADDRESS_SANITIZER @@ -22,6 +23,7 @@ NonnullOwnPtr HeapBlock::create_with_cell_size(Heap& heap, CellAlloca char const* name = nullptr; auto* block = static_cast(cell_allocator.block_allocator().allocate_block(name)); new (block) HeapBlock(heap, cell_allocator, cell_size, overrides_must_survive_garbage_collection, overrides_finalize); + heap.m_live_heap_blocks.set(block); return NonnullOwnPtr(NonnullOwnPtr::Adopt, *block); } diff --git a/Libraries/LibJS/Runtime/FinalizationRegistry.cpp b/Libraries/LibJS/Runtime/FinalizationRegistry.cpp index 920e59896d..9cb2e24b4a 100644 --- a/Libraries/LibJS/Runtime/FinalizationRegistry.cpp +++ b/Libraries/LibJS/Runtime/FinalizationRegistry.cpp @@ -4,6 +4,7 @@ * SPDX-License-Identifier: BSD-2-Clause */ +#include #include #include @@ -54,7 +55,10 @@ void FinalizationRegistry::remove_dead_cells(Badge) { auto any_cells_were_removed = false; for (auto& record : m_records) { - if (!record.target || (record.target->state() == Cell::State::Live && record.target->is_marked())) + if (!record.target) + continue; + auto* block = GC::HeapBlock::from_cell(record.target); + if (heap().is_live_heap_block(block) && record.target->state() == Cell::State::Live && record.target->is_marked()) continue; record.target = nullptr; any_cells_were_removed = true; diff --git a/Libraries/LibJS/Runtime/WeakMap.cpp b/Libraries/LibJS/Runtime/WeakMap.cpp index d09a90ba95..f424fda453 100644 --- a/Libraries/LibJS/Runtime/WeakMap.cpp +++ b/Libraries/LibJS/Runtime/WeakMap.cpp @@ -4,6 +4,7 @@ * SPDX-License-Identifier: BSD-2-Clause */ +#include #include #include @@ -52,8 +53,9 @@ bool WeakMap::weak_map_remove(GC::Ptr key) void WeakMap::remove_dead_cells(Badge) { - m_values.remove_all_matching([](Cell* key, Value) { - return key->state() != Cell::State::Live || !key->is_marked(); + m_values.remove_all_matching([this](Cell* key, Value) { + auto* block = GC::HeapBlock::from_cell(key); + return !heap().is_live_heap_block(block) || key->state() != Cell::State::Live || !key->is_marked(); }); } diff --git a/Libraries/LibJS/Runtime/WeakRef.cpp b/Libraries/LibJS/Runtime/WeakRef.cpp index a6a04b2adb..9cf3b56c6b 100644 --- a/Libraries/LibJS/Runtime/WeakRef.cpp +++ b/Libraries/LibJS/Runtime/WeakRef.cpp @@ -4,6 +4,7 @@ * SPDX-License-Identifier: BSD-2-Clause */ +#include #include namespace JS { @@ -38,7 +39,13 @@ WeakRef::WeakRef(Symbol& value, Object& prototype) void WeakRef::remove_dead_cells(Badge) { - if (m_value.visit([](Cell* cell) -> bool { return cell->state() == Cell::State::Live && cell->is_marked(); }, [](Empty) -> bool { return true; })) + auto is_alive = m_value.visit( + [this](Cell* cell) -> bool { + auto* block = GC::HeapBlock::from_cell(cell); + return heap().is_live_heap_block(block) && cell->state() == Cell::State::Live && cell->is_marked(); + }, + [](Empty) -> bool { return true; }); + if (is_alive) return; m_value = Empty {}; diff --git a/Libraries/LibJS/Runtime/WeakSet.cpp b/Libraries/LibJS/Runtime/WeakSet.cpp index 0e006de9c4..0ff3a237da 100644 --- a/Libraries/LibJS/Runtime/WeakSet.cpp +++ b/Libraries/LibJS/Runtime/WeakSet.cpp @@ -4,6 +4,7 @@ * SPDX-License-Identifier: BSD-2-Clause */ +#include #include #include @@ -45,8 +46,9 @@ bool WeakSet::weak_set_remove(GC::Ptr value) void WeakSet::remove_dead_cells(Badge) { - m_values.remove_all_matching([](Cell* cell) { - return cell->state() != Cell::State::Live || !cell->is_marked(); + m_values.remove_all_matching([this](Cell* cell) { + auto* block = GC::HeapBlock::from_cell(cell); + return !heap().is_live_heap_block(block) || cell->state() != Cell::State::Live || !cell->is_marked(); }); }