aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--src/support/cleanse.cpp32
-rw-r--r--src/support/cleanse.h3
2 files changed, 14 insertions, 21 deletions
diff --git a/src/support/cleanse.cpp b/src/support/cleanse.cpp
index f895e96568..ecb00510f7 100644
--- a/src/support/cleanse.cpp
+++ b/src/support/cleanse.cpp
@@ -11,33 +11,25 @@
#include <Windows.h> // 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 favour 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 <agl@google.com>
- * Commit: ad1907fe73334d6c696c8539646c21b11178f20f
- * BoringSSL (LICENSE: ISC)
- */
void memory_cleanse(void *ptr, size_t len)
{
#if defined(_MSC_VER)
+ /* SecureZeroMemory is guaranteed not to be optimized out by MSVC. */
SecureZeroMemory(ptr, len);
#else
std::memset(ptr, 0, len);
- /* As best as we can tell, this is sufficient to break any optimisations that
- might try to eliminate "superfluous" memsets. If there's an easy way to
- detect memset_s, it would be better to use that. */
+ /* Memory barrier that scares the compiler away from optimizing out the memset.
+ *
+ * Quoting Adam Langley <agl@google.com> 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
}
diff --git a/src/support/cleanse.h b/src/support/cleanse.h
index 5298214e44..b03520315d 100644
--- a/src/support/cleanse.h
+++ b/src/support/cleanse.h
@@ -8,7 +8,8 @@
#include <stdlib.h>
-// 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