sha1_hmac(), sha256_hmac() and the mbedTLS sha1 shim build the HMAC key
schedule in local arrays and clear them with memset() just before they
return. The buffers die with the frame, so the stores are dead and GCC
removes them from -O1 up, leaving the caller's key on the stack as
0x36 ^ key and 0x5c ^ key.

Those functions are generic; in U-Boot the caller is the TPM 1.2 code,
where the key is the usage auth of a TPM key.

Take memzero_explicit() from the Linux kernel, where it is a static inline
in include/linux/string.h, and use it for the three key schedules. Plain
memset() stays where a buffer is only being initialised.

Cc: Tom Rini <[email protected]>
Signed-off-by: Xiaoyi Suo <[email protected]>
---
 include/linux/string.h | 21 +++++++++++++++++++++
 lib/mbedtls/sha1.c     |  8 ++++----
 lib/sha1.c             |  8 ++++----
 lib/sha256.c           | 10 +++++-----
 4 files changed, 34 insertions(+), 13 deletions(-)

diff --git a/include/linux/string.h b/include/linux/string.h
index 986499dfc..a51b39e5d 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -3,6 +3,7 @@
 
 #include <linux/types.h>       /* for size_t */
 #include <linux/stddef.h>      /* for NULL */
+#include <linux/compiler.h>    /* for barrier_data() */
 
 #ifdef __cplusplus
 extern "C" {
@@ -106,6 +107,26 @@ extern void kfree_const(const void *x);
 
 #ifndef __HAVE_ARCH_MEMSET
 extern void * memset(void *,int,__kernel_size_t);
+
+/**
+ * memzero_explicit - Fill a region of memory (e.g. sensitive
+ *                   keying data) with 0s.
+ * @s: Pointer to the start of the area.
+ * @count: The size of the area.
+ *
+ * Note: usually using memset() is just fine (!), but in cases
+ * where clearing out _local_ data at the end of a scope is
+ * necessary, memzero_explicit() should be used instead in
+ * order to prevent the compiler from optimising away zeroing.
+ *
+ * memzero_explicit() doesn't need an arch-specific version as
+ * it just invokes the one of memset() implicitly.
+ */
+static inline void memzero_explicit(void *s, size_t count)
+{
+       memset(s, 0, count);
+       barrier_data(s);
+}
 #endif
 #ifndef __HAVE_ARCH_MEMCPY
 extern void * memcpy(void *,const void *,__kernel_size_t);
diff --git a/lib/mbedtls/sha1.c b/lib/mbedtls/sha1.c
index 2aee50377..c6b92fbe7 100644
--- a/lib/mbedtls/sha1.c
+++ b/lib/mbedtls/sha1.c
@@ -92,8 +92,8 @@ void sha1_hmac(const unsigned char *key, int keylen,
        sha1_update(&ctx, tmpbuf, sizeof(tmpbuf));
        sha1_finish(&ctx, output);
 
-       memset(k_ipad, 0, sizeof(k_ipad));
-       memset(k_opad, 0, sizeof(k_opad));
-       memset(tmpbuf, 0, sizeof(tmpbuf));
-       memset(&ctx, 0, sizeof(sha1_context));
+       memzero_explicit(k_ipad, sizeof(k_ipad));
+       memzero_explicit(k_opad, sizeof(k_opad));
+       memzero_explicit(tmpbuf, sizeof(tmpbuf));
+       memzero_explicit(&ctx, sizeof(sha1_context));
 }
diff --git a/lib/sha1.c b/lib/sha1.c
index be502c612..94f20fb40 100644
--- a/lib/sha1.c
+++ b/lib/sha1.c
@@ -373,10 +373,10 @@ void sha1_hmac(const unsigned char *key, int keylen,
        sha1_update (&ctx, tmpbuf, 20);
        sha1_finish (&ctx, output);
 
-       memset (k_ipad, 0, 64);
-       memset (k_opad, 0, 64);
-       memset (tmpbuf, 0, 20);
-       memset (&ctx, 0, sizeof (sha1_context));
+       memzero_explicit(k_ipad, 64);
+       memzero_explicit(k_opad, 64);
+       memzero_explicit(tmpbuf, 20);
+       memzero_explicit(&ctx, sizeof(sha1_context));
 }
 
 #ifdef SELF_TEST
diff --git a/lib/sha256.c b/lib/sha256.c
index c2e77c854..226af9b2d 100644
--- a/lib/sha256.c
+++ b/lib/sha256.c
@@ -307,11 +307,11 @@ int sha256_hmac(const unsigned char *key, int keylen,
        sha256_update(&ctx, tmpbuf, sizeof(tmpbuf));
        sha256_finish(&ctx, output);
 
-       memset(k_ipad, 0, sizeof(k_ipad));
-       memset(k_opad, 0, sizeof(k_opad));
-       memset(tmpbuf, 0, sizeof(tmpbuf));
-       memset(keybuf, 0, sizeof(keybuf));
-       memset(&ctx, 0, sizeof(sha256_context));
+       memzero_explicit(k_ipad, sizeof(k_ipad));
+       memzero_explicit(k_opad, sizeof(k_opad));
+       memzero_explicit(tmpbuf, sizeof(tmpbuf));
+       memzero_explicit(keybuf, sizeof(keybuf));
+       memzero_explicit(&ctx, sizeof(sha256_context));
 
        return 0;
 }
-- 
2.43.0

Reply via email to