Skip to content

Commit

Permalink
Merge bitcoin/bitcoin#27930: util: Don't derive secure_allocator from…
Browse files Browse the repository at this point in the history
… std::allocator

07c59ed Don't derive secure_allocator from std::allocator (Casey Carter)

Pull request description:

  Giving the C++ Standard Committee control of the public interface of your type means they will break it. C++23 adds a new `allocate_at_least` member to `std::allocator`. Very bad things happen when, say, `std::vector` uses `allocate_at_least` from `secure_allocator`'s base to allocate memory which it then tries to free with `secure_allocator::deallocate`.

  (Discovered by microsoft/STL#3712, which will be reverted by microsoft/STL#3819 before it ships.)

ACKs for top commit:
  jonatack:
    re-ACK 07c59ed no change since my previous ACK apart from squashing the commits
  achow101:
    ACK 07c59ed
  john-moffett:
    ACK 07c59ed Reviewed and tested. Performance appears unaffected in my environment.

Tree-SHA512: 23606c40414d325f5605a9244d4dd50907fdf5f2fbf70f336accb3a2cb98baa8acd2972f46eab1b7fdec1d28a843a96b06083cd2d09791cda7c90ee218e5bbd5
  • Loading branch information
achow101 committed Jul 25, 2023
2 parents 1ed8a0f + 07c59ed commit 32c1523
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 36 deletions.
36 changes: 17 additions & 19 deletions src/support/allocators/secure.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,27 +17,14 @@
// out of memory and clears its contents before deletion.
//
template <typename T>
struct secure_allocator : public std::allocator<T> {
using base = std::allocator<T>;
using traits = std::allocator_traits<base>;
using size_type = typename traits::size_type;
using difference_type = typename traits::difference_type;
using pointer = typename traits::pointer;
using const_pointer = typename traits::const_pointer;
using value_type = typename traits::value_type;
secure_allocator() noexcept {}
secure_allocator(const secure_allocator& a) noexcept : base(a) {}
struct secure_allocator {
using value_type = T;

secure_allocator() = default;
template <typename U>
secure_allocator(const secure_allocator<U>& a) noexcept : base(a)
{
}
~secure_allocator() noexcept {}
template <typename Other>
struct rebind {
typedef secure_allocator<Other> other;
};
secure_allocator(const secure_allocator<U>&) noexcept {}

T* allocate(std::size_t n, const void* hint = nullptr)
T* allocate(std::size_t n)
{
T* allocation = static_cast<T*>(LockedPoolManager::Instance().alloc(sizeof(T) * n));
if (!allocation) {
Expand All @@ -53,6 +40,17 @@ struct secure_allocator : public std::allocator<T> {
}
LockedPoolManager::Instance().free(p);
}

template <typename U>
friend bool operator==(const secure_allocator&, const secure_allocator<U>&) noexcept
{
return true;
}
template <typename U>
friend bool operator!=(const secure_allocator&, const secure_allocator<U>&) noexcept
{
return false;
}
};

// This is exactly like std::string, but with a custom allocator.
Expand Down
39 changes: 22 additions & 17 deletions src/support/allocators/zeroafterfree.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,31 +12,36 @@
#include <vector>

template <typename T>
struct zero_after_free_allocator : public std::allocator<T> {
using base = std::allocator<T>;
using traits = std::allocator_traits<base>;
using size_type = typename traits::size_type;
using difference_type = typename traits::difference_type;
using pointer = typename traits::pointer;
using const_pointer = typename traits::const_pointer;
using value_type = typename traits::value_type;
zero_after_free_allocator() noexcept {}
zero_after_free_allocator(const zero_after_free_allocator& a) noexcept : base(a) {}
struct zero_after_free_allocator {
using value_type = T;

zero_after_free_allocator() noexcept = default;
template <typename U>
zero_after_free_allocator(const zero_after_free_allocator<U>& a) noexcept : base(a)
zero_after_free_allocator(const zero_after_free_allocator<U>&) noexcept
{
}

T* allocate(std::size_t n)
{
return std::allocator<T>{}.allocate(n);
}
~zero_after_free_allocator() noexcept {}
template <typename Other>
struct rebind {
typedef zero_after_free_allocator<Other> other;
};

void deallocate(T* p, std::size_t n)
{
if (p != nullptr)
memory_cleanse(p, sizeof(T) * n);
std::allocator<T>::deallocate(p, n);
std::allocator<T>{}.deallocate(p, n);
}

template <typename U>
friend bool operator==(const zero_after_free_allocator&, const zero_after_free_allocator<U>&) noexcept
{
return true;
}
template <typename U>
friend bool operator!=(const zero_after_free_allocator&, const zero_after_free_allocator<U>&) noexcept
{
return false;
}
};

Expand Down

0 comments on commit 32c1523

Please sign in to comment.