diff options
author | Greg Hudson <ghudson@mit.edu> | 2016-10-31 11:48:54 -0400 |
---|---|---|
committer | Tom Yu <tlyu@mit.edu> | 2016-11-09 13:53:54 -0500 |
commit | 100e4aa16ea03faac8346382ee0ba07c84c3df45 (patch) | |
tree | a1ec3cb3976c6e8e6632c405f31136354e199c9d | |
parent | 71a11259a401aac2997193271c69bffce0260141 (diff) | |
download | krb5-100e4aa16ea03faac8346382ee0ba07c84c3df45.zip krb5-100e4aa16ea03faac8346382ee0ba07c84c3df45.tar.gz krb5-100e4aa16ea03faac8346382ee0ba07c84c3df45.tar.bz2 |
Make zap() more reliable
The gcc assembly version of zap() could still be optimized out under
gcc 5.1 or later, and the krb5int_zap() function could be optimized
out with link-time optimization. Based on work by Zhaomo Yang and
Brian Johannesmeyer, use the C11 memset_s() when available, then fall
back to a memory barrier with gcc or clang, and finally fall back to
using krb5int_zap(). Modify krb5int_zap() to use a volatile pointer
in case link-time optimization is used.
(cherry picked from commit c163275f899b201dc2807b3ff2949d5e2ee7d838)
ticket: 8514
version_fixed: 1.15
-rw-r--r-- | src/aclocal.m4 | 2 | ||||
-rw-r--r-- | src/include/k5-int.h | 43 | ||||
-rw-r--r-- | src/util/support/zap.c | 5 |
3 files changed, 29 insertions, 21 deletions
diff --git a/src/aclocal.m4 b/src/aclocal.m4 index bd2eb48..9c46da4 100644 --- a/src/aclocal.m4 +++ b/src/aclocal.m4 @@ -53,6 +53,8 @@ AC_SUBST(EXTRA_FILES) dnl Consider using AC_USE_SYSTEM_EXTENSIONS when we require autoconf dnl 2.59c or later, but be sure to test on Solaris first. AC_DEFINE([_GNU_SOURCE], 1, [Define to enable extensions in glibc]) +AC_DEFINE([__STDC_WANT_LIB_EXT1__], 1, [Define to enable C11 extensions]) + WITH_CC dnl AC_REQUIRE_CPP if test -z "$LD" ; then LD=$CC; fi diff --git a/src/include/k5-int.h b/src/include/k5-int.h index 3cc32c3..6499173 100644 --- a/src/include/k5-int.h +++ b/src/include/k5-int.h @@ -652,30 +652,33 @@ k5_sha256(const krb5_data *in, uint8_t out[K5_SHA256_HASHLEN]); */ #ifdef _WIN32 # define zap(ptr, len) SecureZeroMemory(ptr, len) -#elif defined(__GNUC__) +#elif defined(__STDC_LIB_EXT1__) +/* + * Use memset_s() which cannot be optimized out. Avoid memset_s(NULL, 0, 0, 0) + * which would cause a runtime constraint violation. + */ static inline void zap(void *ptr, size_t len) { - memset(ptr, 0, len); - /* - * Some versions of gcc have gotten clever enough to eliminate a - * memset call right before the block in question is released. - * This (empty) asm requires it to assume that we're doing - * something interesting with the stored (zero) value, so the - * memset can't be eliminated. - * - * An optimizer that looks at assembly or object code may not be - * fooled, and may still cause the memset to go away. Address - * that problem if and when we encounter it. - * - * This also may not be enough if free() does something - * interesting like purge memory locations from a write-back cache - * that hasn't written back the zero bytes yet. A memory barrier - * instruction would help in that case. - */ - asm volatile ("" : : "g" (ptr), "g" (len)); + if (len > 0) + memset_s(ptr, len, 0, len); +} +#elif defined(__GNUC__) || defined(__clang__) +/* + * Use an asm statement which declares a memory clobber to force the memset to + * be carried out. Avoid memset(NULL, 0, 0) which has undefined behavior. + */ +static inline void zap(void *ptr, size_t len) +{ + if (len > 0) + memset(ptr, 0, len); + __asm__ __volatile__("" : : "r" (ptr) : "memory"); } #else -/* Use a function from libkrb5support to defeat inlining. */ +/* + * Use a function from libkrb5support to defeat inlining unless link-time + * optimization is used. The function uses a volatile pointer, which prevents + * current compilers from optimizing out the memset. + */ # define zap(ptr, len) krb5int_zap(ptr, len) #endif diff --git a/src/util/support/zap.c b/src/util/support/zap.c index 48512a9..ed31630 100644 --- a/src/util/support/zap.c +++ b/src/util/support/zap.c @@ -34,5 +34,8 @@ void krb5int_zap(void *ptr, size_t len) { - memset(ptr, 0, len); + volatile char *p = ptr; + + while (len--) + *p++ = '\0'; } |