Re: [PATCH v2 1/9] crypto: Provide a wrapper for zeroizing crypto_aes_ctx

[Thomas Huth <thuth@redhat.com>]

On 03/08/2026 21.05, Eric Biggers wrote:
> On Mon, Aug 03, 2026 at 11:44:20AM +0200, Thomas Huth wrote:
>> From: Thomas Huth <thuth@redhat.com>
>>
>> Several crypto drivers need to zeroize their local crypto_aes_ctx
>> structures after use to avoid leaking key material on the stack.
>> Currently some call sites do this with their own memzero_explicit()
>> call, which is error-prone since it is easy to miss a return path
>> (what already happened in some drivers). Some other call sites miss
>> to clear crypto_aes_ctx completely.
>>
>> Provide an aes_clear_ctx() helper that can be used with __cleanup()
>> to automatically zeroize the context when it goes out of scope.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   include/crypto/aes.h | 13 +++++++++++++
>>   1 file changed, 13 insertions(+)
>>
>> diff --git a/include/crypto/aes.h b/include/crypto/aes.h
>> index 3279cfa546085..5ca7b1ab50e8c 100644
>> --- a/include/crypto/aes.h
>> +++ b/include/crypto/aes.h
>> @@ -159,6 +159,19 @@ static inline int aes_check_keylen(size_t keylen)
>>   int aes_expandkey(struct crypto_aes_ctx *ctx, const u8 *in_key,
>>   		  unsigned int key_len);
>>   
>> +/**
>> + * aes_clear_ctx - Zeroize a crypto_aes_ctx structure
>> + * @ctx:	The location of the context that should be zeroized
>> + *
>> + * Explicitly fills the crypto_aes_ctx with zeroes. This should be done
>> + * once the context is not required anymore to avoid that its contents
>> + * are leaked on the stack or heap.
>> + */
>> +static inline void aes_clear_ctx(struct crypto_aes_ctx *ctx)
>> +{
>> +	memzero_explicit(ctx, sizeof(*ctx));
>> +}
> 
> Acked-by: Eric Biggers <ebiggers@kernel.org>
> 
> I guess we should start using __cleanup with type-specific zeroization
> functions like this more often.

Yes, and I already got some more patches for other structure in the works 
already, just wanted to get review feedback on this series here first before 
sending them out / continuing that work.

> One gotcha is that __cleanup and 'goto'
> should not be mixed in the same function; see the comment at
> include/linux/cleanup.h line 148.  Patch 8 of this series doesn't follow
> that in safexcel_aead_setkey().
Ah, thanks for the hint, I wasn't aware of that recommendation yet!

As for safexcel_aead_setkey(), I think the change should be fine since the 
__cleanup() is declared at the very top of the function and not somewhere in 
an inner scope, so there is no way that the "gotos" could skip the cleanup here.

To get rid of the "gotos" here, I'd need to introduce another cleanup 
function for crypto_authenc_keys first, so if you insist of not mixing the 
__cleanup(aes_clear_ctx) with the gotos here, I think I'd rather drop that 
hunk from the patch for now, and provide another patch series with a cleanup 
for crypto_authenc_keys later that then adds the __cleanup() to both, struct 
crypto_authenc_keys keys and struct crypto_aes_ctx aes here and removes the 
gotos at the same time.

So WDYT, drop the hunk here for now and send a v3 with that, or keep the 
current v2 of this patch with its (hopefully unproblematic) ugliness?

  Thomas