Thread (4 messages) 4 messages, 2 authors, 8d ago
COOLING8d

[PATCH 1/2] lib: add memzero_explicit() and use it for the HMAC key schedules

From: Xiaoyi Suo <hidden>
Date: 2026-09-21 23:08:05
Subsystem: generic string library, library code, the rest · Maintainers: Kees Cook, Andrew Morton, Linus Torvalds

memset() is enough for correctness but not for security. When the buffer
being cleared dies right after the clear - a local that goes out of
scope, or a block that is freed - the store is dead as far as the
compiler is concerned, and dead store elimination removes it. The buffer
then keeps whatever it held, and the clear only looks like it happened.

sha1_hmac(), sha256_hmac() and the mbedTLS sha1 shim all build the HMAC key
schedule in local arrays and clear them with memset() before returning:

	lib/sha1.c:376-379	  k_ipad, k_opad, tmpbuf, ctx
	lib/sha256.c:310-314	  k_ipad, k_opad, tmpbuf, keybuf, ctx
	lib/mbedtls/sha1.c:95-98  k_ipad, k_opad, tmpbuf, ctx

Those buffers hold the caller's key: it is XORed into the two 64-byte
pads, so once the clear is gone the key can be read back out of the stack
frame with the padding constants (key[i] = k_ipad[i] ^ 0x36). In U-Boot
the caller is the TPMv1 code, where the key is the usage auth of a TPM
key, but the functions themselves are generic. That the intent is real is
not in doubt: the same tree already zeroizes a hash context with a
comment saying "In case it's sensitive" (lib/md5.c) and clears the SM3
schedule under "Zeroize sensitive information." (lib/sm3.c).

GCC deletes the four stores in lib/sha1.c and the five in lib/sha256.c from
-O1 up; -fdump-tree-dse-details names them:

	Deleted dead call: memset (&k_ipad, 0, 64);
	Deleted dead call: memset (&k_opad, 0, 64);

Add memzero_explicit(), which is a memset() the compiler may not remove,
and use it for those buffers. Plain memset() stays where a buffer is only
being initialised. This changes no behaviour: it is a clear that now
actually happens.

Cc: Tom Rini <redacted>
Signed-off-by: Xiaoyi Suo <redacted>
---
 include/linux/string.h |  7 +++++++
 lib/mbedtls/sha1.c     |  8 ++++----
 lib/sha1.c             |  8 ++++----
 lib/sha256.c           | 10 +++++-----
 lib/string.c           | 21 +++++++++++++++++++++
 5 files changed, 41 insertions(+), 13 deletions(-)
diff --git a/include/linux/string.h b/include/linux/string.h
index 986499dfc..f902b2b5a 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -126,6 +126,13 @@ extern void * memchr(const void *,int,__kernel_size_t);
 void *memchr_inv(const void *, int, size_t);
 #endif
 
+/*
+ * memset() that the compiler may not remove. Use it for buffers that must
+ * not be left behind, where the buffer dies right after the clear: a local
+ * that goes out of scope, or a block that is freed.
+ */
+void memzero_explicit(void *s, size_t count);
+
 /**
  * memdup() - allocate a buffer and copy in the contents
  *
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;
 }
diff --git a/lib/string.c b/lib/string.c
index dbf2ce340..8ccb7c17e 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -808,3 +808,24 @@ void *memchr_inv(const void *start, int c, size_t bytes)
 	return check_bytes8(start, value, bytes % 8);
 }
 #endif
+
+/**
+ * memzero_explicit - clear a buffer that the compiler may not remove
+ * @s: the buffer to clear
+ * @count: the number of bytes to write
+ *
+ * memset() is enough for correctness but not for security. When the buffer
+ * being cleared dies right after the clear - a local that goes out of scope,
+ * or a block that is freed - the store is dead as far as the compiler is
+ * concerned and dead store elimination removes it, so the buffer keeps
+ * whatever it held. The barrier below tells the compiler that the memory may
+ * be read, which stops that.
+ *
+ * Use it for key material and for any other data that must not outlive its
+ * use; keep plain memset() for buffers that are only being initialised.
+ */
+void memzero_explicit(void *s, size_t count)
+{
+	memset(s, 0, count);
+	barrier_data(s);
+}
-- 
2.43.0
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help