[PATCH v5 00/10] crypto: Provide a function for zeroizing crypto_aes_ctx

[PATCH v5 00/10] crypto: Provide a function for zeroizing crypto_aes_ctx

[Thomas Huth]

Several crypto drivers need to zeroize their local crypto_aes_ctx
structures after use to avoid leaking key material on the stack.
Currently some call sites do this with their own memzero_explicit()
call, which is error-prone since it is easy to miss a return path
(what already happened in a driver). Some other call sites miss
to clear crypto_aes_ctx completely.

To improve this situation, the first patch introduces an aes_zeroize_ctx()
helper that can be used with __cleanup() to automatically zeroize the
context when it goes out of scope. The following 6 patches add this
__cleanup() to spots in the code where this has been forgotten so far.
The final patches change some files to do the zeroization with
the new __cleanup() way instead of calling memzero_explicit() manually.

v5:
- Use aes_check_keylen() in the eip93 patch and in the 1st safexcel patch

v4:
- Updated the function description in the first patch
- Fixed "return err" bug in the "safexcel - Rework cleanup..." patch

v3:
- Renamed aes_clear_ctx() to aes_zeroize_ctx()
- Split up the safeexcel patch to rework safexcel_aead_setkey in a
  separate patch
- Removed goto in the padlock patch

v2:
- Rebased onto cryptodev master branch, updated the "qat" patch accordingly

Thomas Huth (10):
  crypto: Provide a wrapper function for zeroizing crypto_aes_ctx
  crypto: aspeed - clear the crypto_aes_ctx when done
  crypto: padlock-aes - clear the crypto_aes_ctx when done
  crypto: sa2ul - clear the crypto_aes_ctx when done
  crypto: arm/aes-neonbs - clear the crypto_aes_ctx when done
  crypto: arm64/aes-neonbs - clear the crypto_aes_ctx when done
  crypto: qat - zeroize crypto_aes_ctx with __cleanup(aes_zeroize_ctx)
  crypto: safexcel - Simplify the check for a valid AES key
  crypto: safexcel - zeroize crypto_aes_ctx with
    __cleanup(aes_zeroize_ctx)
  crypto: eip93 - Simplify the check for a valid AES key

 arch/arm/crypto/aes-neonbs-glue.c             |  2 +-
 arch/arm64/crypto/aes-neonbs-glue.c           |  2 +-
 drivers/crypto/aspeed/aspeed-hace-crypto.c    |  3 +--
 .../crypto/inside-secure/eip93/eip93-aead.c   |  3 +--
 .../crypto/inside-secure/safexcel_cipher.c    | 16 +++++---------
 drivers/crypto/inside-secure/safexcel_hash.c  |  3 +--
 .../crypto/intel/qat/qat_common/qat_algs.c    |  3 +--
 drivers/crypto/padlock-aes.c                  | 22 +++++++++----------
 drivers/crypto/sa2ul.c                        |  2 +-
 include/crypto/aes.h                          | 13 +++++++++++
 10 files changed, 35 insertions(+), 34 deletions(-)

-- 
2.55.0

[PATCH v5 01/10] crypto: Provide a wrapper function for zeroizing crypto_aes_ctx

[Thomas Huth]

From: Thomas Huth <thuth@redhat.com>

Several crypto drivers need to zeroize their local crypto_aes_ctx
structures after use to avoid leaking key material on the stack.
Currently some call sites do this with their own memzero_explicit()
call, which is error-prone since it is easy to miss a return path
(what already happened in some drivers). Some other call sites miss
to clear crypto_aes_ctx completely.

Provide an aes_zeroize_ctx() helper that can be used with __cleanup()
to automatically zeroize the context when it goes out of scope.

Acked-by: Eric Biggers <ebiggers@kernel.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 include/crypto/aes.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/include/crypto/aes.h b/include/crypto/aes.h
index 3279cfa546085..e70607249ae5c 100644
--- a/include/crypto/aes.h
+++ b/include/crypto/aes.h
@@ -159,6 +159,19 @@ static inline int aes_check_keylen(size_t keylen)
 int aes_expandkey(struct crypto_aes_ctx *ctx, const u8 *in_key,
 		  unsigned int key_len);
 
+/**
+ * aes_zeroize_ctx - Clear a crypto_aes_ctx structure
+ * @ctx:	The location of the context that should be zeroized
+ *
+ * Explicitly fills the crypto_aes_ctx with zeroes. This should be done
+ * once the context is not required anymore to avoid that its contents
+ * are leaked on the stack or heap (if not using kfree_sensitive()).
+ */
+static inline void aes_zeroize_ctx(struct crypto_aes_ctx *ctx)
+{
+	memzero_explicit(ctx, sizeof(*ctx));
+}
+
 /*
  * The following functions are temporarily exported for use by the AES mode
  * implementations in arch/$(SRCARCH)/crypto/.  These exports will go away when
-- 
2.55.0

[PATCH v5 02/10] crypto: aspeed - clear the crypto_aes_ctx when done

[Thomas Huth]

From: Thomas Huth <thuth@redhat.com>

Declare the gen_aes_key with __cleanup(aes_zeroize_ctx) to avoid that
its contents could be leaking via the stack when the function returns.
And since it is only required in one branch of the if-statement there,
move it to that block, too.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 drivers/crypto/aspeed/aspeed-hace-crypto.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/crypto/aspeed/aspeed-hace-crypto.c b/drivers/crypto/aspeed/aspeed-hace-crypto.c
index fa201dae1f81b..e7d3f611df05c 100644
--- a/drivers/crypto/aspeed/aspeed-hace-crypto.c
+++ b/drivers/crypto/aspeed/aspeed-hace-crypto.c
@@ -576,7 +576,6 @@ static int aspeed_aes_setkey(struct crypto_skcipher *cipher, const u8 *key,
 {
 	struct aspeed_cipher_ctx *ctx = crypto_skcipher_ctx(cipher);
 	struct aspeed_hace_dev *hace_dev = ctx->hace_dev;
-	struct crypto_aes_ctx gen_aes_key;
 
 	CIPHER_DBG(hace_dev, "keylen: %d bits\n", (keylen * 8));
 
@@ -585,9 +584,9 @@ static int aspeed_aes_setkey(struct crypto_skcipher *cipher, const u8 *key,
 		return -EINVAL;
 
 	if (ctx->hace_dev->version == AST2500_VERSION) {
+		struct crypto_aes_ctx gen_aes_key __cleanup(aes_zeroize_ctx);
 		aes_expandkey(&gen_aes_key, key, keylen);
 		memcpy(ctx->key, gen_aes_key.key_enc, AES_MAX_KEYLENGTH);
-
 	} else {
 		memcpy(ctx->key, key, keylen);
 	}
-- 
2.55.0

[PATCH v5 03/10] crypto: padlock-aes - clear the crypto_aes_ctx when done

[Thomas Huth]

From: Thomas Huth <thuth@redhat.com>

Clear the crypto_aes_ctx structure via __cleanup(aes_zeroize_ctx)
when we're done with it to avoid that key data could leak on the
stack.

And since __cleanup() (and __free()) should not be mixed with
"gotos" in the same function, turn the goto statement here
into a proper block of the related if-statement.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 drivers/crypto/padlock-aes.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/crypto/padlock-aes.c b/drivers/crypto/padlock-aes.c
index 1be549a07a219..400a889e924d7 100644
--- a/drivers/crypto/padlock-aes.c
+++ b/drivers/crypto/padlock-aes.c
@@ -109,7 +109,7 @@ static int aes_set_key(struct crypto_tfm *tfm, const u8 *in_key,
 {
 	struct aes_ctx *ctx = aes_ctx(tfm);
 	const __le32 *key = (const __le32 *)in_key;
-	struct crypto_aes_ctx gen_aes;
+	struct crypto_aes_ctx gen_aes __cleanup(aes_zeroize_ctx);
 	int cpu;
 
 	if (key_len % 8)
@@ -137,20 +137,18 @@ static int aes_set_key(struct crypto_tfm *tfm, const u8 *in_key,
 	ctx->cword.decrypt.ksize = ctx->cword.encrypt.ksize;
 
 	/* Don't generate extended keys if the hardware can do it. */
-	if (aes_hw_extkey_available(key_len))
-		goto ok;
+	if (!aes_hw_extkey_available(key_len)) {
+		ctx->D = ctx->d_data;
+		ctx->cword.encrypt.keygen = 1;
+		ctx->cword.decrypt.keygen = 1;
 
-	ctx->D = ctx->d_data;
-	ctx->cword.encrypt.keygen = 1;
-	ctx->cword.decrypt.keygen = 1;
+		if (aes_expandkey(&gen_aes, in_key, key_len))
+			return -EINVAL;
 
-	if (aes_expandkey(&gen_aes, in_key, key_len))
-		return -EINVAL;
-
-	memcpy(ctx->E, gen_aes.key_enc, AES_MAX_KEYLENGTH);
-	memcpy(ctx->D, gen_aes.key_dec, AES_MAX_KEYLENGTH);
+		memcpy(ctx->E, gen_aes.key_enc, AES_MAX_KEYLENGTH);
+		memcpy(ctx->D, gen_aes.key_dec, AES_MAX_KEYLENGTH);
+	}
 
-ok:
 	for_each_online_cpu(cpu)
 		if (&ctx->cword.encrypt == per_cpu(paes_last_cword, cpu) ||
 		    &ctx->cword.decrypt == per_cpu(paes_last_cword, cpu))
-- 
2.55.0

[PATCH v5 04/10] crypto: sa2ul - clear the crypto_aes_ctx when done

[Thomas Huth]

From: Thomas Huth <thuth@redhat.com>

Clear the crypto_aes_ctx structure via __cleanup(aes_zeroize_ctx) when
we're done with it to avoid that key data could leak on the stack.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 drivers/crypto/sa2ul.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/sa2ul.c b/drivers/crypto/sa2ul.c
index d865fd4a098cb..f1a7c2cc7a1af 100644
--- a/drivers/crypto/sa2ul.c
+++ b/drivers/crypto/sa2ul.c
@@ -465,7 +465,7 @@ static void sa_prepare_iopads(struct algo_data *data, const u8 *key,
 /* Derive the inverse key used in AES-CBC decryption operation */
 static inline int sa_aes_inv_key(u8 *inv_key, const u8 *key, u16 key_sz)
 {
-	struct crypto_aes_ctx ctx;
+	struct crypto_aes_ctx ctx __cleanup(aes_zeroize_ctx);
 	int key_pos;
 
 	if (aes_expandkey(&ctx, key, key_sz)) {
-- 
2.55.0

[PATCH v5 05/10] crypto: arm/aes-neonbs - clear the crypto_aes_ctx when done

[Thomas Huth]

From: Thomas Huth <thuth@redhat.com>

Clear the crypto_aes_ctx structure via __cleanup(aes_zeroize_ctx) when
we're done with it to avoid that key data could leak on the stack.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 arch/arm/crypto/aes-neonbs-glue.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/crypto/aes-neonbs-glue.c b/arch/arm/crypto/aes-neonbs-glue.c
index c49ddafc54f34..f0c8bfd2ac8fc 100644
--- a/arch/arm/crypto/aes-neonbs-glue.c
+++ b/arch/arm/crypto/aes-neonbs-glue.c
@@ -60,7 +60,7 @@ static int aesbs_setkey(struct crypto_skcipher *tfm, const u8 *in_key,
 			unsigned int key_len)
 {
 	struct aesbs_ctx *ctx = crypto_skcipher_ctx(tfm);
-	struct crypto_aes_ctx rk;
+	struct crypto_aes_ctx rk __cleanup(aes_zeroize_ctx);
 	int err;
 
 	err = aes_expandkey(&rk, in_key, key_len);
-- 
2.55.0

[PATCH v5 06/10] crypto: arm64/aes-neonbs - clear the crypto_aes_ctx when done

[Thomas Huth]

From: Thomas Huth <thuth@redhat.com>

Clear the crypto_aes_ctx structure via __cleanup(aes_zeroize_ctx) when
we're done with it to avoid that key data could leak on the stack.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 arch/arm64/crypto/aes-neonbs-glue.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/crypto/aes-neonbs-glue.c b/arch/arm64/crypto/aes-neonbs-glue.c
index 5bcbac9798931..c2f3eac0ca661 100644
--- a/arch/arm64/crypto/aes-neonbs-glue.c
+++ b/arch/arm64/crypto/aes-neonbs-glue.c
@@ -247,7 +247,7 @@ static int aesbs_xts_setkey(struct crypto_skcipher *tfm, const u8 *in_key,
 			    unsigned int key_len)
 {
 	struct aesbs_xts_ctx *ctx = crypto_skcipher_ctx(tfm);
-	struct crypto_aes_ctx rk;
+	struct crypto_aes_ctx rk __cleanup(aes_zeroize_ctx);
 	int err;
 
 	err = xts_verify_key(tfm, in_key, key_len);
-- 
2.55.0

[PATCH v5 07/10] crypto: qat - zeroize crypto_aes_ctx with __cleanup(aes_zeroize_ctx)

[Thomas Huth]

From: Thomas Huth <thuth@redhat.com>

A recent patch by Giovanni Cabiddu already added a memzero_explicit()
for the crypto_aes_ctx in the qat_alg_xts_reverse_key() function,
but since we introduced __cleanup(aes_zeroize_ctx) markers in previous
commits in many spots of the code already, let's use it here now, too,
to have the same code pattern everywhere.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 drivers/crypto/intel/qat/qat_common/qat_algs.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/crypto/intel/qat/qat_common/qat_algs.c b/drivers/crypto/intel/qat/qat_common/qat_algs.c
index 91663805d9e60..cb669fb661625 100644
--- a/drivers/crypto/intel/qat/qat_common/qat_algs.c
+++ b/drivers/crypto/intel/qat/qat_common/qat_algs.c
@@ -388,7 +388,7 @@ static void qat_alg_skcipher_init_enc(struct qat_alg_skcipher_ctx *ctx,
 static void qat_alg_xts_reverse_key(const u8 *key_forward, unsigned int keylen,
 				    u8 *key_reverse)
 {
-	struct crypto_aes_ctx aes_expanded;
+	struct crypto_aes_ctx aes_expanded __cleanup(aes_zeroize_ctx);
 	int nrounds;
 	u8 *key;
 
@@ -405,7 +405,6 @@ static void qat_alg_xts_reverse_key(const u8 *key_forward, unsigned int keylen,
 		memcpy(key_reverse + AES_BLOCK_SIZE, key - AES_BLOCK_SIZE,
 		       AES_BLOCK_SIZE);
 	}
-	memzero_explicit(&aes_expanded, sizeof(aes_expanded));
 }
 
 static void qat_alg_skcipher_init_dec(struct qat_alg_skcipher_ctx *ctx,
-- 
2.55.0

[PATCH v5 08/10] crypto: safexcel - Simplify the check for a valid AES key

[Thomas Huth]

From: Thomas Huth <thuth@redhat.com>

safexcel_aead_setkey() currently uses aes_expandkey() to check for a valid
AES key, but then does not use the crypto_aes_ctx afterwards anymore,
i.e. this is just a wasteful way of checking the key length, and thus
aes_check_keylen() should be used instead.
This also fixes a potential leak of sensitive data via the stack, since
this function forgot to zeroize crypto_aes_ctx before returning to the
caller.

Suggested-by: Antoine Tenart <atenart@kernel.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 drivers/crypto/inside-secure/safexcel_cipher.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/crypto/inside-secure/safexcel_cipher.c b/drivers/crypto/inside-secure/safexcel_cipher.c
index a8349b684693e..f07d043c67d45 100644
--- a/drivers/crypto/inside-secure/safexcel_cipher.c
+++ b/drivers/crypto/inside-secure/safexcel_cipher.c
@@ -407,7 +407,6 @@ static int safexcel_aead_setkey(struct crypto_aead *ctfm, const u8 *key,
 	struct safexcel_cipher_ctx *ctx = crypto_tfm_ctx(tfm);
 	struct safexcel_crypto_priv *priv = ctx->base.priv;
 	struct crypto_authenc_keys keys;
-	struct crypto_aes_ctx aes;
 	int err = -EINVAL, i;
 	const char *alg;
 
@@ -438,7 +437,7 @@ static int safexcel_aead_setkey(struct crypto_aead *ctfm, const u8 *key,
 			goto badkey;
 		break;
 	case SAFEXCEL_AES:
-		err = aes_expandkey(&aes, keys.enckey, keys.enckeylen);
+		err = aes_check_keylen(keys.enckeylen);
 		if (unlikely(err))
 			goto badkey;
 		break;
-- 
2.55.0

[PATCH v5 09/10] crypto: safexcel - zeroize crypto_aes_ctx with __cleanup(aes_zeroize_ctx)

[Thomas Huth]

From: Thomas Huth <thuth@redhat.com>

The code clears the crypto_aes_ctx in most cases already with
memzero_explicit(), but safexcel_skcipher_aesxts_setkey() runs
aes_expandkey() twice, and in case the second call fails, the
context from the first call is leaked.

To fix this issue and to avoid future similar problems, let's use
the new __cleanup(aes_zeroize_ctx) mechanism to make sure that we
always clear the crypto_aes_ctx in all cases.

Acked-by: Antoine Tenart <atenart@kernel.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 drivers/crypto/inside-secure/safexcel_cipher.c | 13 ++++---------
 drivers/crypto/inside-secure/safexcel_hash.c   |  3 +--
 2 files changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/crypto/inside-secure/safexcel_cipher.c b/drivers/crypto/inside-secure/safexcel_cipher.c
index f07d043c67d45..b031cb9652ec3 100644
--- a/drivers/crypto/inside-secure/safexcel_cipher.c
+++ b/drivers/crypto/inside-secure/safexcel_cipher.c
@@ -375,7 +375,7 @@ static int safexcel_skcipher_aes_setkey(struct crypto_skcipher *ctfm,
 	struct crypto_tfm *tfm = crypto_skcipher_tfm(ctfm);
 	struct safexcel_cipher_ctx *ctx = crypto_tfm_ctx(tfm);
 	struct safexcel_crypto_priv *priv = ctx->base.priv;
-	struct crypto_aes_ctx aes;
+	struct crypto_aes_ctx aes __cleanup(aes_zeroize_ctx);
 	int ret, i;
 
 	ret = aes_expandkey(&aes, key, len);
@@ -396,7 +396,6 @@ static int safexcel_skcipher_aes_setkey(struct crypto_skcipher *ctfm,
 
 	ctx->key_len = len;
 
-	memzero_explicit(&aes, sizeof(aes));
 	return 0;
 }
 
@@ -1361,7 +1360,7 @@ static int safexcel_skcipher_aesctr_setkey(struct crypto_skcipher *ctfm,
 	struct crypto_tfm *tfm = crypto_skcipher_tfm(ctfm);
 	struct safexcel_cipher_ctx *ctx = crypto_tfm_ctx(tfm);
 	struct safexcel_crypto_priv *priv = ctx->base.priv;
-	struct crypto_aes_ctx aes;
+	struct crypto_aes_ctx aes __cleanup(aes_zeroize_ctx);
 	int ret, i;
 	unsigned int keylen;
 
@@ -1387,7 +1386,6 @@ static int safexcel_skcipher_aesctr_setkey(struct crypto_skcipher *ctfm,
 
 	ctx->key_len = keylen;
 
-	memzero_explicit(&aes, sizeof(aes));
 	return 0;
 }
 
@@ -2541,7 +2539,7 @@ static int safexcel_skcipher_aesxts_setkey(struct crypto_skcipher *ctfm,
 	struct crypto_tfm *tfm = crypto_skcipher_tfm(ctfm);
 	struct safexcel_cipher_ctx *ctx = crypto_tfm_ctx(tfm);
 	struct safexcel_crypto_priv *priv = ctx->base.priv;
-	struct crypto_aes_ctx aes;
+	struct crypto_aes_ctx aes __cleanup(aes_zeroize_ctx);
 	int ret, i;
 	unsigned int keylen;
 
@@ -2589,7 +2587,6 @@ static int safexcel_skcipher_aesxts_setkey(struct crypto_skcipher *ctfm,
 
 	ctx->key_len = keylen << 1;
 
-	memzero_explicit(&aes, sizeof(aes));
 	return 0;
 }
 
@@ -2755,12 +2752,11 @@ static int safexcel_aead_ccm_setkey(struct crypto_aead *ctfm, const u8 *key,
 	struct crypto_tfm *tfm = crypto_aead_tfm(ctfm);
 	struct safexcel_cipher_ctx *ctx = crypto_tfm_ctx(tfm);
 	struct safexcel_crypto_priv *priv = ctx->base.priv;
-	struct crypto_aes_ctx aes;
+	struct crypto_aes_ctx aes __cleanup(aes_zeroize_ctx);
 	int ret, i;
 
 	ret = aes_expandkey(&aes, key, len);
 	if (ret) {
-		memzero_explicit(&aes, sizeof(aes));
 		return ret;
 	}
 
@@ -2789,7 +2785,6 @@ static int safexcel_aead_ccm_setkey(struct crypto_aead *ctfm, const u8 *key,
 	else
 		ctx->hash_alg = CONTEXT_CONTROL_CRYPTO_ALG_XCBC128;
 
-	memzero_explicit(&aes, sizeof(aes));
 	return 0;
 }
 
diff --git a/drivers/crypto/inside-secure/safexcel_hash.c b/drivers/crypto/inside-secure/safexcel_hash.c
index 3402e570d045c..20c17eb09495e 100644
--- a/drivers/crypto/inside-secure/safexcel_hash.c
+++ b/drivers/crypto/inside-secure/safexcel_hash.c
@@ -1905,7 +1905,7 @@ static int safexcel_cbcmac_setkey(struct crypto_ahash *tfm, const u8 *key,
 				 unsigned int len)
 {
 	struct safexcel_ahash_ctx *ctx = crypto_tfm_ctx(crypto_ahash_tfm(tfm));
-	struct crypto_aes_ctx aes;
+	struct crypto_aes_ctx aes __cleanup(aes_zeroize_ctx);
 	int ret, i;
 
 	ret = aes_expandkey(&aes, key, len);
@@ -1928,7 +1928,6 @@ static int safexcel_cbcmac_setkey(struct crypto_ahash *tfm, const u8 *key,
 	}
 	ctx->cbcmac  = true;
 
-	memzero_explicit(&aes, sizeof(aes));
 	return 0;
 }
 
-- 
2.55.0

[PATCH v5 10/10] crypto: eip93 - Simplify the check for a valid AES key

[Thomas Huth]

From: Thomas Huth <thuth@redhat.com>

eip93_aead_setkey() currently uses aes_expandkey() to check for a valid
AES key, but then does not use the crypto_aes_ctx afterwards anymore,
i.e. this is just a wasteful way of checking the key length, and thus
aes_check_keylen() should be used instead.
This also fixes a potential leak of sensitive data via the stack, since
this function forgot to zeroize crypto_aes_ctx before returning to the
caller.

Suggested-by: Antoine Tenart <atenart@kernel.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 drivers/crypto/inside-secure/eip93/eip93-aead.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/crypto/inside-secure/eip93/eip93-aead.c b/drivers/crypto/inside-secure/eip93/eip93-aead.c
index 2bbd0af7b0e0e..0b7a899b14d76 100644
--- a/drivers/crypto/inside-secure/eip93/eip93-aead.c
+++ b/drivers/crypto/inside-secure/eip93/eip93-aead.c
@@ -92,7 +92,6 @@ static int eip93_aead_setkey(struct crypto_aead *ctfm, const u8 *key,
 	struct crypto_tfm *tfm = crypto_aead_tfm(ctfm);
 	struct eip93_crypto_ctx *ctx = crypto_tfm_ctx(tfm);
 	struct crypto_authenc_keys keys;
-	struct crypto_aes_ctx aes;
 	struct sa_record *sa_record = ctx->sa_record;
 	u32 nonce = 0;
 	int ret;
@@ -126,7 +125,7 @@ static int eip93_aead_setkey(struct crypto_aead *ctfm, const u8 *key,
 
 		break;
 	case EIP93_ALG_AES:
-		ret = aes_expandkey(&aes, keys.enckey, keys.enckeylen);
+		ret = aes_check_keylen(keys.enckeylen);
 		if (ret)
 			return ret;
 
-- 
2.55.0

Re: [PATCH v5 08/10] crypto: safexcel - Simplify the check for a valid AES key

[Antoine Tenart]

On Mon, Aug 10, 2026 at 11:30:03AM +0200, Thomas Huth wrote:
> From: Thomas Huth <thuth@redhat.com>
> 
> safexcel_aead_setkey() currently uses aes_expandkey() to check for a valid
> AES key, but then does not use the crypto_aes_ctx afterwards anymore,
> i.e. this is just a wasteful way of checking the key length, and thus
> aes_check_keylen() should be used instead.
> This also fixes a potential leak of sensitive data via the stack, since
> this function forgot to zeroize crypto_aes_ctx before returning to the
> caller.
> 
> Suggested-by: Antoine Tenart <atenart@kernel.org>
> Signed-off-by: Thomas Huth <thuth@redhat.com>

Acked-by: Antoine Tenart <atenart@kernel.org>

Thanks!

> ---
>  drivers/crypto/inside-secure/safexcel_cipher.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/crypto/inside-secure/safexcel_cipher.c b/drivers/crypto/inside-secure/safexcel_cipher.c
> index a8349b684693e..f07d043c67d45 100644
> --- a/drivers/crypto/inside-secure/safexcel_cipher.c
> +++ b/drivers/crypto/inside-secure/safexcel_cipher.c
> @@ -407,7 +407,6 @@ static int safexcel_aead_setkey(struct crypto_aead *ctfm, const u8 *key,
>  	struct safexcel_cipher_ctx *ctx = crypto_tfm_ctx(tfm);
>  	struct safexcel_crypto_priv *priv = ctx->base.priv;
>  	struct crypto_authenc_keys keys;
> -	struct crypto_aes_ctx aes;
>  	int err = -EINVAL, i;
>  	const char *alg;
>  
> @@ -438,7 +437,7 @@ static int safexcel_aead_setkey(struct crypto_aead *ctfm, const u8 *key,
>  			goto badkey;
>  		break;
>  	case SAFEXCEL_AES:
> -		err = aes_expandkey(&aes, keys.enckey, keys.enckeylen);
> +		err = aes_check_keylen(keys.enckeylen);
>  		if (unlikely(err))
>  			goto badkey;
>  		break;
> -- 
> 2.55.0
> 

Re: [PATCH v5 10/10] crypto: eip93 - Simplify the check for a valid AES key

[Antoine Tenart]

On Mon, Aug 10, 2026 at 11:30:05AM +0200, Thomas Huth wrote:
> From: Thomas Huth <thuth@redhat.com>
> 
> eip93_aead_setkey() currently uses aes_expandkey() to check for a valid
> AES key, but then does not use the crypto_aes_ctx afterwards anymore,
> i.e. this is just a wasteful way of checking the key length, and thus
> aes_check_keylen() should be used instead.
> This also fixes a potential leak of sensitive data via the stack, since
> this function forgot to zeroize crypto_aes_ctx before returning to the
> caller.
> 
> Suggested-by: Antoine Tenart <atenart@kernel.org>
> Signed-off-by: Thomas Huth <thuth@redhat.com>

Reviewed-by: Antoine Tenart <atenart@kernel.org>

> ---
>  drivers/crypto/inside-secure/eip93/eip93-aead.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/crypto/inside-secure/eip93/eip93-aead.c b/drivers/crypto/inside-secure/eip93/eip93-aead.c
> index 2bbd0af7b0e0e..0b7a899b14d76 100644
> --- a/drivers/crypto/inside-secure/eip93/eip93-aead.c
> +++ b/drivers/crypto/inside-secure/eip93/eip93-aead.c
> @@ -92,7 +92,6 @@ static int eip93_aead_setkey(struct crypto_aead *ctfm, const u8 *key,
>  	struct crypto_tfm *tfm = crypto_aead_tfm(ctfm);
>  	struct eip93_crypto_ctx *ctx = crypto_tfm_ctx(tfm);
>  	struct crypto_authenc_keys keys;
> -	struct crypto_aes_ctx aes;
>  	struct sa_record *sa_record = ctx->sa_record;
>  	u32 nonce = 0;
>  	int ret;
> @@ -126,7 +125,7 @@ static int eip93_aead_setkey(struct crypto_aead *ctfm, const u8 *key,
>  
>  		break;
>  	case EIP93_ALG_AES:
> -		ret = aes_expandkey(&aes, keys.enckey, keys.enckeylen);
> +		ret = aes_check_keylen(keys.enckeylen);
>  		if (ret)
>  			return ret;
>  
> -- 
> 2.55.0
>