From f7b61b26178290a7a25a4792684e58e80725bf6e Mon Sep 17 00:00:00 2001
From: Milan Broz <gmazyland@gmail.com>
Date: Sun, 11 Jan 2015 20:26:45 +0100
Subject: [PATCH] Prevent compiler to optiize-out memset for on-stack
 variables.

Also see
https://cryptocoding.net/index.php/Coding_rules#Prevent_compiler_interference_with_security-critical_operations

The used code is inspired by the code in Blake2 implementation.
---
 lib/crypto_backend/crypto_backend.h       |  7 +++++++
 lib/crypto_backend/crypto_cipher_kernel.c |  2 +-
 lib/crypto_backend/crypto_nss.c           |  4 ++--
 lib/crypto_backend/crypto_openssl.c       |  4 ++--
 lib/crypto_backend/crypto_storage.c       |  4 ++--
 lib/crypto_backend/pbkdf2_generic.c       |  8 ++++----
 lib/internal.h                            |  1 +
 lib/luks1/keymanage.c                     |  6 +++---
 lib/setup.c                               |  4 ++--
 lib/tcrypt/tcrypt.c                       | 22 +++++++++++-----------
 lib/utils_crypt.c                         | 16 ++++++++++++++--
 lib/volumekey.c                           |  4 ++--
 12 files changed, 51 insertions(+), 31 deletions(-)

diff --git a/lib/crypto_backend/crypto_backend.h b/lib/crypto_backend/crypto_backend.h
index dbe7a3a4..0aab38c9 100644
--- a/lib/crypto_backend/crypto_backend.h
+++ b/lib/crypto_backend/crypto_backend.h
@@ -102,4 +102,11 @@ int crypt_storage_decrypt(struct crypt_storage *ctx, uint64_t sector,
 int crypt_storage_encrypt(struct crypt_storage *ctx, uint64_t sector,
 			  size_t count, char *buffer);
 
+/* Memzero helper (memset on stack can be optimized out) */
+static inline void crypt_backend_memzero(void *s, size_t n)
+{
+	volatile uint8_t *p = (volatile uint8_t *)s;
+	while(n--) *p++ = 0;
+}
+
 #endif /* _CRYPTO_BACKEND_H */
diff --git a/lib/crypto_backend/crypto_cipher_kernel.c b/lib/crypto_backend/crypto_cipher_kernel.c
index 0be04b49..f7d2bcf2 100644
--- a/lib/crypto_backend/crypto_cipher_kernel.c
+++ b/lib/crypto_backend/crypto_cipher_kernel.c
@@ -217,7 +217,7 @@ static int crypt_cipher_crypt(struct crypt_cipher *ctx,
 	if (len != (ssize_t)length)
 		r = -EIO;
 bad:
-	memset(buffer, 0, sizeof(buffer));
+	crypt_backend_memzero(buffer, sizeof(buffer));
 	return r;
 }
 
diff --git a/lib/crypto_backend/crypto_nss.c b/lib/crypto_backend/crypto_nss.c
index c32e8e17..4b9f943b 100644
--- a/lib/crypto_backend/crypto_nss.c
+++ b/lib/crypto_backend/crypto_nss.c
@@ -164,7 +164,7 @@ int crypt_hash_final(struct crypt_hash *ctx, char *buffer, size_t length)
 		return -EINVAL;
 
 	memcpy(buffer, tmp, length);
-	memset(tmp, 0, sizeof(tmp));
+	crypt_backend_memzero(tmp, sizeof(tmp));
 
 	if (tmp_len < length)
 		return -EINVAL;
@@ -266,7 +266,7 @@ int crypt_hmac_final(struct crypt_hmac *ctx, char *buffer, size_t length)
 		return -EINVAL;
 
 	memcpy(buffer, tmp, length);
-	memset(tmp, 0, sizeof(tmp));
+	crypt_backend_memzero(tmp, sizeof(tmp));
 
 	if (tmp_len < length)
 		return -EINVAL;
diff --git a/lib/crypto_backend/crypto_openssl.c b/lib/crypto_backend/crypto_openssl.c
index 8deb2af6..5e4345b3 100644
--- a/lib/crypto_backend/crypto_openssl.c
+++ b/lib/crypto_backend/crypto_openssl.c
@@ -133,7 +133,7 @@ int crypt_hash_final(struct crypt_hash *ctx, char *buffer, size_t length)
 		return -EINVAL;
 
 	memcpy(buffer, tmp, length);
-	memset(tmp, 0, sizeof(tmp));
+	crypt_backend_memzero(tmp, sizeof(tmp));
 
 	if (tmp_len < length)
 		return -EINVAL;
@@ -203,7 +203,7 @@ int crypt_hmac_final(struct crypt_hmac *ctx, char *buffer, size_t length)
 	HMAC_Final(&ctx->md, tmp, &tmp_len);
 
 	memcpy(buffer, tmp, length);
-	memset(tmp, 0, sizeof(tmp));
+	crypt_backend_memzero(tmp, sizeof(tmp));
 
 	if (tmp_len < length)
 		return -EINVAL;
diff --git a/lib/crypto_backend/crypto_storage.c b/lib/crypto_backend/crypto_storage.c
index 75550f88..a567f8b3 100644
--- a/lib/crypto_backend/crypto_storage.c
+++ b/lib/crypto_backend/crypto_storage.c
@@ -103,13 +103,13 @@ static int crypt_sector_iv_init(struct crypt_sector_iv *ctx,
 		r = crypt_hash_final(h, tmp, hash_size);
 		crypt_hash_destroy(h);
 		if (r) {
-			memset(tmp, 0, sizeof(tmp));
+			crypt_backend_memzero(tmp, sizeof(tmp));
 			return r;
 		}
 
 		r = crypt_cipher_init(&ctx->essiv_cipher, cipher_name, "ecb",
 				      tmp, hash_size);
-		memset(tmp, 0, sizeof(tmp));
+		crypt_backend_memzero(tmp, sizeof(tmp));
 		if (r)
 			return r;
 
diff --git a/lib/crypto_backend/pbkdf2_generic.c b/lib/crypto_backend/pbkdf2_generic.c
index e4313aed..2e9a3d24 100644
--- a/lib/crypto_backend/pbkdf2_generic.c
+++ b/lib/crypto_backend/pbkdf2_generic.c
@@ -188,7 +188,7 @@ int pkcs5_pbkdf2(const char *hash,
 
 		if (crypt_hmac_init(&hmac, hash, P_hash, hLen))
 			return -EINVAL;
-		memset(P_hash, 0, sizeof(P_hash));
+		crypt_backend_memzero(P_hash, sizeof(P_hash));
 	} else {
 		if (crypt_hmac_init(&hmac, hash, P, Plen))
 			return -EINVAL;
@@ -224,9 +224,9 @@ int pkcs5_pbkdf2(const char *hash,
 	rc = 0;
 out:
 	crypt_hmac_destroy(hmac);
-	memset(U, 0, sizeof(U));
-	memset(T, 0, sizeof(T));
-	memset(tmp, 0, tmplen);
+	crypt_backend_memzero(U, sizeof(U));
+	crypt_backend_memzero(T, sizeof(T));
+	crypt_backend_memzero(tmp, tmplen);
 
 	return rc;
 }
diff --git a/lib/internal.h b/lib/internal.h
index fb265eef..6a325564 100644
--- a/lib/internal.h
+++ b/lib/internal.h
@@ -57,6 +57,7 @@ struct volume_key {
 	char key[];
 };
 
+void crypt_memzero(void *s, size_t n);
 struct volume_key *crypt_alloc_volume_key(unsigned keylength, const char *key);
 struct volume_key *crypt_generate_volume_key(struct crypt_device *cd, unsigned keylength);
 void crypt_free_volume_key(struct volume_key *vk);
diff --git a/lib/luks1/keymanage.c b/lib/luks1/keymanage.c
index f755824b..f2e5b5de 100644
--- a/lib/luks1/keymanage.c
+++ b/lib/luks1/keymanage.c
@@ -212,7 +212,7 @@ int LUKS_hdr_backup(const char *backup_file, struct crypt_device *ctx)
 out:
 	if (devfd != -1)
 		close(devfd);
-	memset(&hdr, 0, sizeof(hdr));
+	crypt_memzero(&hdr, sizeof(hdr));
 	crypt_safe_free(buffer);
 	return r;
 }
@@ -398,7 +398,7 @@ static int _keyslot_repair(struct luks_phdr *phdr, struct crypt_device *ctx)
 	}
 out:
 	crypt_free_volume_key(vk);
-	memset(&temp_phdr, 0, sizeof(temp_phdr));
+	crypt_memzero(&temp_phdr, sizeof(temp_phdr));
 	return r;
 }
 
@@ -618,7 +618,7 @@ static int LUKS_check_cipher(struct luks_phdr *hdr, struct crypt_device *ctx)
 				      empty_key, 0, ctx);
 
 	crypt_free_volume_key(empty_key);
-	memset(buf, 0, sizeof(buf));
+	crypt_memzero(buf, sizeof(buf));
 	return r;
 }
 
diff --git a/lib/setup.c b/lib/setup.c
index 0ca9e118..00a38cae 100644
--- a/lib/setup.c
+++ b/lib/setup.c
@@ -1453,7 +1453,7 @@ int crypt_header_restore(struct crypt_device *cd,
 
 	r = LUKS_hdr_restore(backup_file, isLUKS(cd->type) ? &cd->u.luks1.hdr : &hdr, cd);
 
-	memset(&hdr, 0, sizeof(hdr));
+	crypt_memzero(&hdr, sizeof(hdr));
 	return r;
 }
 
@@ -1486,7 +1486,7 @@ void crypt_free(struct crypt_device *cd)
 
 		free(cd->type);
 		/* Some structures can contain keys (TCRYPT), wipe it */
-		memset(cd, 0, sizeof(*cd));
+		crypt_memzero(cd, sizeof(*cd));
 		free(cd);
 	}
 }
diff --git a/lib/tcrypt/tcrypt.c b/lib/tcrypt/tcrypt.c
index 4d07a176..eca9302c 100644
--- a/lib/tcrypt/tcrypt.c
+++ b/lib/tcrypt/tcrypt.c
@@ -269,8 +269,8 @@ static int decrypt_blowfish_le_cbc(struct tcrypt_alg *alg,
 	}
 
 	crypt_cipher_destroy(cipher);
-	memset(iv, 0, bs);
-	memset(iv_old, 0, bs);
+	crypt_memzero(iv, bs);
+	crypt_memzero(iv_old, bs);
 	return r;
 }
 
@@ -336,8 +336,8 @@ static int TCRYPT_decrypt_hdr_one(struct tcrypt_alg *alg, const char *mode,
 		crypt_cipher_destroy(cipher);
 	}
 
-	memset(backend_key, 0, sizeof(backend_key));
-	memset(iv, 0, TCRYPT_HDR_IV_LEN);
+	crypt_memzero(backend_key, sizeof(backend_key));
+	crypt_memzero(iv, TCRYPT_HDR_IV_LEN);
 	return r;
 }
 
@@ -387,8 +387,8 @@ out:
 		if (cipher[j])
 			crypt_cipher_destroy(cipher[j]);
 
-	memset(iv, 0, bs);
-	memset(iv_old, 0, bs);
+	crypt_memzero(iv, bs);
+	crypt_memzero(iv_old, bs);
 	return r;
 }
 
@@ -434,7 +434,7 @@ static int TCRYPT_decrypt_hdr(struct crypt_device *cd, struct tcrypt_phdr *hdr,
 		r = -EPERM;
 	}
 
-	memset(&hdr2, 0, sizeof(hdr2));
+	crypt_memzero(&hdr2, sizeof(hdr2));
 	return r;
 }
 
@@ -471,8 +471,8 @@ static int TCRYPT_pool_keyfile(struct crypt_device *cd,
 		j %= TCRYPT_KEY_POOL_LEN;
 	}
 
-	memset(&crc, 0, sizeof(crc));
-	memset(data, 0, TCRYPT_KEYFILE_LEN);
+	crypt_memzero(&crc, sizeof(crc));
+	crypt_memzero(data, TCRYPT_KEYFILE_LEN);
 
 	return 0;
 }
@@ -562,9 +562,9 @@ static int TCRYPT_init_hdr(struct crypt_device *cd,
 			params->cipher, params->mode, params->key_size);
 	}
 out:
-	memset(pwd, 0, TCRYPT_KEY_POOL_LEN);
+	crypt_memzero(pwd, TCRYPT_KEY_POOL_LEN);
 	if (key)
-		memset(key, 0, TCRYPT_HDR_KEY_LEN);
+		crypt_memzero(key, TCRYPT_HDR_KEY_LEN);
 	free(key);
 	return r;
 }
diff --git a/lib/utils_crypt.c b/lib/utils_crypt.c
index 7382f0d1..5cfe4777 100644
--- a/lib/utils_crypt.c
+++ b/lib/utils_crypt.c
@@ -81,6 +81,18 @@ int crypt_parse_name_and_mode(const char *s, char *cipher, int *key_nums,
 	return -EINVAL;
 }
 
+/*
+ * Replacement for memset(s, 0, n) on stack that can be optimized out
+ * Also used in safe allocations for explicit memory wipe.
+ */
+void crypt_memzero(void *s, size_t n)
+{
+	volatile uint8_t *p = (volatile uint8_t *)s;
+
+	while(n--)
+		*p++ = 0;
+}
+
 /* safe allocations */
 void *crypt_safe_alloc(size_t size)
 {
@@ -94,7 +106,7 @@ void *crypt_safe_alloc(size_t size)
 		return NULL;
 
 	alloc->size = size;
-	memset(&alloc->data, 0, size);
+	crypt_memzero(&alloc->data, size);
 
 	/* coverity[leaked_storage] */
 	return &alloc->data;
@@ -110,7 +122,7 @@ void crypt_safe_free(void *data)
 	alloc = (struct safe_allocation *)
 		((char *)data - offsetof(struct safe_allocation, data));
 
-	memset(data, 0, alloc->size);
+	crypt_memzero(data, alloc->size);
 
 	alloc->size = 0x55aa55aa;
 	free(alloc);
diff --git a/lib/volumekey.c b/lib/volumekey.c
index 9a80529e..e7150aae 100644
--- a/lib/volumekey.c
+++ b/lib/volumekey.c
@@ -35,7 +35,7 @@ struct volume_key *crypt_alloc_volume_key(unsigned keylength, const char *key)
 	if (key)
 		memcpy(&vk->key, key, keylength);
 	else
-		memset(&vk->key, 0, keylength);
+		crypt_memzero(&vk->key, keylength);
 
 	return vk;
 }
@@ -43,7 +43,7 @@ struct volume_key *crypt_alloc_volume_key(unsigned keylength, const char *key)
 void crypt_free_volume_key(struct volume_key *vk)
 {
 	if (vk) {
-		memset(vk->key, 0, vk->keylength);
+		crypt_memzero(vk->key, vk->keylength);
 		vk->keylength = 0;
 		free(vk);
 	}