From 2a67ddb38a564b7b9c36c2d0b2b95879616918d1 Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Wed, 24 Oct 2018 10:05:17 +0200 Subject: [PATCH 1/6] Add pooled_secure_allocator and mt_pooled_secure_allocator --- src/support/allocators/mt_pooled_secure.h | 86 +++++++++++++++++++++++ src/support/allocators/pooled_secure.h | 72 +++++++++++++++++++ 2 files changed, 158 insertions(+) create mode 100644 src/support/allocators/mt_pooled_secure.h create mode 100644 src/support/allocators/pooled_secure.h diff --git a/src/support/allocators/mt_pooled_secure.h b/src/support/allocators/mt_pooled_secure.h new file mode 100644 index 0000000000000..dc7a34d9fdb69 --- /dev/null +++ b/src/support/allocators/mt_pooled_secure.h @@ -0,0 +1,86 @@ +// Copyright (c) 2009-2010 Satoshi Nakamoto +// Copyright (c) 2009-2015 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_SUPPORT_ALLOCATORS_MT_POOLED_SECURE_H +#define BITCOIN_SUPPORT_ALLOCATORS_MT_POOLED_SECURE_H + +#include "pooled_secure.h" + +#include +#include + +// +// Manages a pool of pools to balance allocation between those when multiple threads are involved +// This allocator is fully thread safe +// +template +struct mt_pooled_secure_allocator : public std::allocator { + // MSVC8 default copy constructor is broken + typedef std::allocator base; + typedef typename base::size_type size_type; + typedef typename base::difference_type difference_type; + typedef typename base::pointer pointer; + typedef typename base::const_pointer const_pointer; + typedef typename base::reference reference; + typedef typename base::const_reference const_reference; + typedef typename base::value_type value_type; + mt_pooled_secure_allocator(size_type nrequested_size = 32, + size_type nnext_size = 32, + size_type nmax_size = 0) throw() + { + // we add enough bytes to the requested size so that we can store the bucket as well + nrequested_size += sizeof(size_t); + + size_t pools_count = std::thread::hardware_concurrency(); + pools.resize(pools_count); + for (size_t i = 0; i < pools_count; i++) { + pools[i] = std::make_unique(nrequested_size, nnext_size, nmax_size); + } + } + ~mt_pooled_secure_allocator() throw() {} + + T* allocate(std::size_t n, const void* hint = 0) + { + size_t bucket = get_bucket(); + std::lock_guard lock(pools[bucket]->mutex); + uint8_t* ptr = pools[bucket]->allocate(n * sizeof(T) + sizeof(size_t)); + *(size_t*)ptr = bucket; + return static_cast(ptr + sizeof(size_t)); + } + + void deallocate(T* p, std::size_t n) + { + if (!p) { + return; + } + uint8_t* ptr = (uint8_t*)p - sizeof(size_t); + size_t bucket = *(size_t*)ptr; + std::lock_guard lock(pools[bucket]->mutex); + pools[bucket]->deallocate(ptr, n * sizeof(T)); + } + +private: + size_t get_bucket() + { + auto tid = std::this_thread::get_id(); + size_t x = std::hash{}(std::this_thread::get_id()); + return x % pools.size(); + } + + struct internal_pool : pooled_secure_allocator { + internal_pool(size_type nrequested_size, + size_type nnext_size, + size_type nmax_size) : + pooled_secure_allocator(nrequested_size, nnext_size, nmax_size) + { + } + std::mutex mutex; + }; + +private: + std::vector> pools; +}; + +#endif // BITCOIN_SUPPORT_ALLOCATORS_MT_POOLED_SECURE_H diff --git a/src/support/allocators/pooled_secure.h b/src/support/allocators/pooled_secure.h new file mode 100644 index 0000000000000..3d88850adfb18 --- /dev/null +++ b/src/support/allocators/pooled_secure.h @@ -0,0 +1,72 @@ +// Copyright (c) 2009-2010 Satoshi Nakamoto +// Copyright (c) 2009-2015 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_SUPPORT_ALLOCATORS_POOLED_SECURE_H +#define BITCOIN_SUPPORT_ALLOCATORS_POOLED_SECURE_H + +#include "support/lockedpool.h" +#include "support/cleanse.h" + +#include +#include + +#include + +// +// Allocator that allocates memory in chunks from a pool, which in turn allocates larger chunks from secure memory +// Memory is cleaned when freed as well. This allocator is NOT thread safe +// +template +struct pooled_secure_allocator : public std::allocator { + // MSVC8 default copy constructor is broken + typedef std::allocator base; + typedef typename base::size_type size_type; + typedef typename base::difference_type difference_type; + typedef typename base::pointer pointer; + typedef typename base::const_pointer const_pointer; + typedef typename base::reference reference; + typedef typename base::const_reference const_reference; + typedef typename base::value_type value_type; + pooled_secure_allocator(const size_type nrequested_size = 32, + const size_type nnext_size = 32, + const size_type nmax_size = 0) throw() : + pool(nrequested_size, nnext_size, nmax_size){} + ~pooled_secure_allocator() throw() {} + + T* allocate(std::size_t n, const void* hint = 0) + { + size_t chunks = (n * sizeof(T) + pool.get_requested_size() - 1) / pool.get_requested_size(); + return static_cast(pool.ordered_malloc(chunks)); + } + + void deallocate(T* p, std::size_t n) + { + size_t chunks = (n * sizeof(T) + pool.get_requested_size() - 1) / pool.get_requested_size(); + if (p != NULL) { + memory_cleanse(p, chunks * pool.get_requested_size()); + } + pool.ordered_free(p, chunks); + } + +public: + struct internal_secure_allocator { + typedef std::size_t size_type; + typedef std::ptrdiff_t difference_type; + + static char* malloc(const size_type bytes) + { + return static_cast(LockedPoolManager::Instance().alloc(bytes)); + } + + static void free(char* const block) + { + LockedPoolManager::Instance().free(block); + } + }; +private: + boost::pool pool; +}; + +#endif // BITCOIN_SUPPORT_ALLOCATORS_POOLED_SECURE_H From 1678d75fbeabae5010315cece4fc64e74f11f51d Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Wed, 24 Oct 2018 13:05:54 +0200 Subject: [PATCH 2/6] Use mt_pooled_secure_allocator for BLS secure allocation The old solution relied on thread-local-storage and was thus not compatible to libc6 2.11 (which is the minimum supported version we use). Also, the old solution turned out to be erroneous. It would have crashed or memory leaked when ownership of CBLSPrivateKey would be handled over to another thread. --- src/bls/bls.cpp | 51 ++++++++----------------------------------------- 1 file changed, 8 insertions(+), 43 deletions(-) diff --git a/src/bls/bls.cpp b/src/bls/bls.cpp index 83cac42f77117..d09fa5e0fadcf 100644 --- a/src/bls/bls.cpp +++ b/src/bls/bls.cpp @@ -9,8 +9,7 @@ #include "tinyformat.h" #ifndef BUILD_BITCOIN_INTERNAL -#include "support/allocators/secure.h" -#include +#include "support/allocators/mt_pooled_secure.h" #endif #include @@ -425,55 +424,21 @@ bool CBLSSignature::Recover(const std::vector& sigs, const std::v } #ifndef BUILD_BITCOIN_INTERNAL -struct secure_user_allocator { - typedef std::size_t size_type; - typedef std::ptrdiff_t difference_type; - static char* malloc(const size_type bytes) - { - return static_cast(LockedPoolManager::Instance().alloc(bytes)); - } - - static void free(char* const block) - { - LockedPoolManager::Instance().free(block); - } -}; - -// every thread has it's own pool allocator for secure data to speed things up -// otherwise locking of mutexes slows down the system at places were you'd never expect it -// downside is that we must make sure that all threads have destroyed their copy of the pool before the global -// LockedPool is destroyed. This means that all worker threads must finish before static destruction begins -// we use sizeof(bn_t) as the pool request size as this is what Chia's BLS library will request in most cases -// In case something larger is requested, we directly call into LockedPool and accept the slowness -thread_local static boost::pool securePool(sizeof(bn_t) + sizeof(size_t)); +static mt_pooled_secure_allocator g_blsSecureAllocator(sizeof(bn_t) + sizeof(size_t)); static void* secure_allocate(size_t n) { - void* p; - if (n <= securePool.get_requested_size() - sizeof(size_t)) { - p = securePool.ordered_malloc(); - } else { - p = secure_user_allocator::malloc(n + sizeof(size_t)); - } - *(size_t*)p = n; - p = (uint8_t*)p + sizeof(size_t); - return p; + uint8_t* ptr = g_blsSecureAllocator.allocate(n + sizeof(size_t)); + *(size_t*)ptr = n; + return ptr + sizeof(size_t); } static void secure_free(void* p) { - if (!p) { - return; - } - p = (uint8_t*)p - sizeof(size_t); - size_t n = *(size_t*)p; - memory_cleanse(p, n + sizeof(size_t)); - if (n <= securePool.get_requested_size() - sizeof(size_t)) { - securePool.ordered_free(p); - } else { - secure_user_allocator::free((char*)p); - } + uint8_t* ptr = (uint8_t*)p - sizeof(size_t); + size_t n = *(size_t*)ptr; + return g_blsSecureAllocator.deallocate(ptr, n); } #endif From fdbf68a4121ca610f09ebbbaffb02e1e6acffaaf Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Wed, 24 Oct 2018 15:05:52 +0200 Subject: [PATCH 3/6] Add new header files to Makefile.am --- src/Makefile.am | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Makefile.am b/src/Makefile.am index 8cbd73d0f3f22..659af594ff7d6 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -172,6 +172,8 @@ BITCOIN_CORE_H = \ script/ismine.h \ spork.h \ streams.h \ + support/allocators/mt_pooled_secure.h \ + support/allocators/pooled_secure.h \ support/allocators/secure.h \ support/allocators/zeroafterfree.h \ support/cleanse.h \ From 37ff5f1e2e2d85e50c2584623ecc45dcca8e3827 Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Wed, 24 Oct 2018 15:18:42 +0200 Subject: [PATCH 4/6] Fix copyright headers of new files --- src/support/allocators/mt_pooled_secure.h | 3 +-- src/support/allocators/pooled_secure.h | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/support/allocators/mt_pooled_secure.h b/src/support/allocators/mt_pooled_secure.h index dc7a34d9fdb69..d857da1616477 100644 --- a/src/support/allocators/mt_pooled_secure.h +++ b/src/support/allocators/mt_pooled_secure.h @@ -1,5 +1,4 @@ -// Copyright (c) 2009-2010 Satoshi Nakamoto -// Copyright (c) 2009-2015 The Bitcoin Core developers +// Copyright (c) 2014-2018 The Dash Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. diff --git a/src/support/allocators/pooled_secure.h b/src/support/allocators/pooled_secure.h index 3d88850adfb18..86b1489fef11d 100644 --- a/src/support/allocators/pooled_secure.h +++ b/src/support/allocators/pooled_secure.h @@ -1,5 +1,4 @@ -// Copyright (c) 2009-2010 Satoshi Nakamoto -// Copyright (c) 2009-2015 The Bitcoin Core developers +// Copyright (c) 2014-2018 The Dash Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. From f63cfd5b844e07ce8c33ead1cee6889a14e0a8ca Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Wed, 24 Oct 2018 16:44:24 +0200 Subject: [PATCH 5/6] Bail out early from secure deallocation --- src/bls/bls.cpp | 4 ++++ src/support/allocators/pooled_secure.h | 8 +++++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/bls/bls.cpp b/src/bls/bls.cpp index d09fa5e0fadcf..8de75f42b28ad 100644 --- a/src/bls/bls.cpp +++ b/src/bls/bls.cpp @@ -436,6 +436,10 @@ static void* secure_allocate(size_t n) static void secure_free(void* p) { + if (!p) { + return; + } + uint8_t* ptr = (uint8_t*)p - sizeof(size_t); size_t n = *(size_t*)ptr; return g_blsSecureAllocator.deallocate(ptr, n); diff --git a/src/support/allocators/pooled_secure.h b/src/support/allocators/pooled_secure.h index 86b1489fef11d..03fb2610ce105 100644 --- a/src/support/allocators/pooled_secure.h +++ b/src/support/allocators/pooled_secure.h @@ -42,10 +42,12 @@ struct pooled_secure_allocator : public std::allocator { void deallocate(T* p, std::size_t n) { - size_t chunks = (n * sizeof(T) + pool.get_requested_size() - 1) / pool.get_requested_size(); - if (p != NULL) { - memory_cleanse(p, chunks * pool.get_requested_size()); + if (!p) { + return; } + + size_t chunks = (n * sizeof(T) + pool.get_requested_size() - 1) / pool.get_requested_size(); + memory_cleanse(p, chunks * pool.get_requested_size()); pool.ordered_free(p, chunks); } From d6d140385f236ea6a7fc839979c12e4820e53910 Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Thu, 25 Oct 2018 13:41:59 +0200 Subject: [PATCH 6/6] Clean up global BLS keys when shutting down --- src/init.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/init.cpp b/src/init.cpp index eaa8b2a697f0d..5e65f783aee71 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -324,6 +324,10 @@ void PrepareShutdown() UnregisterValidationInterface(activeMasternodeManager); } + // make sure to clean up BLS keys before global destructors are called (they have allocated from the secure memory pool) + activeMasternodeInfo.blsKeyOperator.reset(); + activeMasternodeInfo.blsPubKeyOperator.reset(); + #ifndef WIN32 try { boost::filesystem::remove(GetPidFile());