Re: [PATCH v2 13/13] dm crypt: wipe key material before kdump

On 8/11/26 22:18, Milan Broz wrote:
> On 8/11/26 7:53 PM, Jan Sebastian Götte wrote:
>> Wipe volume key/iv copies kept by dm-crypt with
>> CONFIG_CRASH_WIPE_SECRETS. The backend tfms are already handled
>> separately.
>>
>> Add a list tracking struct crypt_config instances when
>> CONFIG_CRASH_WIPE_SECRETS is set. Structs are tracked here to avoid
>> having to enumerate them through some roundabout way before kdump, when
>> we can't safely take locks anymore.
> 
> Well, dm-crypt has crypt_wipe_key(), which can be called through a 
> device-mapper message.
> It also sets keys to zero in the crypto API.
> 
> Why do we need yet another way to wipe keys here, reimplementing 
> everything twice?
> 
> I can imagine an emergency wrapper callback that will suspend dm-crypt 
> and call existing code.

I originally decided I'd keep these function separate since you can't 
rely on memory allocation/freeing to work during panic. crypt_wipe_key 
currently calls kfree_sensitive, and inside crypto_*_setkey there's also 
kalloc/kfree calls hiding.

I could rework the patch to call into crypt_wipe_key, but I'd have to 
make that avoid memory allocation/freeing. The direct kfree_sensitive 
call can be replaced with a memzero_explicit, but I think I'd have to 
add a dedicated "wipe without allocations" function to the crypto 
backends as an alternative to setkey with a zero key.

>> Use custom wipe handlers even for things like ivs that have existing
>> wipe functions elsewhere because we need to use crash_wipe_memzero
>> instead of memzero_explicit. The crash_wipe helper memzero_explicit's
>> the target buffers and flushes data caches. On ARM64, missing that cache
>> flush could lead to the zeros not being written to DRAM before the kdump
>> code turns off the data caches moments later.
> 
> Please no. It looks to me like you are trying to fix this on the wrong 
> layer.
> This way everyone will need their own memzero...

You're probably right. I'll remove this from the next version and make 
sure the caches are flushed properly during kexec instead.

 > Dunno, but I really do not like dm-crypt becoming completely bloated 
 > with code
> that has nothing to do with the original purpose of this driver.

I feel like "delete key quick" is a pretty normal function for crypto code.