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

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

On Wed, Sep 09, 2026 at 12:47:21AM +0000, sashiko-bot@kernel.org wrote:
>Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
>New issues:
>- [High] Severe locking imbalance in `crash_load_dm_crypt_keys()` leads to a leaked mutex on the error path and unlocking an unacquired mutex on the success path.

Thanks for catching this issue! I'll address it in new version.

>
>Pre-existing issues:
>- [High] Premature and unconditional reset of `is_dm_key_reused` to false breaks ARM64/LoongArch kexec load retries and defeats intended cleanup logic.
>--
>
>commit a9405cf8948ba39213612c61a1f3b10b23f437e4
>Author: Coiby Xu <coiby.xu@gmail.com>
>
>crash_dump: Disallow writing to dm-crypt configfs during kexec_file_load syscall
>
>This commit prevents concurrent writes to the configfs group during the
>kexec_file_load syscall by utilizing config_keys_subsys.su_mutex. It aims to
>avoid buffer overflows, stale keys, or memory leaks caused by changes to
>dm-crypt keys during iteration.
>
>> diff --git a/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c
>> index 3a416933979f8..580534d43b66e 100644
>> --- a/kernel/crash_dump_dm_crypt.c
>> +++ b/kernel/crash_dump_dm_crypt.c
>
>[ ... ]
>
>> @@ -478,6 +490,9 @@ int crash_load_dm_crypt_keys(struct kimage *image)
>>  		"Loaded dm crypt keys to kexec_buffer bufsz=0x%lx memsz=0x%lx\n",
>>  		kbuf.bufsz, kbuf.memsz);
>>
>> +unlock:
>> +	mutex_unlock(&config_keys_subsys.su_mutex);
>> +
>
>[Severity: High]
>Looking at crash_load_dm_crypt_keys(), if kexec_add_buffer() fails earlier
>in the function, it jumps to the out label, bypassing this unlock label.
>Does this leak the config_keys_subsys.su_mutex if the lock was acquired?
>
>Also, on the success path, mutex_unlock() is executed unconditionally. If
>is_dm_key_reused was true at the top of the function, the mutex was never
>acquired. Can this cause a kernel panic or corruption by unlocking an
>unacquired mutex?
>
>>  out:
>> -	return r;
>> +	is_dm_key_reused = false;
>> +	return r;
>>  }
>
>[Severity: High]
>This is a pre-existing issue, but since the error and success paths
>unconditionally reset is_dm_key_reused to false here, does this break
>architectures like ARM64 that might retry segment loading?
>
>If load_other_segments() retries the load sequence, the second pass
>through crash_load_dm_crypt_keys() will see is_dm_key_reused as false,
>find zero keys, and silently load nothing into the kdump kernel.
>
>Additionally, crash_dm_crypt_cleanup() checks !is_dm_key_reused to determine
>whether it should free keys_header. Since the flag is unconditionally cleared
>here first, does this defeat the intended cleanup protection logic?
>
>-- 
>Sashiko AI review · https://sashiko.dev/#/patchset/20260909003657.1570544-1-coiby.xu@gmail.com?part=3

-- 
Best regards,
Coiby