Re: [PATCH v3 04/10] crash_dump: Read the number of dm-crypt keys from reserved memory

[Sourabh Jain]

Hi Coiby,

On 29/07/26 09:06, Coiby Xu wrote:
> In case user adds/deletes the keys by mistake, it's safer to read the
> number of keys from reserved memory.

I am not sure how we differentiate between a key being deleted 
accidentally and a user
intentionally deleting it. However, I have a question about how the 
kernel handles key
addition and removal.

How does the kernel handle key add/remove operations to keep the kexec 
segment
corresponding to the key header up to date?

The reason I am asking is to understand what happens when a user deletes 
a key. Does the
kexec segment corresponding to that key header still retain information 
about the deleted key?

If it does, could you explain why? If it does not, could you explain how 
the kexec segment gets updated?

Also, for my understanding, could you please point me to what exactly is 
stored in the key header's kexec
segment? During restore, kernel access the old kernel memory using the 
information stored in that kexec
segment, so I would like to better understand what data it contains.

Thanks,
Sourabh Jain



> Fixes: 9ebfa8dcaea7 ("crash_dump: reuse saved dm crypt keys for CPU/memory hot-plugging")
> Reported-and-Suggested-by: Sourabh Jain <sourabhjain@xxxxxxxxxxxxx>
> Signed-off-by: Coiby Xu <coiby.xu@xxxxxxxxx>
> ---
>   kernel/crash_dump_dm_crypt.c | 36 +++++++++++++++++++++++-------------
>   1 file changed, 23 insertions(+), 13 deletions(-)
>
> diff --git a/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c
> index d2e66c6fe6f3..a3996208738b 100644
> --- a/kernel/crash_dump_dm_crypt.c
> +++ b/kernel/crash_dump_dm_crypt.c
> @@ -88,21 +88,31 @@ static int get_keys_from_kdump_reserved_memory(void)
>   {
>   	struct keys_header *keys_header_loaded;
>   	size_t keys_header_size;
> -
> -	keys_header_size = get_keys_header_size(key_count);
> -	keys_header = kzalloc(keys_header_size, GFP_KERNEL);
> -	if (!keys_header)
> -		return -ENOMEM;
> +	int r = 0;
>   
>   	arch_kexec_unprotect_crashkres();
>   	keys_header_loaded = kmap_local_page(pfn_to_page(
>   		kexec_crash_image->dm_crypt_keys_addr >> PAGE_SHIFT));
>   
> +	if (keys_header_loaded->total_keys <= 0 ||
> +	    keys_header_loaded->total_keys > KEY_NUM_MAX) {
> +		pr_warn("keys_header saved to reserved memory may be corrupt\n");
> +		r = -EINVAL;
> +		goto kunmap;
> +	}
> +
> +	keys_header_size = get_keys_header_size(keys_header_loaded->total_keys);
> +	keys_header = kzalloc(keys_header_size, GFP_KERNEL);
> +	if (!keys_header) {
> +		r = -ENOMEM;
> +		goto kunmap;
> +	}
> +
>   	memcpy(keys_header, keys_header_loaded, keys_header_size);
> +kunmap:
>   	kunmap_local(keys_header_loaded);
>   	arch_kexec_protect_crashkres();
> -
> -	return 0;
> +	return r;
>   }
>   
>   static int restore_dm_crypt_keys_to_thread_keyring(void)
> @@ -446,12 +456,12 @@ int crash_load_dm_crypt_keys(struct kimage *image)
>   	mutex_lock(&config_keys_subsys.su_mutex);
>   	mutex_acquired = true;
>   
> -	if (key_count <= 0) {
> -		kexec_dprintk("No dm-crypt keys\n");
> -		return 0;
> -	}
> -
>   	if (!is_dm_key_reused) {
> +		if (key_count <= 0) {
> +			kexec_dprintk("No dm-crypt keys\n");
> +			return 0;
> +		}
> +
>   		r = build_keys_header();
>   		if (r)
>   			goto out;
> @@ -462,7 +472,7 @@ int crash_load_dm_crypt_keys(struct kimage *image)
>   	 * cleaned up at the end of kexec_file_load syscall
>   	 */
>   	kbuf.buffer = keys_header;
> -	kbuf.bufsz = get_keys_header_size(key_count);
> +	kbuf.bufsz = get_keys_header_size(keys_header->total_keys);
>   
>   	kbuf.memsz = kbuf.bufsz;
>   	kbuf.buf_align = ELF_CORE_HEADER_ALIGN;