Re: [PATCH v4 09/10] crypto: safexcel - Rework cleanup of sensitive structs in safexcel_aead_setkey
From: Thomas Huth
Date: Mon Aug 10 2026 - 04:45:03 EST
On 07/08/2026 10.02, Antoine Tenart wrote:
On Fri, Aug 07, 2026 at 09:40:58AM +0200, Antoine Tenart wrote:You're right, using aes_check_keylen sounds like the better idea here. Thanks for the hint! I'll update it in v5...
On Fri, Aug 07, 2026 at 09:06:36AM +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, see Eric's recommendation
here:
https://lore.kernel.org/linux-crypto/20260804185402.GD2904385@xxxxxxxxxx/
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.
Since __cleanup() and gotos should not be mixed in the same function,
replace the gotos with early return statements, which is fine now that
we dropped the memzero_explicit(&keys, sizeof(keys)) at the end.
Suggested-by: Eric Biggers <ebiggers@xxxxxxxxxx>
Signed-off-by: Thomas Huth <thuth@xxxxxxxxxx>
Reviewed-by: Antoine Tenart <atenart@xxxxxxxxxx>
Actually I think we could do better here:
The AES key is expanded in safexcel_aead_setkey using aes_expandkey but
the result (the aes variable) is discarded and never used. This seems a
waste of CPU cycles for doing a validity check on the input key, which
can be replaced by a simple aes_check_keylen.
The same applies to the EIP93 patch for both chunks.
Thomas