[PATCH v2 0/9] crypto: Provide a function for zeroizing crypto_aes_ctx

[PATCH v2 0/9] 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 some drivers). Some other call sites miss
to clear crypto_aes_ctx completely.

To improve this situation, the first patch introduces an aes_clear_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 two patches change some files to do the zeroization with
the new __cleanup() way instead of calling memzero_explicit() manually.

v2:
- I sent out v1 last week, but I think it never reached the mailing list.
  Either a PEBKAC or a mail server problem... 🤷‍♂️ 
- Rebased onto cryptodev master branch, updated the "qat" patch accordingly

Thomas Huth (9):
  crypto: Provide a wrapper for zeroizing crypto_aes_ctx
  crypto: aspeed - clear the crypto_aes_ctx when done
  crypto: inside-secure/eip93 - 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: safexcel - zeroize crypto_aes_ctx with
    __cleanup(aes_clear_ctx)
  crypto: qat - zeroize crypto_aes_ctx with __cleanup(aes_clear_ctx)

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

-- 
2.55.0

[PATCH v2 1/9] crypto: Provide a wrapper 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_clear_ctx() helper that can be used with __cleanup()
to automatically zeroize the context when it goes out of scope.

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..5ca7b1ab50e8c 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_clear_ctx - Zeroize 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.
+ */
+static inline void aes_clear_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 v2 2/9] 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_clear_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..74a4ce62fcd93 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_clear_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 v2 3/9] crypto: inside-secure/eip93 - clear the crypto_aes_ctx when done

[Thomas Huth]

From: Thomas Huth <thuth@redhat.com>

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

Cc: Christian Marangi <ansuelsmth@gmail.com>
Cc: Antoine Tenart <atenart@kernel.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 drivers/crypto/inside-secure/eip93/eip93-aead.c   | 2 +-
 drivers/crypto/inside-secure/eip93/eip93-cipher.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/inside-secure/eip93/eip93-aead.c b/drivers/crypto/inside-secure/eip93/eip93-aead.c
index 2bbd0af7b0e0e..8bbbf8c0b8c98 100644
--- a/drivers/crypto/inside-secure/eip93/eip93-aead.c
+++ b/drivers/crypto/inside-secure/eip93/eip93-aead.c
@@ -92,7 +92,7 @@ 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 crypto_aes_ctx aes __cleanup(aes_clear_ctx);
 	struct sa_record *sa_record = ctx->sa_record;
 	u32 nonce = 0;
 	int ret;
diff --git a/drivers/crypto/inside-secure/eip93/eip93-cipher.c b/drivers/crypto/inside-secure/eip93/eip93-cipher.c
index 4dd7ab7503e85..3f9a15aa93541 100644
--- a/drivers/crypto/inside-secure/eip93/eip93-cipher.c
+++ b/drivers/crypto/inside-secure/eip93/eip93-cipher.c
@@ -116,7 +116,7 @@ static int eip93_skcipher_setkey(struct crypto_skcipher *ctfm, const u8 *key,
 	}
 
 	if (flags & EIP93_ALG_AES) {
-		struct crypto_aes_ctx aes;
+		struct crypto_aes_ctx aes __cleanup(aes_clear_ctx);
 
 		ctx->blksize = AES_BLOCK_SIZE;
 		ret = aes_expandkey(&aes, key, keylen);
-- 
2.55.0

[PATCH v2 4/9] 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_clear_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/padlock-aes.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/padlock-aes.c b/drivers/crypto/padlock-aes.c
index 1be549a07a219..ac0aace62b299 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_clear_ctx);
 	int cpu;
 
 	if (key_len % 8)
-- 
2.55.0

[PATCH v2 5/9] 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_clear_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..b921993d86a7b 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_clear_ctx);
 	int key_pos;
 
 	if (aes_expandkey(&ctx, key, key_sz)) {
-- 
2.55.0

[PATCH v2 6/9] 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_clear_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..2bb21f7b06a8e 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_clear_ctx);
 	int err;
 
 	err = aes_expandkey(&rk, in_key, key_len);
-- 
2.55.0

[PATCH v2 7/9] 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_clear_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..4c0f1bbd03fee 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_clear_ctx);
 	int err;
 
 	err = xts_verify_key(tfm, in_key, key_len);
-- 
2.55.0

[PATCH v2 8/9] crypto: safexcel - zeroize crypto_aes_ctx with __cleanup(aes_clear_ctx)

[Thomas Huth]

From: Thomas Huth <thuth@redhat.com>

The code clears the crypto_aes_ctx in most cases already with
memzero_explicit(), but two cases are missed:

1) safexcel_aead_setkey() never cleared it
2) 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 these and to avoid future similar problems, let's use the new
__cleanup(aes_clear_ctx) mechanism to make sure that we clear the
crypto_aes_ctx everywhere.

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

diff --git a/drivers/crypto/inside-secure/safexcel_cipher.c b/drivers/crypto/inside-secure/safexcel_cipher.c
index a8349b684693e..2d21ef388beaf 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_clear_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;
 }
 
@@ -407,7 +406,7 @@ 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;
+	struct crypto_aes_ctx aes __cleanup(aes_clear_ctx);
 	int err = -EINVAL, i;
 	const char *alg;
 
@@ -1362,7 +1361,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_clear_ctx);
 	int ret, i;
 	unsigned int keylen;
 
@@ -1388,7 +1387,6 @@ static int safexcel_skcipher_aesctr_setkey(struct crypto_skcipher *ctfm,
 
 	ctx->key_len = keylen;
 
-	memzero_explicit(&aes, sizeof(aes));
 	return 0;
 }
 
@@ -2542,7 +2540,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_clear_ctx);
 	int ret, i;
 	unsigned int keylen;
 
@@ -2590,7 +2588,6 @@ static int safexcel_skcipher_aesxts_setkey(struct crypto_skcipher *ctfm,
 
 	ctx->key_len = keylen << 1;
 
-	memzero_explicit(&aes, sizeof(aes));
 	return 0;
 }
 
@@ -2756,12 +2753,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_clear_ctx);
 	int ret, i;
 
 	ret = aes_expandkey(&aes, key, len);
 	if (ret) {
-		memzero_explicit(&aes, sizeof(aes));
 		return ret;
 	}
 
@@ -2790,7 +2786,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..d3b701c96d871 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_clear_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 v2 9/9] crypto: qat - zeroize crypto_aes_ctx with __cleanup(aes_clear_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_clear_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.

Cc: Giovanni Cabiddu <giovanni.cabiddu@intel.com>
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..9a56fcdedd579 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_clear_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

Re: [PATCH v2 1/9] crypto: Provide a wrapper for zeroizing crypto_aes_ctx

[Eric Biggers]

On Mon, Aug 03, 2026 at 11:44:20AM +0200, Thomas Huth wrote:
> 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_clear_ctx() helper that can be used with __cleanup()
> to automatically zeroize the context when it goes out of scope.
> 
> 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..5ca7b1ab50e8c 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_clear_ctx - Zeroize 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.
> + */
> +static inline void aes_clear_ctx(struct crypto_aes_ctx *ctx)
> +{
> +	memzero_explicit(ctx, sizeof(*ctx));
> +}

Acked-by: Eric Biggers <ebiggers@kernel.org>

I guess we should start using __cleanup with type-specific zeroization
functions like this more often.  One gotcha is that __cleanup and 'goto'
should not be mixed in the same function; see the comment at
include/linux/cleanup.h line 148.  Patch 8 of this series doesn't follow
that in safexcel_aead_setkey().

- Eric

Re: [PATCH v2 1/9] crypto: Provide a wrapper for zeroizing crypto_aes_ctx

[Simon Richter]

Hi,

On 8/4/26 4:05 AM, Eric Biggers wrote:

> I guess we should start using __cleanup with type-specific zeroization
> functions like this more often.
Frame challenge: should key material be copied that often that we need a 
mechanism to keep track of it?

My feeling is that this wasn't a conscious decision, but is the result 
of two other decisions (that individually make sense): contexts need to 
be self-contained (so need to include key material), and context 
creation should be cheap (so stack contexts are allowed).

So I can see two other approaches:

1. add a parameter to library functions that instructs them to clear the 
key material from the context. Most callers with the context on the 
stack could probably use that mechanism, and SIMD based implementations 
might even be able to do this rather early.

2. allow contexts to refer to key material stored elsewhere to avoid the 
copy. That opens the lifetime tracking can of worms, but the on-stack 
crypto context is rather short-lived.

    Simon

Re: [PATCH v2 1/9] crypto: Provide a wrapper for zeroizing crypto_aes_ctx

[Eric Biggers]

On Tue, Aug 04, 2026 at 11:21:30AM +0900, Simon Richter wrote:
> Hi,
> 
> On 8/4/26 4:05 AM, Eric Biggers wrote:
> 
> > I guess we should start using __cleanup with type-specific zeroization
> > functions like this more often.
> Frame challenge: should key material be copied that often that we need a
> mechanism to keep track of it?

This seems to be yet another case where you are responding to some
thread and trying to start a mostly unrelated discussion.

There are many cases where data on the stack can be sensitive, and this
has always been the case.  If __cleanup helps to manage such data, it's
probably worth using more often.

> My feeling is that this wasn't a conscious decision, but is the result of
> two other decisions (that individually make sense): contexts need to be
> self-contained (so need to include key material), and context creation
> should be cheap (so stack contexts are allowed).

The entire point of "crypto_aes_ctx" is that it is an expanded AES key.
The callers of it are using it to compute the round keys.

If you're actually referring to the contexts for per-message incremental
calculation (e.g. sha256_ctx) offered by the library API, those are a
bit different.  Sometimes they contain key material, sometimes they
don't.  Even if the algorithm is unkeyed, it may still contain key
material, since the user could be hashing a key.  Either way, the
finalization function for each algorithm zeroizes it.  The caller needs
to zeroize only if it abandons a context without zeroizing it.

> 2. allow contexts to refer to key material stored elsewhere to avoid the
> copy. That opens the lifetime tracking can of worms, but the on-stack crypto
> context is rather short-lived.

That is already what is being done in most cases.  The exception is the
HMAC library code since the 'ostate' is not very large.  But again the
per-message contexts are supposed to be zeroized anyway.  Also, if the
one-shot functions are used, then no context is exposed to the caller...

- Eric

Re: [PATCH v2 1/9] crypto: Provide a wrapper for zeroizing crypto_aes_ctx

[Thomas Huth]

On 03/08/2026 21.05, Eric Biggers wrote:
> On Mon, Aug 03, 2026 at 11:44:20AM +0200, Thomas Huth wrote:
>> 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_clear_ctx() helper that can be used with __cleanup()
>> to automatically zeroize the context when it goes out of scope.
>>
>> 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..5ca7b1ab50e8c 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_clear_ctx - Zeroize 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.
>> + */
>> +static inline void aes_clear_ctx(struct crypto_aes_ctx *ctx)
>> +{
>> +	memzero_explicit(ctx, sizeof(*ctx));
>> +}
> 
> Acked-by: Eric Biggers <ebiggers@kernel.org>
> 
> I guess we should start using __cleanup with type-specific zeroization
> functions like this more often.

Yes, and I already got some more patches for other structure in the works 
already, just wanted to get review feedback on this series here first before 
sending them out / continuing that work.

> One gotcha is that __cleanup and 'goto'
> should not be mixed in the same function; see the comment at
> include/linux/cleanup.h line 148.  Patch 8 of this series doesn't follow
> that in safexcel_aead_setkey().
Ah, thanks for the hint, I wasn't aware of that recommendation yet!

As for safexcel_aead_setkey(), I think the change should be fine since the 
__cleanup() is declared at the very top of the function and not somewhere in 
an inner scope, so there is no way that the "gotos" could skip the cleanup here.

To get rid of the "gotos" here, I'd need to introduce another cleanup 
function for crypto_authenc_keys first, so if you insist of not mixing the 
__cleanup(aes_clear_ctx) with the gotos here, I think I'd rather drop that 
hunk from the patch for now, and provide another patch series with a cleanup 
for crypto_authenc_keys later that then adds the __cleanup() to both, struct 
crypto_authenc_keys keys and struct crypto_aes_ctx aes here and removes the 
gotos at the same time.

So WDYT, drop the hunk here for now and send a v3 with that, or keep the 
current v2 of this patch with its (hopefully unproblematic) ugliness?

  Thomas

Re: [PATCH v2 1/9] crypto: Provide a wrapper for zeroizing crypto_aes_ctx

[Eric Biggers]

On Tue, Aug 04, 2026 at 09:42:34AM +0200, Thomas Huth wrote:
> On 03/08/2026 21.05, Eric Biggers wrote:
> > On Mon, Aug 03, 2026 at 11:44:20AM +0200, Thomas Huth wrote:
> > > 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_clear_ctx() helper that can be used with __cleanup()
> > > to automatically zeroize the context when it goes out of scope.
> > > 
> > > 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..5ca7b1ab50e8c 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_clear_ctx - Zeroize 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.
> > > + */
> > > +static inline void aes_clear_ctx(struct crypto_aes_ctx *ctx)
> > > +{
> > > +	memzero_explicit(ctx, sizeof(*ctx));
> > > +}
> > 
> > Acked-by: Eric Biggers <ebiggers@kernel.org>
> > 
> > I guess we should start using __cleanup with type-specific zeroization
> > functions like this more often.
> 
> Yes, and I already got some more patches for other structure in the works
> already, just wanted to get review feedback on this series here first before
> sending them out / continuing that work.

By the way, in the function names could you consider using the verb
"zeroize" instead of "clear"?  So "aes_zeroize_ctx()".  We already have
sha3_zeroize_ctx(), shake_zeroize_ctx(), and chacha_zeroize_state().
And try 'git grep -i zeroize'.  It is the usual word used to mean
zeroization for crypto purposes specifically.

> > One gotcha is that __cleanup and 'goto'
> > should not be mixed in the same function; see the comment at
> > include/linux/cleanup.h line 148.  Patch 8 of this series doesn't follow
> > that in safexcel_aead_setkey().
> Ah, thanks for the hint, I wasn't aware of that recommendation yet!
> 
> As for safexcel_aead_setkey(), I think the change should be fine since the
> __cleanup() is declared at the very top of the function and not somewhere in
> an inner scope, so there is no way that the "gotos" could skip the cleanup
> here.
> 
> To get rid of the "gotos" here, I'd need to introduce another cleanup
> function for crypto_authenc_keys first, so if you insist of not mixing the
> __cleanup(aes_clear_ctx) with the gotos here, I think I'd rather drop that
> hunk from the patch for now, and provide another patch series with a cleanup
> for crypto_authenc_keys later that then adds the __cleanup() to both, struct
> crypto_authenc_keys keys and struct crypto_aes_ctx aes here and removes the
> gotos at the same time.
> 
> So WDYT, drop the hunk here for now and send a v3 with that, or keep the
> current v2 of this patch with its (hopefully unproblematic) ugliness?

Dropping that hunk sounds good.  I recommend avoiding the mixed
__cleanup and gotos even when they are done correctly, as it's easy to
get wrong and it raises questions.

Zeroziation of struct crypto_authenc_keys should just be removed, as
it's just a helper struct for parsing the key buffer.  It contains only
pointers into the key buffer, not the keys themselves.

- Eric