Re: [PATCH v5 8/9] crash_dump: Disallow configfs/crash_dm_crypt_key/reuse if crash hotplug supported

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Setting `image->dm_crypt_keys_addr = 0` during kexec image cleanup bypasses page table memory protections, leaving sensitive dm-crypt volume logon keys accessible/readable in the primary kernel's memory.

Pre-existing issues:
- [High] Reused dm-crypt keys are permanently lost and silently dropped during ARM64 kexec segment placement retries due to premature freeing of `keys_header`.
--

commit ccd76380ffc2464293abaa1bf7ed098e55227890
Author: Coiby Xu <coiby.xu@gmail.com>

crash_dump: Disallow configfs/crash_dm_crypt_key/reuse if crash hotplug supported

If crash hotplug is supported, dm-crypt keys saved to reserved memory
will be taken care of automatically. Thus it doesn't make sense to use
configfs/crash_dm_crypt_key/reuse. Not reserving
image->dm_crypt_keys_addr makes it implicitly to disallow using this
API. Currently x86_64 and ppc64le have implemented crash hotplug
feature.

Also update the doc accordingly. Note two doc issues are fixed as well.

> diff --git a/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c
> index bc70fbb79e0f6..666c1f0c9e3b4 100644
> --- a/kernel/crash_dump_dm_crypt.c
> +++ b/kernel/crash_dump_dm_crypt.c

[ ... ]

> @@ -522,14 +522,18 @@ int crash_load_dm_crypt_keys(struct kimage *image)
>  void crash_dm_crypt_cleanup(struct kimage *image)
>  {
>  	/*
> -	 * For CPU/memory hot-plugging, the kdump image will be reloaded. Prevent
> -	 * keys_header from being cleaned up during unloading when
> -	 * is_dm_key_reused=true
> +	 * For CPU/memory hot-plugging without CONFIG_CRASH_HOTPLUG, the whole kdump
> +	 * image will be reloaded. Prevent keys_header from being cleaned up during
> +	 * unloading when is_dm_key_reused=true
>  	 */
>  	if (!is_dm_key_reused) {

[Severity: High]
This is a pre-existing issue, but can reused dm-crypt keys be permanently
lost during ARM64 kexec segment placement retries due to premature freeing
of keys_header?

On ARM64, if the first kexec segment placement attempt succeeds for dm-crypt
keys but fails on a subsequent segment (like initrd), it triggers a retry
loop. On the first pass, crash_load_dm_crypt_keys() unsets the
is_dm_key_reused flag:

kernel/crash_dump_dm_crypt.c:crash_load_dm_crypt_keys() {
    ...
out:
    is_dm_key_reused = false;
    return r;
}

The error path then calls crash_dm_crypt_cleanup(), which unconditionally
frees keys_header since the flag is now false:

arch/arm64/kernel/kexec_image.c:image_load() {
    ...
        image->nr_segments -= 1;
        kbuf.buf_min = kernel_segment->mem + kernel_segment->memsz;
        kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
        crash_dm_crypt_cleanup(image);
    ...
}

On the next iteration, crash_load_dm_crypt_keys() is called again but exits
early without allocating keys, leaving the crash kernel without decryption
keys.

>  		kfree_sensitive(keys_header);
>  		keys_header = NULL;
>  	}
> +#ifdef CONFIG_CRASH_HOTPLUG
> +	if (image->hotplug_support)
> +		image->dm_crypt_keys_addr = 0;

[Severity: High]
Does setting image->dm_crypt_keys_addr to 0 bypass page table memory
protections, leaving sensitive dm-crypt volume logon keys accessible in the
primary kernel's memory?

When crash_dm_crypt_cleanup() is called during kexec file load, it zeroes
image->dm_crypt_keys_addr. Subsequently, sys_kexec_file_load() applies
memory protections:

kernel/kexec_file.c:sys_kexec_file_load() {
    ...
    kimage_file_post_load_cleanup(image);
exchange:
    image = xchg(dest_image, image);
out:
#ifdef CONFIG_CRASH_DUMP
    if ((flags & KEXEC_FILE_ON_CRASH) && kexec_crash_image)
        arch_kexec_protect_crashkres();
    ...
}

Immediately afterward, kexec_mark_dm_crypt_keys() (called by
arch_kexec_protect_crashkres()) checks this address:

arch/x86/kernel/machine_kexec_64.c:kexec_mark_dm_crypt_keys() {
    ...
    if (kexec_crash_image->dm_crypt_keys_addr) {
        start_paddr = kexec_crash_image->dm_crypt_keys_addr;
        ...
        set_memory_np(...);
    ...
}

Since the address evaluates to 0, it completely skips applying the
non-present page table protections (set_memory_np()), exposing decryption
keys in system RAM.

> +#endif
>  }
>  
>  static int __init configfs_dmcrypt_keys_init(void)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909003657.1570544-1-coiby.xu@gmail.com?part=8