Re: [PATCH v3 09/10] crypto: safexcel - Rework cleanup of sensitive structs in safexcel_aead_setkey
From: Thomas Huth
Date: Thu Aug 06 2026 - 04:18:23 EST
On 06/08/2026 10.10, Antoine Tenart wrote:
On Wed, Aug 05, 2026 at 01:57:47PM +0200, Thomas Huth wrote:
From: Thomas Huth <thuth@xxxxxxxxxx>
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.
It has been suggested by Eric here (unless I got him wrong):
https://lore.kernel.org/linux-crypto/20260804185402.GD2904385@xxxxxxxxxx/
... I should have maybe added that link to this patch description ...
(On the other hand mixing gotos and __cleanup is not advised but is thatOops, good catch, thanks! I will fix it in the next version (assuming that removing the memzero_explicit is ok and we'll keep this patch...)
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.
Thomas