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

[Thomas Huth <thuth@redhat.com>]

On 07/08/2026 10.02, Antoine Tenart wrote:
> On Fri, Aug 07, 2026 at 09:40:58AM +0200, Antoine Tenart wrote:
>> On Fri, Aug 07, 2026 at 09:06:36AM +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, see Eric's recommendation
>>> here:
>>>
>>>   https://lore.kernel.org/linux-crypto/20260804185402.GD2904385@google.com/
>>>
>>> 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@kernel.org>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>
>> Reviewed-by: Antoine Tenart <atenart@kernel.org>
> 
> 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.
You're right, using aes_check_keylen sounds like the better idea here. 
Thanks for the hint! I'll update it in v5...

  Thomas