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

[Coiby Xu <coxu@redhat.com>]

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