Re: [PATCH v3 09/10] crypto: safexcel - Rework cleanup of sensitive structs in safexcel_aead_setkey

[Antoine Tenart <atenart@kernel.org>]

On Wed, Aug 05, 2026 at 01:57:47PM +0200, Thomas Huth wrote:
> From: Thomas Huth <thuth@redhat.com>
> 
> The crypto_authenc_keys structure only contains pointers to keys,
> but not the key data itself. So explicitly clearing the structure
> at the end of safexcel_aead_setkey() is not really necessary.
> 
> On the other hand, the crypto_aes_ctx might contain sensitive information,
> so this structure should be cleaned up at the end instead. Do this
> now via the new __cleanup(aes_zeroize_ctx) marker.

Looking at other crypto drivers it seems zeroing the key pointers was
explicitly added (sometimes later) and my impression is a good chunk of
the users are zeroing it. I don't know whether removing that is fine or
not, my limited understanding is that provides in-depth defense against
leaking were the key reside in memory. Would love to see an explicit
statement from someone with that knowledge.

(On the other hand mixing gotos and __cleanup is not advised but is that
an issue here? Or if zeroing crypto_authenc_keys is actually important
can we use __cleanup too?).

> --- a/drivers/crypto/inside-secure/safexcel_cipher.c
> +++ b/drivers/crypto/inside-secure/safexcel_cipher.c
> @@ -407,17 +407,17 @@ 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;
> +	struct crypto_aes_ctx aes __cleanup(aes_zeroize_ctx);
> +	int err, i;
>  	const char *alg;

> @@ -430,25 +430,25 @@ static int safexcel_aead_setkey(struct crypto_aead *ctfm, const u8 *key,
>  	case SAFEXCEL_DES:
>  		err = verify_aead_des_key(ctfm, keys.enckey, keys.enckeylen);
>  		if (unlikely(err))
> -			goto badkey;
> +			return err;
>  		break;
>  	case SAFEXCEL_3DES:
>  		err = verify_aead_des3_key(ctfm, keys.enckey, keys.enckeylen);
>  		if (unlikely(err))
> -			goto badkey;
> +			return err;
>  		break;
>  	case SAFEXCEL_AES:
>  		err = aes_expandkey(&aes, keys.enckey, keys.enckeylen);
>  		if (unlikely(err))
> -			goto badkey;
> +			return err;
>  		break;
>  	case SAFEXCEL_SM4:
>  		if (unlikely(keys.enckeylen != SM4_KEY_SIZE))
> -			goto badkey;
> +			return err;

'err' is uninitialized here. You can use '-EINVAL' instead.