Re: [PATCH 00/11] libcrypto: Provide more __cleanup functions for zeroizing data

[Thomas Huth <thuth@redhat.com>]

On 01/09/2026 18.53, Eric Biggers wrote:
> On Mon, Aug 31, 2026 at 05:12:45PM -0700, Eric Biggers wrote:
>> Hi Thomas,
>>
>> On Thu, Aug 13, 2026 at 03:49:38PM +0200, Thomas Huth wrote:
>>> Code that uses crypto-related structures (containing keys or context data)
>>> should zeroize their local structures on the stack after use to avoid
>>> leaking this sensitive material via the stack when the function returns.
>>> Using the __cleanup() marker is a very elegant way to assert that the
>>> data is zeroized without having to painfully verify that each early return
>>> in a function might miss it.
>>>
>>> Thus this series introduces zeroization functions for many crypto-related
>>> structures that can be used with __cleanup(). The series focuses on the
>>> introduction of the functions - most call sights will be adjusted to use
>>> these new functions in separate patch series later (since each subsystem
>>> needs separate review from the corresponding maintainer). However, I
>>> already included the two "safexcel" patches, since they already got ack'ed
>>> by the maintainer Antoine, so I think they should be fine to go via the
>>> libcrypto tree.
>>>
>>> Note there is one minor ugliness in patch 10: Since sha2.h is also used
>>> in the x86 purgatory code, and that code ships with its own implementation
>>> of string functions, we have to compile the purgatory.c file with
>>> -D__NO_FORTIFY now to be able to include <linux/string.h> in sha2.h.
>>> I hope that solution is OK (especially since the sha256.c file in the
>>> same folder gets that treatment already, too), if not - I'm certainly
>>> open for other suggestions here!
>>>
>>> Thomas Huth (11):
>>>    lib/crypto: aes: Provide a wrapper function for zeroizing
>>>      crypto_aes_ctx
>>>    crypto: safexcel - Simplify the check for a valid AES key
>>>    crypto: safexcel - zeroize crypto_aes_ctx with
>>>      __cleanup(aes_zeroize_ctx)
>>>    lib/crypto: aes: Provide functions for zeroizing aes_key and
>>>      aes_enckey
>>>    lib/crypto: aes: Use aes_zeroize_*key() instead of memzero_explicit()
>>>    lib/crypto: md5: Provide a function for zeroizing hmac_md5_ctx
>>>      structures
>>>    lib/crypto: md5: Use hmac_md5_zeroize_ctx() instead of
>>>      memzero_explicit()
>>>    lib/crypto: sha1: Provide a wrapper for zeroizing hmac_sha1_ctx
>>>    lib/crypto: sha1: Use hmac_sha1_zeroize_ctx() instead of
>>>      memzero_explicit()
>>>    x86/purgatory: Compile purgatory.c with -D__NO_FORTIFY
>>>    lib/crypto: sha2: Provide wrappers for zeroizing SHA2 hmac_sha*_ctx
>>>      structures
>>
>> I'm taking a look at these again for 7.3.  This is still missing quite a
>> few of the crypto library structs.  I would prefer to handle them all in
>> a consistent way.
>>
>> Also, given that we'll end up with a lot of these, we should be
>> thoughtful about the kerneldoc.  Using aes_enckey as an example:
>>
>> /**
>>   * aes_zeroize_enckey() - Zeroize an aes_enckey structure
>>   * @key: The location of the key structure that should be zeroized
>>   *
>>   * Explicitly fills the aes_enckey with zeroes. For example, use it with
>>   * __cleanup() for local aes_enckey structures on the stack, so that their
>>   * content is not leaked when the context is left.
>>   */
>>
>> It's not clear what is meant by "context".  And it doesn't explicitly
>> connect the zeroization requirement to the lifetime of the struct, so I
>> don't think it makes it clear when this should be called.  It also says
>> nothing about kfree_sensitive() which many users should use instead.
>>
>> Meanwhile, aes_prepareenckey() already has the following, which at least
>> connects the zeroization requirement to the lifetime of the key:
>>
>>     * The caller is responsible for zeroizing both the struct aes_enckey and the
>>     * raw key once they are no longer needed.
>>
>> How about we remove that and the free-form description of
>> aes_zeroize_enckey(), and instead expand the comment on the struct
>> itself:
>>
>> /**
>>   * struct aes_enckey - An AES key prepared for encryption
>>   * ...
>>   * Once prepared, users must zeroize this struct at the end of its lifetime.
>>   * For stack-allocated structs, use __cleanup(aes_zeroize_enckey) to zeroize
>>   * when the variable goes out of scope.  For slab-allocated structs, use
>>   * kfree_sensitive() or else call aes_zeroize_enckey() before freeing.
>>   */
>>
>> That would connect the zeroization requirement to the actual struct and
>> its lifetime, and make it clear how to handle each case.  It would also
>> make it clear that stack allocation is supported for this struct, which
>> might not have been obvious before.
>>
>> aes_zeroize_enckey() would then omit a free-form description, which
>> makes sense as it's a trivial wrapper around memzero_explicit():
>>
>> /**
>>   * aes_zeroize_enckey() - Zeroize an aes_enckey structure
>>   * @key: The aes_enckey to zeroize
>>   */
>> static inline void aes_zeroize_enckey(struct aes_enckey *key)
>> {
>> 	memzero_explicit(key, sizeof(*key));
>> }
>>
>> Similarly for all the other structs, of course.
>>
>> (For the hash contexts, the struct comment should mention that the
>> finalization function zeroizes as well.)
>>
>> Does that sound good?
> 
> There's also the subtlety that __cleanup shouldn't be used in functions
> that are already using goto-based cleanup.
> 
> And any kernel code preparing a key also has the raw key too and has to
> zeroize that too, otherwise doing so for the prepared key is pointless.
> 
> ... unless it's not actually a secret key, which sometimes it isn't.
> We've been getting a lot of zeroization patches that try to add
> zeroization for public keys, which is nonsense.  And also for pointers
> to keys, which people confuse with the keys themselves.
> 
> There are also Rust bindings being proposed, and they handle zeroization
> by calling memzero_explicit() directly.  So that's yet another case
> where these new functions are not applicable.
> 
> I'm increasingly thinking that we shouldn't try to explain all the
> zeroization stuff in the kerneldoc, and instead add a documentation file
> in Documentation/crypto/ that properly explains the conventions for
> crypto key zeroization in the kernel and defer to that.
> 
> The struct-specific zeroization functions are still worthwhile, but I
> think they should be seen as trivial wrappers around memzero_explicit().
> So maybe just add all of them with minimal comments like I suggested
> above (maybe even just add them for all the algorithms in a single
> patch), and add a file Documentation/crypto/zeroization.rst separately.
Ok, makes sense, I can have a try to come up with a patch series for this 
within the next days.

However, I'm still stuck on the question how to handle the #include 
<linux/string.h> for the purgatory:

  https://lore.kernel.org/lkml/7d373eca-c23c-4f79-9391-29d75bd5a53f@redhat.com/

Do you maybe have any good suggestions how to tackle this?

  Thomas