diff --git a/src/support/cleanse.h b/src/support/cleanse.h --- a/src/support/cleanse.h +++ b/src/support/cleanse.h @@ -8,7 +8,10 @@ #include -// Attempt to overwrite data in the specified memory span. +/** + * Secure overwrite a buffer (possibly containing secret data) with zero-bytes. + * The write operation will not be optimized out by the compiler. + */ void memory_cleanse(void *ptr, size_t len); #endif // BITCOIN_SUPPORT_CLEANSE_H diff --git a/src/support/cleanse.cpp b/src/support/cleanse.cpp --- a/src/support/cleanse.cpp +++ b/src/support/cleanse.cpp @@ -11,35 +11,27 @@ #include // For SecureZeroMemory. #endif -/** - * Compilers have a bad habit of removing "superfluous" memset calls that - * are trying to zero memory. For example, when memset()ing a buffer and - * then free()ing it, the compiler might decide that the memset is - * unobservable and thus can be removed. - * - * Previously we used OpenSSL which tried to stop this by a) implementing - * memset in assembly on x86 and b) putting the function in its own file - * for other platforms. - * - * This change removes those tricks in favor of using asm directives to - * scare the compiler away. As best as our compiler folks can tell, this is - * sufficient and will continue to be so. - * - * Adam Langley - * Commit: ad1907fe73334d6c696c8539646c21b11178f20f - * BoringSSL (LICENSE: ISC) - */ void memory_cleanse(void *ptr, size_t len) { - std::memset(ptr, 0, len); - - /** - * As best as we can tell, this is sufficient to break any optimizations - * that might try to eliminate "superfluous" memsets. If there's an easy way - * to detect memset_s, it would be better to use that. - */ #if defined(_MSC_VER) + /* SecureZeroMemory is guaranteed not to be optimized out by MSVC. */ SecureZeroMemory(ptr, len); #else + std::memset(ptr, 0, len); + + /* + * Memory barrier that scares the compiler away from optimizing out the + * memset. + * + * Quoting Adam Langley in commit + * ad1907fe73334d6c696c8539646c21b11178f20f in BoringSSL (ISC License): + * As best as we can tell, this is sufficient to break any optimisations + * that might try to eliminate "superfluous" memsets. + * This method is used in memzero_explicit() the Linux kernel, too. Its + * advantage is that it is pretty efficient because the compiler can still + * implement the memset() efficiently, just not remove it entirely. See + * "Dead Store Elimination (Still) Considered Harmful" by Yang et al. + * (USENIX Security 2017) for more background. + */ __asm__ __volatile__("" : : "r"(ptr) : "memory"); #endif }