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

Antoine Tenart <[email protected]>
Newsgroups org.kernel.vger.linux-crypto,org.kernel.vger.linux-kernel
Message-ID <anRC5AYai6TVHcac@kwain>
On Wed, Aug 05, 2026 at 01:57:48PM +0200, Thomas Huth wrote:
> From: Thomas Huth <[email protected]>
> 
> 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.
> 
> Signed-off-by: Thomas Huth <[email protected]>

Acked-by: Antoine Tenart <[email protected]>

> ---
>  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 e94686490bb27..50e8792b399de 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;
>  }
>  
> @@ -1357,7 +1356,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;
>  
> @@ -1383,7 +1382,6 @@ static int safexcel_skcipher_aesctr_setkey(struct crypto_skcipher *ctfm,
>  
>  	ctx->key_len = keylen;
>  
> -	memzero_explicit(&aes, sizeof(aes));
>  	return 0;
>  }
>  
> @@ -2537,7 +2535,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;
>  
> @@ -2585,7 +2583,6 @@ static int safexcel_skcipher_aesxts_setkey(struct crypto_skcipher *ctfm,
>  
>  	ctx->key_len = keylen << 1;
>  
> -	memzero_explicit(&aes, sizeof(aes));
>  	return 0;
>  }
>  
> @@ -2751,12 +2748,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;
>  	}
>  
> @@ -2785,7 +2781,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
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.