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

[Coiby Xu]

On Fri, May 08, 2026 at 09:08:39PM +0800, Coiby Xu wrote:
>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.

You are right that writing to an attribute of an configfs item won't be
protected by the mutux lock. Thanks for raising the concern! 

>
>>
>>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 at 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.

After comparing the kexec lock approach with the configfs mutex lock
approach, I think the latter is a simpler solution because

1. the kexec lock is non-blocking and we have to repeatedly try until the
    lock get acquired. So it means user space has to make changes as
    well.

2. configfs already acquires the mutex lock automatically for
    creating/deleting configfs items. So if we use configfs mutex lock,
    it means one less place to use the lock.

So I prefer the configfs mutex lock approach unless I miss something,

Sorry it took me long time to get back to you. I got a bit sidetracked
by things including exploring ways to prevent these issues using unit
tests, Smatch or even rust. But I think it's better I do them in a
separate patch set.

-- 
Best regards,
Coiby