krb5 commit [krb5-1.14]: Make zap() more reliable

Tom Yu <[email protected]>
Newsgroups gmane.comp.encryption.kerberos.cvs
Message-ID <[email protected]>
https://github.com/krb5/krb5/commit/f468bec257787f0656c70ef1f247d43be2796245
commit f468bec257787f0656c70ef1f247d43be2796245
Author: Greg Hudson <[email protected]>
Date:   Mon Oct 31 11:48:54 2016 -0400

    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.14.5

 src/aclocal.m4         |    2 ++
 src/include/k5-int.h   |   43 +++++++++++++++++++++++--------------------
 src/util/support/zap.c |    5 ++++-
 3 files changed, 29 insertions(+), 21 deletions(-)

diff --git a/src/aclocal.m4 b/src/aclocal.m4
index dbb7db2..00db022 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 41c3d1b..22ddd72 100644
--- a/src/include/k5-int.h
+++ b/src/include/k5-int.h
@@ -639,30 +639,33 @@ krb5int_arcfour_gsscrypt(const krb5_keyblock *keyblock, krb5_keyusage usage,
  */
 #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';
 }
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.