Re: [PATCH v5 3/3] arm64,ppc64le/kdump: pass dm-crypt keys to kdump kernel

Coiby Xu
2026-04-03T09:40:29Z

On Thu, Apr 02, 2026 at 04:24:14PM +0530, Sourabh Jain wrote:
>
>
>On 25/02/26 11:33, Coiby Xu wrote:
>>CONFIG_CRASH_DM_CRYPT has been introduced to support LUKS-encrypted
>>device dump target by addressing two challenges [1],
>> - Kdump kernel may not be able to decrypt the LUKS partition. For some
>> machines, a system administrator may not have a chance to enter the
>> password to decrypt the device in kdump initramfs after the 1st kernel
>> crashes
>>
>> - LUKS2 by default use the memory-hard Argon2 key derivation function
>> which is quite memory-consuming compared to the limited memory reserved
>> for kdump.
>>
>>To also enable this feature for ARM64 and PowerPC, the missing piece is
>>to let the kdump kernel know where to find the dm-crypt keys which are
>>randomly stored in memory reserved for kdump. Introduce a new device
>>tree property dmcryptkeys [2] as similar to elfcorehdr to pass the
>>memory address of the stored info of dm-crypt keys to the kdump kernel.
>>Since this property is only needed by the kdump kernel, it won't be
>>exposed to user space.
>>
>>[1] https://lore.kernel.org/all/20250502011246.99238-1-coxu@redhat.com/
>>[2] https://github.com/devicetree-org/dt-schema/pull/181
>>
>>Cc: Arnaud Lefebvre <arnaud.lefebvre@clever-cloud.com>
>>Cc: Baoquan he <bhe@redhat.com>
>>Cc: Dave Young <dyoung@redhat.com>
>>Cc: Kairui Song <ryncsn@gmail.com>
>>Cc: Pingfan Liu <kernelfans@gmail.com>
>>Cc: Andrew Morton <akpm@linux-foundation.org>
>>Cc: Krzysztof Kozlowski <krzk@kernel.org>
>>Cc: Rob Herring <robh@kernel.org>
>>Cc: Thomas Staudt <tstaudt@de.ibm.com>
>>Cc: Sourabh Jain <sourabhjain@linux.ibm.com>
>>Cc: Will Deacon <will@kernel.org>
>>Cc: Christophe Leroy (CS GROUP) <chleroy@kernel.org>
>>Signed-off-by: Coiby Xu <coxu@redhat.com>
>>---
>> arch/arm64/kernel/machine_kexec_file.c | 4 ++++
>> arch/powerpc/kexec/elf_64.c | 4 ++++
>> drivers/of/fdt.c | 21 +++++++++++++++++++++
>> drivers/of/kexec.c | 19 +++++++++++++++++++
>> 4 files changed, 48 insertions(+)
>>
>>diff --git a/arch/arm64/kernel/machine_kexec_file.c b/arch/arm64/kernel/machine_kexec_file.c
>>index fba260ad87a9..e31fabed378a 100644
>>--- a/arch/arm64/kernel/machine_kexec_file.c
>>+++ b/arch/arm64/kernel/machine_kexec_file.c
>>@@ -134,6 +134,10 @@ int load_other_segments(struct kimage *image,
>> kexec_dprintk("Loaded elf core header at 0x%lx bufsz=0x%lx memsz=0x%lx\n",
>> image->elf_load_addr, kbuf.bufsz, kbuf.memsz);
>>+
>>+ ret = crash_load_dm_crypt_keys(image);
>>+ if (ret)
>>+ goto out_err;
>> }
>> #endif
>>diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
>>index 5d6d616404cf..ea50a072debf 100644
>>--- a/arch/powerpc/kexec/elf_64.c
>>+++ b/arch/powerpc/kexec/elf_64.c
>>@@ -79,6 +79,10 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
>> goto out;
>> }
>>+ ret = crash_load_dm_crypt_keys(image);
>>+ if (ret)
>>+ goto out;
>>+
>> /* Setup cmdline for kdump kernel case */
>> modified_cmdline = setup_kdump_cmdline(image, cmdline,
>> cmdline_len);
>>diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
>>index 331646d667b9..2967e4aff807 100644
>>--- a/drivers/of/fdt.c
>>+++ b/drivers/of/fdt.c
>>@@ -866,6 +866,26 @@ static void __init early_init_dt_check_for_elfcorehdr(unsigned long node)
>> elfcorehdr_addr, elfcorehdr_size);
>> }
>>+static void __init early_init_dt_check_for_dmcryptkeys(unsigned long node)
>>+{
>>+ const char *prop_name = "linux,dmcryptkeys";
>>+ const __be32 *prop;
>>+
>>+ if (!IS_ENABLED(CONFIG_CRASH_DM_CRYPT))
>>+ return;
>>+
>>+ pr_debug("Looking for dmcryptkeys property... ");
>>+
>>+ prop = of_get_flat_dt_prop(node, prop_name, NULL);
>>+ if (!prop)
>>+ return;
>>+
>>+ dm_crypt_keys_addr = dt_mem_next_cell(dt_root_addr_cells, &prop);
>>+
>>+ /* Property only accessible to crash dump kernel */
>>+ fdt_delprop(initial_boot_params, node, prop_name);
>>+}
>>+
>> static unsigned long chosen_node_offset = -FDT_ERR_NOTFOUND;
>> /*
>>@@ -1097,6 +1117,7 @@ int __init early_init_dt_scan_chosen(char *cmdline)
>> early_init_dt_check_for_initrd(node);
>> early_init_dt_check_for_elfcorehdr(node);
>>+ early_init_dt_check_for_dmcryptkeys(node);
>> rng_seed = of_get_flat_dt_prop(node, "rng-seed", &l);
>> if (rng_seed && l > 0) {
>>diff --git a/drivers/of/kexec.c b/drivers/of/kexec.c
>>index c4cf3552c018..fbd253f0d3c5 100644
>>--- a/drivers/of/kexec.c
>>+++ b/drivers/of/kexec.c
>>@@ -423,6 +423,25 @@ void *of_kexec_alloc_and_setup_fdt(const struct kimage *image,
>> if (ret)
>> goto out;
>>+ if (image->dm_crypt_keys_addr != 0) {
>>+ ret = fdt_appendprop_addrrange(fdt, 0, chosen_node,
>>+ "linux,dmcryptkeys",
>>+ image->dm_crypt_keys_addr,
>>+ image->dm_crypt_keys_sz);
>>+
>>+ if (ret)
>>+ goto out;
>>+
>>+ /*
>>+ * Avoid dmcryptkeys from being stomped on in kdump kernel by
>>+ * setting up memory reserve map.
>>+ */
>>+ ret = fdt_add_mem_rsv(fdt, image->dm_crypt_keys_addr,
>>+ image->dm_crypt_keys_sz);
>>+ if (ret)
>>+ goto out;
>>+ }
>>+
>> #ifdef CONFIG_CRASH_DUMP
>> /* add linux,usable-memory-range */
>> ret = fdt_appendprop_addrrange(fdt, 0, chosen_node,
>
>The above changes look good to me.
>
>Feel free to add:
>Reviewed-by: Sourabh Jain <sourabhjain@linux.ibm.com>

Thanks for reviewing the patch!

>
>But while reading crash_load_dm_crypt_keys() I noticed a possibility of a
>double free at the address pointed by `keys_header`:
>
>In crash_load_dm_crypt_keys()/crash_dump_dm_crypt.c
>    snip...
>
>    kbuf.buffer = keys_header;
>
>    snip....
>
>    r = kexec_add_buffer(&kbuf);
>    if (r) {
>        pr_err("Failed to call kexec_add_buffer, ret=%d\n", r);
>        kvfree((void *)kbuf.buffer);                           <---
>First Free
>        return r;
>    }
>
>Since `keys_header` is not reset, the next call to build_keys_header()
>will cause a double free at `keys_header`.
>
>static int build_keys_header(void)
>{
>
>    snip...
>
>    if (keys_header != NULL)
>        kvfree(keys_header);
>
>    snip...
>}
>
>What do you think?
>
>- Sourabh Jain

Good catch! I'll send a patch to address this issue. Thanks!

--
Best regards,
Coiby