From e006a4057fbbc87fd99974ff675d490f6ad8f86d Mon Sep 17 00:00:00 2001 From: Sebastian Messmer Date: Thu, 13 Sep 2018 07:56:59 -0700 Subject: [PATCH] Improve UnswappableAllocator for Windows. It is now guaranteed to not unlock memory too early because of close allocations next to it --- src/cpp-utils/system/memory.h | 7 ++++- src/cpp-utils/system/memory_windows.cpp | 39 +++++++++++++++++-------- 2 files changed, 33 insertions(+), 13 deletions(-) diff --git a/src/cpp-utils/system/memory.h b/src/cpp-utils/system/memory.h index fb754fd4..52dbd928 100644 --- a/src/cpp-utils/system/memory.h +++ b/src/cpp-utils/system/memory.h @@ -7,7 +7,12 @@ namespace cpputils { -// This allocator allocates memory that won't be swapped out to the disk, but will be kept in RAM +/** +* Allocator for security relevant memory like key data. +* The operating system will be given a hint that this memory shouldn't be swapped out to disk +* (which is, however, only a hint and might be ignored), +* and we'll make sure the memory is zeroed-out when deallocated. +*/ class UnswappableAllocator final : public Allocator { public: void* allocate(size_t size) override; diff --git a/src/cpp-utils/system/memory_windows.cpp b/src/cpp-utils/system/memory_windows.cpp index aabef3a7..ed96efaf 100644 --- a/src/cpp-utils/system/memory_windows.cpp +++ b/src/cpp-utils/system/memory_windows.cpp @@ -10,24 +10,39 @@ using namespace cpputils::logging; namespace cpputils { void* UnswappableAllocator::allocate(size_t size) { - void* data = DefaultAllocator().allocate(size); - const BOOL result = ::VirtualLock(data, size); - if (!result) { - throw std::runtime_error("Error calling VirtualLock. Errno: " + std::to_string(GetLastError())); - } + // VirtualAlloc allocates memory in full pages. This is needed, because VirtualUnlock unlocks full pages + // and might otherwise unlock unrelated memory of other allocations. + + // allocate pages + void* data = ::VirtualAlloc(nullptr, size, MEM_RESERVE | MEM_COMMIT, PAGE_READWRITE); + if (nullptr == data) { + throw std::runtime_error("Error calling VirtualAlloc. Errno: " + std::to_string(GetLastError())); + } + + // lock allocated pages into RAM + const BOOL success = ::VirtualLock(data, size); + if (!success) { + throw std::runtime_error("Error calling VirtualLock. Errno: " + std::to_string(GetLastError())); + } + return data; } void UnswappableAllocator::free(void* data, size_t size) { - const BOOL result = ::VirtualUnlock(data, size); - if (!result) { - LOG(WARN, "Error calling VirtualUnlock. Errno: {}", GetLastError()); - } + // overwrite the memory with zeroes before we free it + std::memset(data, 0, size); - // overwrite the memory with zeroes before we free it - std::memset(data, 0, size); + // unlock allocated pages from RAM + BOOL success = ::VirtualUnlock(data, size); + if (!success) { + throw std::runtime_error("Error calling VirtualUnlock. Errno: " + std::to_string(GetLastError())); + } - DefaultAllocator().free(data, size); + // free allocated pages + success = ::VirtualFree(data, 0, MEM_RELEASE); + if (!success) { + throw std::runtime_error("Error calling VirtualFree. Errno: " + std::to_string(GetLastError())); + } } }