mirror of
https://github.com/bitcoin/bitcoin.git
synced 2025-08-25 20:11:35 +02:00
Merge bitcoin/bitcoin#32581: allocators: Apply manual ASan poisoning to PoolResource
ad132761fc
[allocators] Apply manual ASan poisoning to PoolResource (dergoegge) Pull request description: Currently ASan will not detect use-after-free issues for memory allocated by a `PoolResource`. This is because ASan is only aware of the memory chunks allocated by `PoolResource` but not the individual "sub-chunks" within. E.g. this test will not produce an ASan error even though the referenced coin has been deallocated: ```c++ diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp index c46144b34b..aa6ca15ce1 100644 --- a/src/test/coins_tests.cpp +++ b/src/test/coins_tests.cpp @@ -508,6 +508,17 @@ BOOST_FIXTURE_TEST_CASE(updatecoins_simulation_test, UpdateTest) BOOST_CHECK(spent_a_duplicate_coinbase); } +BOOST_AUTO_TEST_CASE(asan_uaf) +{ + CCoinsMapMemoryResource cache_coins_memory_resource{}; + CCoinsMap map(0, SaltedOutpointHasher(/*deterministic=*/true), CCoinsMap::key_equal{}, &cache_coins_memory_resource); + COutPoint outpoint{}; + map.emplace(outpoint, Coin{}); + auto& coin = map.at(outpoint); + map.erase(outpoint); + coin.coin.nHeight = 1; +} + BOOST_AUTO_TEST_CASE(ccoins_serialization) { // Good example ``` Fix this by applying [manual ASan poisoning](https://github.com/google/sanitizers/wiki/AddressSanitizerManualPoisoning) for memory allocated by `PoolResource`: * Newly allocated chunks are poisoned as a whole * "Sub-chunks" are unpoisoned/re-poisoned during allocation/deallocation With the poisoning applied, ASan catches the issue in the test above: ``` $ ./build_unit/bin/test_bitcoin --run_test="coins_tests/asan_uaf" Running 1 test case... ================================================================= ==366064==ERROR: AddressSanitizer: use-after-poison on address 0x7f99c3204870 at pc 0x55569dab6f8a bp 0x7ffe0210e4d0 sp 0x7ffe0210e4c8 READ of size 4 at 0x7f99c3204870 thread T0 (b-test) ``` ACKs for top commit: achow101: ACKad132761fc
marcofleon: code review ACKad132761fc
Tree-SHA512: eb5e80bfa9509225e784151807bd8aa21fb0826ca1781dfe81b1d60bd3766019384ea3f9cb8e53398fde2f4e994a9c201b5a9962b4d279d7e52bb60e8961be11
This commit is contained in:
@@ -14,6 +14,8 @@
|
|||||||
#include <type_traits>
|
#include <type_traits>
|
||||||
#include <utility>
|
#include <utility>
|
||||||
|
|
||||||
|
#include <util/check.h>
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* A memory resource similar to std::pmr::unsynchronized_pool_resource, but
|
* A memory resource similar to std::pmr::unsynchronized_pool_resource, but
|
||||||
* optimized for node-based containers. It has the following properties:
|
* optimized for node-based containers. It has the following properties:
|
||||||
@@ -155,12 +157,15 @@ class PoolResource final
|
|||||||
// if there is still any available memory left, put it into the freelist.
|
// if there is still any available memory left, put it into the freelist.
|
||||||
size_t remaining_available_bytes = std::distance(m_available_memory_it, m_available_memory_end);
|
size_t remaining_available_bytes = std::distance(m_available_memory_it, m_available_memory_end);
|
||||||
if (0 != remaining_available_bytes) {
|
if (0 != remaining_available_bytes) {
|
||||||
|
ASAN_UNPOISON_MEMORY_REGION(m_available_memory_it, sizeof(ListNode));
|
||||||
PlacementAddToList(m_available_memory_it, m_free_lists[remaining_available_bytes / ELEM_ALIGN_BYTES]);
|
PlacementAddToList(m_available_memory_it, m_free_lists[remaining_available_bytes / ELEM_ALIGN_BYTES]);
|
||||||
|
ASAN_POISON_MEMORY_REGION(m_available_memory_it, sizeof(ListNode));
|
||||||
}
|
}
|
||||||
|
|
||||||
void* storage = ::operator new (m_chunk_size_bytes, std::align_val_t{ELEM_ALIGN_BYTES});
|
void* storage = ::operator new (m_chunk_size_bytes, std::align_val_t{ELEM_ALIGN_BYTES});
|
||||||
m_available_memory_it = new (storage) std::byte[m_chunk_size_bytes];
|
m_available_memory_it = new (storage) std::byte[m_chunk_size_bytes];
|
||||||
m_available_memory_end = m_available_memory_it + m_chunk_size_bytes;
|
m_available_memory_end = m_available_memory_it + m_chunk_size_bytes;
|
||||||
|
ASAN_POISON_MEMORY_REGION(m_available_memory_it, m_chunk_size_bytes);
|
||||||
m_allocated_chunks.emplace_back(m_available_memory_it);
|
m_allocated_chunks.emplace_back(m_available_memory_it);
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -202,6 +207,7 @@ public:
|
|||||||
for (std::byte* chunk : m_allocated_chunks) {
|
for (std::byte* chunk : m_allocated_chunks) {
|
||||||
std::destroy(chunk, chunk + m_chunk_size_bytes);
|
std::destroy(chunk, chunk + m_chunk_size_bytes);
|
||||||
::operator delete ((void*)chunk, std::align_val_t{ELEM_ALIGN_BYTES});
|
::operator delete ((void*)chunk, std::align_val_t{ELEM_ALIGN_BYTES});
|
||||||
|
ASAN_UNPOISON_MEMORY_REGION(chunk, m_chunk_size_bytes);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -217,7 +223,11 @@ public:
|
|||||||
// we've already got data in the pool's freelist, unlink one element and return the pointer
|
// we've already got data in the pool's freelist, unlink one element and return the pointer
|
||||||
// to the unlinked memory. Since FreeList is trivially destructible we can just treat it as
|
// to the unlinked memory. Since FreeList is trivially destructible we can just treat it as
|
||||||
// uninitialized memory.
|
// uninitialized memory.
|
||||||
return std::exchange(m_free_lists[num_alignments], m_free_lists[num_alignments]->m_next);
|
ASAN_UNPOISON_MEMORY_REGION(m_free_lists[num_alignments], sizeof(ListNode));
|
||||||
|
auto* next{m_free_lists[num_alignments]->m_next};
|
||||||
|
ASAN_POISON_MEMORY_REGION(m_free_lists[num_alignments], sizeof(ListNode));
|
||||||
|
ASAN_UNPOISON_MEMORY_REGION(m_free_lists[num_alignments], bytes);
|
||||||
|
return std::exchange(m_free_lists[num_alignments], next);
|
||||||
}
|
}
|
||||||
|
|
||||||
// freelist is empty: get one allocation from allocated chunk memory.
|
// freelist is empty: get one allocation from allocated chunk memory.
|
||||||
@@ -228,6 +238,7 @@ public:
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Make sure we use the right amount of bytes for that freelist (might be rounded up),
|
// Make sure we use the right amount of bytes for that freelist (might be rounded up),
|
||||||
|
ASAN_UNPOISON_MEMORY_REGION(m_available_memory_it, round_bytes);
|
||||||
return std::exchange(m_available_memory_it, m_available_memory_it + round_bytes);
|
return std::exchange(m_available_memory_it, m_available_memory_it + round_bytes);
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -244,7 +255,9 @@ public:
|
|||||||
const std::size_t num_alignments = NumElemAlignBytes(bytes);
|
const std::size_t num_alignments = NumElemAlignBytes(bytes);
|
||||||
// put the memory block into the linked list. We can placement construct the FreeList
|
// put the memory block into the linked list. We can placement construct the FreeList
|
||||||
// into the memory since we can be sure the alignment is correct.
|
// into the memory since we can be sure the alignment is correct.
|
||||||
|
ASAN_UNPOISON_MEMORY_REGION(p, sizeof(ListNode));
|
||||||
PlacementAddToList(p, m_free_lists[num_alignments]);
|
PlacementAddToList(p, m_free_lists[num_alignments]);
|
||||||
|
ASAN_POISON_MEMORY_REGION(p, std::max(bytes, sizeof(ListNode)));
|
||||||
} else {
|
} else {
|
||||||
// Can't use the pool => forward deallocation to ::operator delete().
|
// Can't use the pool => forward deallocation to ::operator delete().
|
||||||
::operator delete (p, std::align_val_t{alignment});
|
::operator delete (p, std::align_val_t{alignment});
|
||||||
|
@@ -6,6 +6,7 @@
|
|||||||
#define BITCOIN_TEST_UTIL_POOLRESOURCETESTER_H
|
#define BITCOIN_TEST_UTIL_POOLRESOURCETESTER_H
|
||||||
|
|
||||||
#include <support/allocators/pool.h>
|
#include <support/allocators/pool.h>
|
||||||
|
#include <util/check.h>
|
||||||
|
|
||||||
#include <algorithm>
|
#include <algorithm>
|
||||||
#include <cassert>
|
#include <cassert>
|
||||||
@@ -48,7 +49,10 @@ public:
|
|||||||
size_t size = 0;
|
size_t size = 0;
|
||||||
while (ptr != nullptr) {
|
while (ptr != nullptr) {
|
||||||
++size;
|
++size;
|
||||||
|
const auto* ptr_ = ptr;
|
||||||
|
ASAN_UNPOISON_MEMORY_REGION(ptr_, sizeof(typename PoolResource<MAX_BLOCK_SIZE_BYTES, ALIGN_BYTES>::ListNode));
|
||||||
ptr = ptr->m_next;
|
ptr = ptr->m_next;
|
||||||
|
ASAN_POISON_MEMORY_REGION(ptr_, sizeof(typename PoolResource<MAX_BLOCK_SIZE_BYTES, ALIGN_BYTES>::ListNode));
|
||||||
}
|
}
|
||||||
sizes.push_back(size);
|
sizes.push_back(size);
|
||||||
}
|
}
|
||||||
@@ -81,7 +85,10 @@ public:
|
|||||||
auto* ptr = resource.m_free_lists[freelist_idx];
|
auto* ptr = resource.m_free_lists[freelist_idx];
|
||||||
while (ptr != nullptr) {
|
while (ptr != nullptr) {
|
||||||
free_blocks.emplace_back(ptr, bytes);
|
free_blocks.emplace_back(ptr, bytes);
|
||||||
|
const auto* ptr_ = ptr;
|
||||||
|
ASAN_UNPOISON_MEMORY_REGION(ptr_, sizeof(typename PoolResource<MAX_BLOCK_SIZE_BYTES, ALIGN_BYTES>::ListNode));
|
||||||
ptr = ptr->m_next;
|
ptr = ptr->m_next;
|
||||||
|
ASAN_POISON_MEMORY_REGION(ptr_, sizeof(typename PoolResource<MAX_BLOCK_SIZE_BYTES, ALIGN_BYTES>::ListNode));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
// also add whatever has not yet been used for blocks
|
// also add whatever has not yet been used for blocks
|
||||||
|
@@ -126,4 +126,15 @@ constexpr T&& inline_assertion_check(LIFETIMEBOUND T&& val, [[maybe_unused]] con
|
|||||||
|
|
||||||
// NOLINTEND(bugprone-lambda-function-name)
|
// NOLINTEND(bugprone-lambda-function-name)
|
||||||
|
|
||||||
|
#if defined(__has_feature)
|
||||||
|
# if __has_feature(address_sanitizer)
|
||||||
|
# include <sanitizer/asan_interface.h>
|
||||||
|
# endif
|
||||||
|
#endif
|
||||||
|
|
||||||
|
#ifndef ASAN_POISON_MEMORY_REGION
|
||||||
|
# define ASAN_POISON_MEMORY_REGION(addr, size) ((void)(addr), (void)(size))
|
||||||
|
# define ASAN_UNPOISON_MEMORY_REGION(addr, size) ((void)(addr), (void)(size))
|
||||||
|
#endif
|
||||||
|
|
||||||
#endif // BITCOIN_UTIL_CHECK_H
|
#endif // BITCOIN_UTIL_CHECK_H
|
||||||
|
Reference in New Issue
Block a user