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 <[email protected]> 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/[email protected]?part=8
