Re: [PATCH v2 3/9] crash_dump: Disallow writing to dm-crypt configfs during kexec_file_load syscall

[Coiby Xu <coiby.xu@gmail.com>]

On Wed, May 06, 2026 at 07:26:16PM +0530, Sourabh Jain wrote:
>
>
>On 02/05/26 05:13, Coiby Xu wrote:
>>If writing to the configfs group happens concurrently during
>>kexec_file_load syscall, it may lead to the following issues,
>>   - buffer overflow if dm-crypt keys are added after allocation
>>   - stale total_keys if dm-crypt keys are removed during iteration
>>   - keys_header will not be freed if config/crash_dm_crypt_key/reuse is
>>     set true
>>
>>So hold config_keys_subsys.su_mutex for the entire sequence during the
>>kexec_file_load syscall to ensure a consistent snapshot.
>
>
>Yes, this will solve many synchronization problems when a  user tries to
>perform any operation under our configfs_subsystem while the kernel is
>executing the kexec_file_load system call.
>
>Now, regarding the third point about freeing key_header: this will work
>only if configfs takes the su_mutex lock before invoking the store callback.
>I am not sure whether it actually does.

I can confirm configfs will automatically acquire the su_mutex lock when
creating configfs item. But I forgot to try if writing to an item will
also acquire the mutux lock. I'll do an experiment and share the result
later.

>
>However, based on my previous comment on (2/9), if we keep 
>config_keys_reuse_store()
>under the kexec lock, this will be taken care of. This is because the entire
>kexec_file_load system call already runs under the kexec lock.
>
>
>>
>>Fixes: 479e58549b0f ("crash_dump: store dm crypt keys in kdump reserved memory")
>>Signed-off-by: Coiby Xu <coiby.xu@gmail.com>
>>---
>>  kernel/crash_dump_dm_crypt.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>>diff --git a/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c
>>index 4d8a3331bbe7..6377ee86ec50 100644
>>--- a/kernel/crash_dump_dm_crypt.c
>>+++ b/kernel/crash_dump_dm_crypt.c
>>@@ -429,6 +429,7 @@ int crash_load_dm_crypt_keys(struct kimage *image)
>>  	};
>>  	int r = 0;
>>+	mutex_lock(&config_keys_subsys.su_mutex);
>>  	if (key_count <= 0) {
>>  		kexec_dprintk("No dm-crypt keys\n");
>>@@ -479,6 +480,9 @@ void kexec_file_post_load_cleanup_dm_crypt(struct kimage *image)
>>  		kfree_sensitive(keys_header);
>>  		keys_header = NULL;
>>  	}
>>+
>>+	if (mutex_is_locked(&config_keys_subsys.su_mutex))
>>+		mutex_unlock(&config_keys_subsys.su_mutex);
>How about release the lock in crash_load_dm_crypt_keys() itself? Given 
>that config_keys_reuse_store() is placed under kexec lock?

Thanks for proposing another solution! I'll give a try and make a
comparison.

-- 
Best regards,
Coiby