[PATCH v5 1/9] crash_dump: Fix potential double free and UAF of keys_header

[PATCH v5 1/9] crash_dump: Fix potential double free and UAF of keys_header

[Coiby Xu]

If kexec_add_buffer somehow fails, keys_header will be freed. Depending
on /sys/kernel/config/crash_dm_crypt_key/reuse, it will lead to the
following two problems if the crash_load_dm_crypt_keys is called again,
  1. Double free of keys_header if reuse=false
  2. UAF of keys_header if reuse=true

To address these problems and also make it easier to reason about the
code, keep two invariants,
  1. keys_header will always be freed at the end of kexec_file_load
     syscall except during kdump image unloading for CPU/memory
     hot-plugging support
  2. There will always be valid keys_header if reuse=true

Since dm-crypt keys are sensitive data, always wiping keys_header
can also potentially reduce attack surface.

For ARM64, because of commit 108aa503657e ("arm64: kexec_file: try more
regions if loading segments fails"), keys_header should be freed before
retry.

Fixes: 479e58549b0f ("crash_dump: store dm crypt keys in kdump reserved memory")
Fixes: 9ebfa8dcaea7 ("crash_dump: reuse saved dm crypt keys for CPU/memory hot-plugging")
Reported-by: Sourabh Jain <sourabhjain@linux.ibm.com>
Reported-by: Jinjie Ruan <ruanjinjie@huawei.com>
Signed-off-by: Coiby Xu <coiby.xu@gmail.com>
---
 arch/arm64/kernel/kexec_image.c |  1 +
 include/linux/kexec.h           |  6 +++
 kernel/crash_dump_dm_crypt.c    | 68 ++++++++++++++++++++++++---------
 kernel/kexec_file.c             |  2 +
 4 files changed, 59 insertions(+), 18 deletions(-)

diff --git a/arch/arm64/kernel/kexec_image.c b/arch/arm64/kernel/kexec_image.c
index b70f4df15a1a..1a0ca54adc9a 100644
--- a/arch/arm64/kernel/kexec_image.c
+++ b/arch/arm64/kernel/kexec_image.c
@@ -110,6 +110,7 @@ static void *image_load(struct kimage *image,
 		image->nr_segments -= 1;
 		kbuf.buf_min = kernel_segment->mem + kernel_segment->memsz;
 		kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
+		crash_dm_crypt_cleanup(image);
 	}
 
 	if (ret) {
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index 0af8ae4fdd08..82ee550e3cd0 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -552,6 +552,12 @@ void set_kexec_sig_enforced(void);
 static inline void set_kexec_sig_enforced(void) {}
 #endif
 
+#ifdef CONFIG_CRASH_DM_CRYPT
+void crash_dm_crypt_cleanup(struct kimage *image);
+#else
+static inline void crash_dm_crypt_cleanup(struct kimage *image) {}
+#endif
+
 #endif /* !defined(__ASSEBMLY__) */
 
 #endif /* LINUX_KEXEC_H */
diff --git a/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c
index c685497cd470..f638fb4b8554 100644
--- a/kernel/crash_dump_dm_crypt.c
+++ b/kernel/crash_dump_dm_crypt.c
@@ -84,18 +84,25 @@ static int add_key_to_keyring(struct dm_crypt_key *dm_key,
 	return r;
 }
 
-static void get_keys_from_kdump_reserved_memory(void)
+static int get_keys_from_kdump_reserved_memory(void)
 {
 	struct keys_header *keys_header_loaded;
+	size_t keys_header_size;
 
-	arch_kexec_unprotect_crashkres();
+	keys_header_size = get_keys_header_size(key_count);
+	keys_header = kzalloc(keys_header_size, GFP_KERNEL);
+	if (!keys_header)
+		return -ENOMEM;
 
+	arch_kexec_unprotect_crashkres();
 	keys_header_loaded = kmap_local_page(pfn_to_page(
 		kexec_crash_image->dm_crypt_keys_addr >> PAGE_SHIFT));
 
-	memcpy(keys_header, keys_header_loaded, get_keys_header_size(key_count));
+	memcpy(keys_header, keys_header_loaded, keys_header_size);
 	kunmap_local(keys_header_loaded);
 	arch_kexec_protect_crashkres();
+
+	return 0;
 }
 
 static int restore_dm_crypt_keys_to_thread_keyring(void)
@@ -286,17 +293,28 @@ static ssize_t config_keys_reuse_show(struct config_item *item, char *page)
 static ssize_t config_keys_reuse_store(struct config_item *item,
 					   const char *page, size_t count)
 {
+	bool val;
+	int r;
+
 	if (!kexec_crash_image || !kexec_crash_image->dm_crypt_keys_addr) {
 		kexec_dprintk(
 			"dm-crypt keys haven't be saved to crash-reserved memory\n");
 		return -EINVAL;
 	}
 
-	if (kstrtobool(page, &is_dm_key_reused))
+	if (kstrtobool(page, &val) || !val)
 		return -EINVAL;
 
-	if (is_dm_key_reused)
-		get_keys_from_kdump_reserved_memory();
+	if (is_dm_key_reused) {
+		pr_info("Already got dm-crypt keys, please continue with kexec_file_load syscall\n");
+	} else {
+		r = get_keys_from_kdump_reserved_memory();
+		if (r) {
+			pr_warn("Failed to get dm-crypt keys from reserved memory\n");
+			return r;
+		}
+		is_dm_key_reused = true;
+	}
 
 	return count;
 }
@@ -369,9 +387,6 @@ static int build_keys_header(void)
 	struct config_key *key;
 	int i, r;
 
-	if (keys_header != NULL)
-		kvfree(keys_header);
-
 	keys_header = kzalloc(get_keys_header_size(key_count), GFP_KERNEL);
 	if (!keys_header)
 		return -ENOMEM;
@@ -415,8 +430,7 @@ int crash_load_dm_crypt_keys(struct kimage *image)
 		.top_down = false,
 		.random = true,
 	};
-	int r;
-
+	int r = 0;
 
 	if (key_count <= 0) {
 		kexec_dprintk("No dm-crypt keys\n");
@@ -424,14 +438,17 @@ int crash_load_dm_crypt_keys(struct kimage *image)
 	}
 
 	if (!is_dm_key_reused) {
-		image->dm_crypt_keys_addr = 0;
 		r = build_keys_header();
-		if (r) {
-			pr_err("Failed to build dm-crypt keys header, ret=%d\n", r);
-			return r;
-		}
+		if (r)
+			goto out;
 	}
 
+	/*
+	 * keys_header will be copied to reserver memory later and then be
+	 * cleaned up by calling crash_dm_crypt_cleanup at the end of
+	 * kexec_file_load syscall or during re-try load_other_segments for
+	 * ARM64
+	 */
 	kbuf.buffer = keys_header;
 	kbuf.bufsz = get_keys_header_size(key_count);
 
@@ -441,18 +458,33 @@ int crash_load_dm_crypt_keys(struct kimage *image)
 	r = kexec_add_buffer(&kbuf);
 	if (r) {
 		pr_err("Failed to call kexec_add_buffer, ret=%d\n", r);
-		kvfree((void *)kbuf.buffer);
-		return r;
+		goto out;
 	}
+
 	image->dm_crypt_keys_addr = kbuf.mem;
 	image->dm_crypt_keys_sz = kbuf.bufsz;
 	kexec_dprintk(
 		"Loaded dm crypt keys to kexec_buffer bufsz=0x%lx memsz=0x%lx\n",
 		kbuf.bufsz, kbuf.memsz);
 
+out:
+	is_dm_key_reused = false;
 	return r;
 }
 
+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
+	 */
+	if (!is_dm_key_reused) {
+		kfree_sensitive(keys_header);
+		keys_header = NULL;
+	}
+}
+
 static int __init configfs_dmcrypt_keys_init(void)
 {
 	int ret;
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 59fb9d71e9d8..5609301db0bf 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -140,6 +140,8 @@ void kimage_file_post_load_cleanup(struct kimage *image)
 	kfree(image->image_loader_data);
 	image->image_loader_data = NULL;
 
+	crash_dm_crypt_cleanup(image);
+
 	kexec_file_dbg_print = false;
 }
 
-- 
2.55.0

[PATCH v5 2/9] crash_dump: Read the number of dm-crypt keys from reserved memory

[Coiby Xu]

In case user adds/deletes the keys by mistake, it's safer to read the
number of keys from reserved memory.

Fixes: 9ebfa8dcaea7 ("crash_dump: reuse saved dm crypt keys for CPU/memory hot-plugging")
Reported-and-Suggested-by: Sourabh Jain <sourabhjain@linux.ibm.com>
Signed-off-by: Coiby Xu <coiby.xu@gmail.com>
---
 kernel/crash_dump_dm_crypt.c | 37 +++++++++++++++++++++++-------------
 1 file changed, 24 insertions(+), 13 deletions(-)

diff --git a/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c
index f638fb4b8554..3a416933979f 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)
@@ -432,12 +442,13 @@ int crash_load_dm_crypt_keys(struct kimage *image)
 	};
 	int r = 0;
 
-	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");
+			r = 0;
+			goto out;
+		}
+
 		r = build_keys_header();
 		if (r)
 			goto out;
@@ -450,7 +461,7 @@ int crash_load_dm_crypt_keys(struct kimage *image)
 	 * ARM64
 	 */
 	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;
-- 
2.55.0

[PATCH v5 3/9] crash_dump: Disallow writing to dm-crypt configfs during kexec_file_load syscall

[Coiby Xu]

If writing to the configfs group happens concurrently during
kexec_file_load syscall, it may lead to the following issues,
  - buffer overflow if dm-crypt keys are added after allocation
  - stale total_keys if dm-crypt keys are removed during iteration
  - keys_header will not be freed if config/crash_dm_crypt_key/reuse is
    set true

So hold config_keys_subsys.su_mutex for the entire sequence during the
kexec_file_load syscall to ensure a consistent snapshot. To have serial
access to config/crash_dm_crypt_key/reuse, use the kexec lock as we also
need to access kexec_crash_image serially.

Fixes: 479e58549b0f ("crash_dump: store dm crypt keys in kdump reserved memory")
Suggested-by: Sourabh Jain <sourabhjain@linux.ibm.com>
Signed-off-by: Coiby Xu <coiby.xu@gmail.com>
---
 kernel/crash_dump_dm_crypt.c | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c
index 3a416933979f..580534d43b66 100644
--- a/kernel/crash_dump_dm_crypt.c
+++ b/kernel/crash_dump_dm_crypt.c
@@ -7,6 +7,7 @@
 #include <linux/configfs.h>
 #include <linux/module.h>
 #include <linux/sysfs.h>
+#include "kexec_internal.h"
 
 #define KEY_NUM_MAX 128	/* maximum dm crypt keys */
 #define KEY_SIZE_MAX 256	/* maximum dm crypt key size */
@@ -306,14 +307,20 @@ static ssize_t config_keys_reuse_store(struct config_item *item,
 	bool val;
 	int r;
 
+	if (!kexec_trylock()) {
+		pr_warn("Failed to acquire the kexec lock\n");
+		return -EBUSY;
+	}
+
+	r = -EINVAL;
 	if (!kexec_crash_image || !kexec_crash_image->dm_crypt_keys_addr) {
 		kexec_dprintk(
 			"dm-crypt keys haven't be saved to crash-reserved memory\n");
-		return -EINVAL;
+		goto unlock;
 	}
 
 	if (kstrtobool(page, &val) || !val)
-		return -EINVAL;
+		goto unlock;
 
 	if (is_dm_key_reused) {
 		pr_info("Already got dm-crypt keys, please continue with kexec_file_load syscall\n");
@@ -321,12 +328,15 @@ static ssize_t config_keys_reuse_store(struct config_item *item,
 		r = get_keys_from_kdump_reserved_memory();
 		if (r) {
 			pr_warn("Failed to get dm-crypt keys from reserved memory\n");
-			return r;
+			goto unlock;
 		}
 		is_dm_key_reused = true;
 	}
 
-	return count;
+	r = count;
+unlock:
+	kexec_unlock();
+	return r;
 }
 
 CONFIGFS_ATTR(config_keys_, reuse);
@@ -443,15 +453,17 @@ int crash_load_dm_crypt_keys(struct kimage *image)
 	int r = 0;
 
 	if (!is_dm_key_reused) {
+		mutex_lock(&config_keys_subsys.su_mutex);
+
 		if (key_count <= 0) {
 			kexec_dprintk("No dm-crypt keys\n");
 			r = 0;
-			goto out;
+			goto unlock;
 		}
 
 		r = build_keys_header();
 		if (r)
-			goto out;
+			goto unlock;
 	}
 
 	/*
@@ -478,6 +490,9 @@ int crash_load_dm_crypt_keys(struct kimage *image)
 		"Loaded dm crypt keys to kexec_buffer bufsz=0x%lx memsz=0x%lx\n",
 		kbuf.bufsz, kbuf.memsz);
 
+unlock:
+	mutex_unlock(&config_keys_subsys.su_mutex);
+
 out:
 	is_dm_key_reused = false;
 	return r;
-- 
2.55.0

[PATCH v5 4/9] crash_dump: Free temporary dm-crypt keys_header buffer in kdump kernel

[Coiby Xu]

Although we expect the system to reboot immediately after vmcore dumping
is finished, it's still good to free the temporary keys_header buffer.

Fixes: 62f17d9df692 ("crash_dump: retrieve dm crypt keys in kdump kernel")
Reported-by: Sourabh Jain <sourabhjain@linux.ibm.com>
Reviewed-by: Sourabh Jain <sourabhjain@linux.ibm.com>
Signed-off-by: Coiby Xu <coiby.xu@gmail.com>
---
 kernel/crash_dump_dm_crypt.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c
index 580534d43b66..ff6055a97a4b 100644
--- a/kernel/crash_dump_dm_crypt.c
+++ b/kernel/crash_dump_dm_crypt.c
@@ -118,6 +118,7 @@ static int get_keys_from_kdump_reserved_memory(void)
 
 static int restore_dm_crypt_keys_to_thread_keyring(void)
 {
+	struct keys_header *keys_header __free(kfree_sensitive) = NULL;
 	struct dm_crypt_key *key;
 	size_t keys_header_size;
 	key_ref_t keyring_ref;
-- 
2.55.0

[PATCH v5 5/9] crash_dump: Only use kexec_dprintk during the kexec_file_load syscall

[Coiby Xu]

kexec_dprintk will only be activated by "kexec -d" during
kexec_file_load syscall. So use pr_* outside of this syscall.

Fixes: 9ebfa8dcaea7 ("crash_dump: reuse saved dm crypt keys for CPU/memory hot-plugging")
Fixes: 62f17d9df692 ("crash_dump: retrieve dm crypt keys in kdump kernel")
Reviewed-by: Sourabh Jain <sourabhjain@linux.ibm.com>
Signed-off-by: Coiby Xu <coiby.xu@gmail.com>
---
 kernel/crash_dump_dm_crypt.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c
index ff6055a97a4b..9ad50ac54866 100644
--- a/kernel/crash_dump_dm_crypt.c
+++ b/kernel/crash_dump_dm_crypt.c
@@ -76,10 +76,10 @@ static int add_key_to_keyring(struct dm_crypt_key *dm_key,
 	if (!IS_ERR(key_ref)) {
 		r = key_ref_to_ptr(key_ref)->serial;
 		key_ref_put(key_ref);
-		kexec_dprintk("Success adding key %s", dm_key->key_desc);
+		pr_debug("Success adding key %s\n", dm_key->key_desc);
 	} else {
 		r = PTR_ERR(key_ref);
-		kexec_dprintk("Error when adding key");
+		pr_warn("Error when adding key, ret=%d\n", r);
 	}
 
 	return r;
@@ -129,19 +129,19 @@ static int restore_dm_crypt_keys_to_thread_keyring(void)
 	keyring_ref =
 		lookup_user_key(KEY_SPEC_USER_KEYRING, 0x01, KEY_NEED_WRITE);
 	if (IS_ERR(keyring_ref)) {
-		kexec_dprintk("Failed to get the user keyring\n");
+		pr_warn("Failed to get the user keyring\n");
 		return PTR_ERR(keyring_ref);
 	}
 
 	addr = dm_crypt_keys_addr;
 	dm_crypt_keys_read((char *)&key_count, sizeof(key_count), &addr);
 	if (key_count > KEY_NUM_MAX) {
-		kexec_dprintk("Failed to read the number of dm-crypt keys\n");
+		pr_warn("Failed to read the number of dm-crypt keys\n");
 		ret = -1;
 		goto out;
 	}
 
-	kexec_dprintk("There are %u keys\n", key_count);
+	pr_debug("There are %u keys\n", key_count);
 	addr = dm_crypt_keys_addr;
 
 	keys_header_size = get_keys_header_size(key_count);
@@ -155,7 +155,7 @@ static int restore_dm_crypt_keys_to_thread_keyring(void)
 
 	for (int i = 0; i < keys_header->total_keys; i++) {
 		key = &keys_header->keys[i];
-		kexec_dprintk("Get key (size=%u)\n", key->key_size);
+		pr_debug("Get key (size=%u)\n", key->key_size);
 		add_key_to_keyring(key, keyring_ref);
 	}
 
@@ -315,8 +315,7 @@ static ssize_t config_keys_reuse_store(struct config_item *item,
 
 	r = -EINVAL;
 	if (!kexec_crash_image || !kexec_crash_image->dm_crypt_keys_addr) {
-		kexec_dprintk(
-			"dm-crypt keys haven't be saved to crash-reserved memory\n");
+		pr_debug("dm-crypt keys haven't be saved to crash-reserved memory\n");
 		goto unlock;
 	}
 
-- 
2.55.0

[PATCH v5 6/9] crash_dump: Improve readability of config_keys_restore_store

[Coiby Xu]

config_keys_restore_store currently doesn't validate the user input
before restoring dm-crypt keys. Although it's not necessary for the case
of vmcore dumping, it's better to do it for the sake of consistency and
code readability. Also check the return code of
restore_dm_crypt_keys_to_thread_keyring.

Fixes: 62f17d9df692 ("crash_dump: retrieve dm crypt keys in kdump kernel")
Reviewed-by: Sourabh Jain <sourabhjain@linux.ibm.com>
Signed-off-by: Coiby Xu <coiby.xu@gmail.com>
---
 kernel/crash_dump_dm_crypt.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c
index 9ad50ac54866..b8fe630462cf 100644
--- a/kernel/crash_dump_dm_crypt.c
+++ b/kernel/crash_dump_dm_crypt.c
@@ -371,13 +371,25 @@ static ssize_t config_keys_restore_show(struct config_item *item, char *page)
 static ssize_t config_keys_restore_store(struct config_item *item,
 					  const char *page, size_t count)
 {
-	if (!restore)
-		restore_dm_crypt_keys_to_thread_keyring();
+	bool val;
+	int r;
 
-	if (kstrtobool(page, &restore))
+	if (kstrtobool(page, &val))
 		return -EINVAL;
 
-	return count;
+	if (val) {
+		if (restore) {
+			pr_warn("dm-crypt keys already restored!\n");
+			return count;
+		}
+		r = restore_dm_crypt_keys_to_thread_keyring();
+		if (!r) {
+			restore = true;
+			r = count;
+		}
+	}
+
+	return r;
 }
 
 CONFIGFS_ATTR(config_keys_, restore);
-- 
2.55.0

[PATCH v5 7/9] crash_dump: Check the function return codes in restore_dm_crypt_keys_to_thread_keyring

[Coiby Xu]

We should check the return codes so we can abort if keyring allocation
or reading old memory fails.

Note there is no need to refer a key add_key_to_keyring, so delete
related code.

Reviewed-by: Sourabh Jain <sourabhjain@linux.ibm.com>
Signed-off-by: Coiby Xu <coiby.xu@gmail.com>
---
 kernel/crash_dump_dm_crypt.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c
index b8fe630462cf..bc70fbb79e0f 100644
--- a/kernel/crash_dump_dm_crypt.c
+++ b/kernel/crash_dump_dm_crypt.c
@@ -66,7 +66,7 @@ static int add_key_to_keyring(struct dm_crypt_key *dm_key,
 			      key_ref_t keyring_ref)
 {
 	key_ref_t key_ref;
-	int r;
+	int r = 0;
 
 	/* create or update the requested key and add it to the target keyring */
 	key_ref = key_create_or_update(keyring_ref, "user", dm_key->key_desc,
@@ -74,8 +74,6 @@ static int add_key_to_keyring(struct dm_crypt_key *dm_key,
 				       KEY_USR_ALL, KEY_ALLOC_IN_QUOTA);
 
 	if (!IS_ERR(key_ref)) {
-		r = key_ref_to_ptr(key_ref)->serial;
-		key_ref_put(key_ref);
 		pr_debug("Success adding key %s\n", dm_key->key_desc);
 	} else {
 		r = PTR_ERR(key_ref);
@@ -134,9 +132,14 @@ static int restore_dm_crypt_keys_to_thread_keyring(void)
 	}
 
 	addr = dm_crypt_keys_addr;
-	dm_crypt_keys_read((char *)&key_count, sizeof(key_count), &addr);
+	ret = dm_crypt_keys_read((char *)&key_count, sizeof(key_count), &addr);
+	if (ret < 0) {
+		pr_err("Failed to read the number of dm-crypt keys\n");
+		goto out;
+	}
+
 	if (key_count > KEY_NUM_MAX) {
-		pr_warn("Failed to read the number of dm-crypt keys\n");
+		pr_warn("Read %u dm-crypt keys (max=%u)\n", key_count, KEY_NUM_MAX);
 		ret = -1;
 		goto out;
 	}
@@ -151,12 +154,18 @@ static int restore_dm_crypt_keys_to_thread_keyring(void)
 		goto out;
 	}
 
-	dm_crypt_keys_read((char *)keys_header, keys_header_size, &addr);
+	ret = dm_crypt_keys_read((char *)keys_header, keys_header_size, &addr);
+	if (ret < 0) {
+		pr_err("Failed to read dm-crypt keys\n");
+		goto out;
+	}
 
 	for (int i = 0; i < keys_header->total_keys; i++) {
 		key = &keys_header->keys[i];
 		pr_debug("Get key (size=%u)\n", key->key_size);
-		add_key_to_keyring(key, keyring_ref);
+		ret = add_key_to_keyring(key, keyring_ref);
+		if (ret)
+			break;
 	}
 
 out:
-- 
2.55.0

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

[Coiby Xu]

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.

Fixes: 9ebfa8dcaea7 ("crash_dump: reuse saved dm crypt keys for CPU/memory hot-plugging")
Reviewed-by: Sourabh Jain <sourabhjain@linux.ibm.com>
Signed-off-by: Coiby Xu <coiby.xu@gmail.com>
---
 Documentation/admin-guide/kdump/kdump.rst | 16 ++++++++++------
 kernel/crash_dump_dm_crypt.c              | 12 ++++++++----
 2 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/Documentation/admin-guide/kdump/kdump.rst b/Documentation/admin-guide/kdump/kdump.rst
index 7587caadbae1..0bf2eb100a05 100644
--- a/Documentation/admin-guide/kdump/kdump.rst
+++ b/Documentation/admin-guide/kdump/kdump.rst
@@ -577,9 +577,10 @@ with /sys/kernel/config/crash_dm_crypt_keys for setup,
 
 1. Tell the first kernel what logon keys are needed to unlock the disk volumes,
     # Add key #1
-    mkdir /sys/kernel/config/crash_dm_crypt_keys/7d26b7b4-e342-4d2d-b660-7426b0996720
+    VOL1_UUID=7d26b7b4-e342-4d2d-b660-7426b0996720
+    mkdir /sys/kernel/config/crash_dm_crypt_keys/$VOL1_UUID
     # Add key #1's description
-    echo cryptsetup:7d26b7b4-e342-4d2d-b660-7426b0996720 > /sys/kernel/config/crash_dm_crypt_keys/description
+    echo cryptsetup:$VOL1_UUID > /sys/kernel/config/crash_dm_crypt_keys/$VOL1_UUID/description
 
     # how many keys do we have now?
     cat /sys/kernel/config/crash_dm_crypt_keys/count
@@ -591,15 +592,18 @@ with /sys/kernel/config/crash_dm_crypt_keys for setup,
     cat /sys/kernel/config/crash_dm_crypt_keys/count
     2
 
-    # To support CPU/memory hot-plugging, reuse keys already saved to reserved
-    # memory
-    echo true > /sys/kernel/config/crash_dm_crypt_key/reuse
-
 2. Load the dump-capture kernel
 
 3. After the dump-capture kerne get booted, restore the keys to user keyring
    echo yes > /sys/kernel/crash_dm_crypt_keys/restore
 
+For CPU/memory hot-plugging, you can reuse keys already saved to reserved
+memory before reloading the kdump image,
+    echo true > /sys/kernel/config/crash_dm_crypt_keys/reuse
+
+Note if crash hotplug is supported, this API is totally unnecessary thus will
+be disabled automatically.
+
 Contact
 =======
 
diff --git a/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c
index bc70fbb79e0f..666c1f0c9e3b 100644
--- a/kernel/crash_dump_dm_crypt.c
+++ b/kernel/crash_dump_dm_crypt.c
@@ -324,7 +324,7 @@ static ssize_t config_keys_reuse_store(struct config_item *item,
 
 	r = -EINVAL;
 	if (!kexec_crash_image || !kexec_crash_image->dm_crypt_keys_addr) {
-		pr_debug("dm-crypt keys haven't be saved to crash-reserved memory\n");
+		pr_debug("dm-crypt keys haven't be saved to crash-reserved memory or crash hotplug supported\n");
 		goto unlock;
 	}
 
@@ -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) {
 		kfree_sensitive(keys_header);
 		keys_header = NULL;
 	}
+#ifdef CONFIG_CRASH_HOTPLUG
+	if (image->hotplug_support)
+		image->dm_crypt_keys_addr = 0;
+#endif
 }
 
 static int __init configfs_dmcrypt_keys_init(void)
-- 
2.55.0

[PATCH v5 9/9] Documentation: kdump: Add arm64 and ppc64le to encrypted dump target support list

[Coiby Xu]

The encrypted dump target support is now extended to arm64 and ppc64le.

Fixes: e3a84be1ec2f ("arm64,ppc64le/kdump: pass dm-crypt keys to kdump kernel")
Reported-by: Sourabh Jain <sourabhjain@linux.ibm.com>
Reviewed-by: Sourabh Jain <sourabhjain@linux.ibm.com>
Signed-off-by: Coiby Xu <coiby.xu@gmail.com>
---
 Documentation/admin-guide/kdump/kdump.rst | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/admin-guide/kdump/kdump.rst b/Documentation/admin-guide/kdump/kdump.rst
index 0bf2eb100a05..36ad6125c670 100644
--- a/Documentation/admin-guide/kdump/kdump.rst
+++ b/Documentation/admin-guide/kdump/kdump.rst
@@ -572,8 +572,8 @@ Write the dump file to encrypted disk volume
 ============================================
 
 CONFIG_CRASH_DM_CRYPT can be enabled to support saving the dump file to an
-encrypted disk volume (only x86_64 supported for now). User space can interact
-with /sys/kernel/config/crash_dm_crypt_keys for setup,
+encrypted disk volume (only x86_64, arm64, ppc64le supported for now). User
+space can interact with /sys/kernel/config/crash_dm_crypt_keys for setup,
 
 1. Tell the first kernel what logon keys are needed to unlock the disk volumes,
     # Add key #1
-- 
2.55.0

Re: [PATCH v5 9/9] Documentation: kdump: Add arm64 and ppc64le to encrypted dump target support list

[sashiko-bot]

> The encrypted dump target support is now extended to arm64 and ppc64le.
> 
> Fixes: e3a84be1ec2f ("arm64,ppc64le/kdump: pass dm-crypt keys to kdump kernel")
> Reported-by: Sourabh Jain <sourabhjain@linux.ibm.com>
> Reviewed-by: Sourabh Jain <sourabhjain@linux.ibm.com>
> Signed-off-by: Coiby Xu <coiby.xu@gmail.com>

Sashiko has reviewed this patch and found no issues. It looks great!

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

Re: [PATCH v5 6/9] crash_dump: Improve readability of config_keys_restore_store

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The local variable `r` is returned uninitialized when the user writes a value evaluating to false.
--

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

crash_dump: Improve readability of config_keys_restore_store

This commit introduces user input validation to config_keys_restore_store
before restoring dm-crypt keys. It ensures the restore function's return
code is verified and improves overall code readability.

> diff --git a/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c
> index 9ad50ac54866a..b8fe630462cfa 100644
> --- a/kernel/crash_dump_dm_crypt.c
> +++ b/kernel/crash_dump_dm_crypt.c
> @@ -371,13 +371,25 @@ static ssize_t config_keys_restore_show(struct config_item *item, char *page)
>  static ssize_t config_keys_restore_store(struct config_item *item,
>  					  const char *page, size_t count)
>  {
> -	if (!restore)
> -		restore_dm_crypt_keys_to_thread_keyring();
> +	bool val;
> +	int r;
>  
> -	if (kstrtobool(page, &restore))
> +	if (kstrtobool(page, &val))
>  		return -EINVAL;
>  
> -	return count;
> +	if (val) {
> +		if (restore) {
> +			pr_warn("dm-crypt keys already restored!\n");
> +			return count;
> +		}
> +		r = restore_dm_crypt_keys_to_thread_keyring();
> +		if (!r) {
> +			restore = true;
> +			r = count;
> +		}
> +	}
> +

[Severity: High]
If a user writes a value evaluating to false (e.g., '0' or 'n') to this
configfs attribute, kstrtobool() will succeed and set val to false.

This causes the if (val) block to be bypassed entirely, resulting in the
uninitialized variable r being returned here. Could this leak uninitialized
kernel stack memory to userspace or cause undefined behavior?

> +	return r;
>  }
>  
>  CONFIGFS_ATTR(config_keys_, restore);

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

Re: [PATCH v5 4/9] crash_dump: Free temporary dm-crypt keys_header buffer in kdump kernel

[sashiko-bot]

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

New issues:
- [Medium] Mixing goto-based cleanup labels with __free() scope-based cleanup in the same function.

Pre-existing issues:
- [High] Out-of-bounds memory read using kmap_local_page() for a multi-page buffer.
--

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

    crash_dump: Free temporary dm-crypt keys_header buffer in kdump kernel
    
    Although we expect the system to reboot immediately after vmcore dumping
    is finished, it's still good to free the temporary keys_header buffer.
    
    Fixes: 62f17d9df692 ("crash_dump: retrieve dm crypt keys in kdump kernel")
    Reported-by: Sourabh Jain <sourabhjain@linux.ibm.com>
    Reviewed-by: Sourabh Jain <sourabhjain@linux.ibm.com>
    Signed-off-by: Coiby Xu <coiby.xu@gmail.com>

> diff --git a/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c
> index 580534d43b66e..ff6055a97a4b6 100644
> --- a/kernel/crash_dump_dm_crypt.c
> +++ b/kernel/crash_dump_dm_crypt.c
> @@ -118,6 +118,7 @@ static int get_keys_from_kdump_reserved_memory(void)
>  
>  static int restore_dm_crypt_keys_to_thread_keyring(void)
>  {
> +	struct keys_header *keys_header __free(kfree_sensitive) = NULL;

[Severity: Medium]
Does this code mix __free() scope-based cleanup with goto cleanup labels?

The function restore_dm_crypt_keys_to_thread_keyring() newly introduces
a __free() declaration for keys_header, but still contains a goto out block
at the end:

kernel/crash_dump_dm_crypt.c:restore_dm_crypt_keys_to_thread_keyring() {
    ...
out:
	key_ref_put(keyring_ref);
	return ret;
}

Mixing these two cleanup styles creates confusing ownership semantics and
increases the risk of double-frees or resource leaks when the code is
modified in the future.

[Severity: High]
This isn't a bug introduced by this patch, but does the adjacent function
get_keys_from_kdump_reserved_memory() read out-of-bounds?

It uses kmap_local_page() which only maps a single 4KB page, but memcpy()
copies keys_header_size bytes (which can be up to ~49KB) from it:

kernel/crash_dump_dm_crypt.c:get_keys_from_kdump_reserved_memory() {
    ...
	keys_header_loaded = kmap_local_page(pfn_to_page(
		kexec_crash_image->dm_crypt_keys_addr >> PAGE_SHIFT));
    ...
	keys_header_size = get_keys_header_size(keys_header_loaded->total_keys);
    ...
	memcpy(keys_header, keys_header_loaded, keys_header_size);
    ...
}

On high memory systems (e.g., 32-bit architectures), this will trigger a
kernel page fault or read garbage data if the total size of the keys exceeds
what fits in a single page.

>  	struct dm_crypt_key *key;
>  	size_t keys_header_size;
>  	key_ref_t keyring_ref;

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

Re: [PATCH v5 3/9] crash_dump: Disallow writing to dm-crypt configfs during kexec_file_load syscall

[sashiko-bot]

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

New issues:
- [High] Severe locking imbalance in `crash_load_dm_crypt_keys()` leads to a leaked mutex on the error path and unlocking an unacquired mutex on the success path.

Pre-existing issues:
- [High] Premature and unconditional reset of `is_dm_key_reused` to false breaks ARM64/LoongArch kexec load retries and defeats intended cleanup logic.
--

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

crash_dump: Disallow writing to dm-crypt configfs during kexec_file_load syscall

This commit prevents concurrent writes to the configfs group during the
kexec_file_load syscall by utilizing config_keys_subsys.su_mutex. It aims to
avoid buffer overflows, stale keys, or memory leaks caused by changes to
dm-crypt keys during iteration.

> diff --git a/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c
> index 3a416933979f8..580534d43b66e 100644
> --- a/kernel/crash_dump_dm_crypt.c
> +++ b/kernel/crash_dump_dm_crypt.c

[ ... ]

> @@ -478,6 +490,9 @@ int crash_load_dm_crypt_keys(struct kimage *image)
>  		"Loaded dm crypt keys to kexec_buffer bufsz=0x%lx memsz=0x%lx\n",
>  		kbuf.bufsz, kbuf.memsz);
>  
> +unlock:
> +	mutex_unlock(&config_keys_subsys.su_mutex);
> +

[Severity: High]
Looking at crash_load_dm_crypt_keys(), if kexec_add_buffer() fails earlier
in the function, it jumps to the out label, bypassing this unlock label.
Does this leak the config_keys_subsys.su_mutex if the lock was acquired?

Also, on the success path, mutex_unlock() is executed unconditionally. If
is_dm_key_reused was true at the top of the function, the mutex was never
acquired. Can this cause a kernel panic or corruption by unlocking an
unacquired mutex?

>  out:
> -	return r;
> +	is_dm_key_reused = false;
> +	return r;
>  }

[Severity: High]
This is a pre-existing issue, but since the error and success paths
unconditionally reset is_dm_key_reused to false here, does this break
architectures like ARM64 that might retry segment loading?

If load_other_segments() retries the load sequence, the second pass
through crash_load_dm_crypt_keys() will see is_dm_key_reused as false,
find zero keys, and silently load nothing into the kdump kernel.

Additionally, crash_dm_crypt_cleanup() checks !is_dm_key_reused to determine
whether it should free keys_header. Since the flag is unconditionally cleared
here first, does this defeat the intended cleanup protection logic?

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

Re: [PATCH v5 5/9] crash_dump: Only use kexec_dprintk during the kexec_file_load syscall

[sashiko-bot]

> kexec_dprintk will only be activated by "kexec -d" during
> kexec_file_load syscall. So use pr_* outside of this syscall.
> 
> Fixes: 9ebfa8dcaea7 ("crash_dump: reuse saved dm crypt keys for CPU/memory hot-plugging")
> Fixes: 62f17d9df692 ("crash_dump: retrieve dm crypt keys in kdump kernel")
> Reviewed-by: Sourabh Jain <sourabhjain@linux.ibm.com>
> Signed-off-by: Coiby Xu <coiby.xu@gmail.com>

Sashiko has reviewed this patch and found no issues. It looks great!

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

Re: [PATCH v5 7/9] crash_dump: Check the function return codes in restore_dm_crypt_keys_to_thread_keyring

[sashiko-bot]

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

New issues:
- [Medium] Memory and reference leak of the key returned by key_create_or_update() on the success path.
- [Medium] Incorrect return value assignment from `dm_crypt_keys_read()` breaks the caller's success check for empty key sets.

Pre-existing issues:
- [High] Missing validation of `dm_crypt_keys_addr` leads to arbitrary memory reads.
- [High] Buffer over-read from single mapped page in get_keys_from_kdump_reserved_memory().
- [High] Missing validation on parsed key attributes leads to massive Heap Out-of-Bounds reads and Information Leak.
- [Medium] Double fetch of the key count from untrusted memory leads to Heap Out-of-Bounds read.
--

commit e9dcb18898d315e8ec7102932e9391f9c0892b97
Author: Coiby Xu <coiby.xu@gmail.com>
Subject: crash_dump: Check the function return codes in restore_dm_crypt_keys_to_thread_keyring

This commit adds return code checking for dm_crypt_keys_read() and
add_key_to_keyring() to properly abort when reading old memory or
allocating a keyring fails. It also deletes unused code related to
the key reference.

> diff --git a/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c
> index b8fe630462cfa..bc70fbb79e0f6 100644
> --- a/kernel/crash_dump_dm_crypt.c
> +++ b/kernel/crash_dump_dm_crypt.c
> @@ -66,7 +66,7 @@ static int add_key_to_keyring(struct dm_crypt_key *dm_key,
>  			      key_ref_t keyring_ref)
>  {
>  	key_ref_t key_ref;
> -	int r;
> +	int r = 0;
>  
>  	/* create or update the requested key and add it to the target keyring */
>  	key_ref = key_create_or_update(keyring_ref, "user", dm_key->key_desc,
> @@ -74,8 +74,6 @@ static int add_key_to_keyring(struct dm_crypt_key *dm_key,
>  				       KEY_USR_ALL, KEY_ALLOC_IN_QUOTA);
>  
>  	if (!IS_ERR(key_ref)) {
> -		r = key_ref_to_ptr(key_ref)->serial;
> -		key_ref_put(key_ref);
>  		pr_debug("Success adding key %s\n", dm_key->key_desc);

[Severity: Medium]
Does removing key_ref_put() here leak the key reference?

key_create_or_update() returns a key reference specifically for the caller
with an incremented usage count, so without key_ref_put(), the key's usage
count might be permanently elevated, preventing it from ever being garbage
collected.

>  	} else {
>  		r = PTR_ERR(key_ref);

[ ... ]

> @@ -134,9 +132,14 @@ static int restore_dm_crypt_keys_to_thread_keyring(void)
>  	}
>  
>  	addr = dm_crypt_keys_addr;
> -	dm_crypt_keys_read((char *)&key_count, sizeof(key_count), &addr);
> +	ret = dm_crypt_keys_read((char *)&key_count, sizeof(key_count), &addr);

[Severity: High]
This is a pre-existing issue, but is it safe to read from dm_crypt_keys_addr
without verifying if it was ever initialized by the command-line argument?

If the kdump kernel is booted without the dmcryptkeys= parameter,
dm_crypt_keys_addr might remain 0, leading to arbitrary memory reads
when this sysfs attribute is triggered.

> +	if (ret < 0) {
> +		pr_err("Failed to read the number of dm-crypt keys\n");
> +		goto out;
> +	}
> +

[ ... ]

> @@ -151,12 +154,18 @@ static int restore_dm_crypt_keys_to_thread_keyring(void)
>  		goto out;
>  	}
>  
> -	dm_crypt_keys_read((char *)keys_header, keys_header_size, &addr);
> +	ret = dm_crypt_keys_read((char *)keys_header, keys_header_size, &addr);

[Severity: Medium]
If dm_crypt_keys_read() succeeds, it returns a positive byte count which is
assigned to ret. If there are no keys and the loop below doesn't execute,
will this function incorrectly return a positive value instead of 0?

The caller config_keys_restore_store() expects !ret to set restore = true,
which would fail if ret is a positive byte count.

> +	if (ret < 0) {
> +		pr_err("Failed to read dm-crypt keys\n");
> +		goto out;
> +	}
>  
>  	for (int i = 0; i < keys_header->total_keys; i++) {

[Severity: Medium]
This is a pre-existing issue, but does the second read from oldmem physically
overwrite keys_header->total_keys with unvalidated data?

If oldmem changes concurrently or the read partially fails, the newly fetched
total_keys can exceed the originally validated key_count. The loop bounds
check uses this unvalidated bound, potentially causing a heap out-of-bounds
read.

>  		key = &keys_header->keys[i];
>  		pr_debug("Get key (size=%u)\n", key->key_size);
> -		add_key_to_keyring(key, keyring_ref);
> +		ret = add_key_to_keyring(key, keyring_ref);

[Severity: High]
This isn't a bug introduced by this patch, but are key->key_size and
key->key_desc validated before being passed to add_key_to_keyring()?

The completely untrusted key_size is passed as the payload length to
key_create_or_update(), which might read far out-of-bounds from dm_key->data
and embed leaked kernel heap memory into the key payload. Additionally,
key_desc is not checked for null-termination.

> +		if (ret)
> +			break;
>  	}

[ ... ]

While reviewing this file, I also noticed another pre-existing issue in
get_keys_from_kdump_reserved_memory():

kernel/crash_dump_dm_crypt.c:get_keys_from_kdump_reserved_memory() {
    ...
	keys_header_loaded = kmap_local_page(pfn_to_page(
		kexec_crash_image->dm_crypt_keys_addr >> PAGE_SHIFT));
    ...
	memcpy(keys_header, keys_header_loaded, keys_header_size);
    ...
}

[Severity: High]
This isn't a bug introduced by this patch, but does this memcpy() read
past the boundaries of the mapped page?

kmap_local_page() establishes a mapping for exactly one page (4096 bytes),
but keys_header_size could be up to ~49KB. This will read far beyond the
single page boundary, potentially causing a kernel panic on architectures
where kmap_local_page() does not fall back to the direct map.

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

Re: [PATCH v5 2/9] crash_dump: Read the number of dm-crypt keys from reserved memory

[sashiko-bot]

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

New issues:
- [High] Unconditionally clearing `is_dm_key_reused` defeats the key caching mechanism, silently breaking hotplug kdump reloads and ARM64 segment retries.

Pre-existing issues:
- [High] Out-of-bounds read past the 4KB window of `kmap_local_page` when reading dm-crypt keys from reserved memory.
--

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

crash_dump: Read the number of dm-crypt keys from reserved memory

This patch reads the number of dm-crypt keys from reserved memory to handle
cases where users add or delete keys by mistake. This aims to improve safety
over reusing saved keys for CPU/memory hot-plugging.

> diff --git a/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c
> index f638fb4b8554c..3a416933979f8 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);

[Severity: High]
This is a pre-existing issue, but does this memcpy() cause an out-of-bounds
read in get_keys_from_kdump_reserved_memory()?

The pointer keys_header_loaded is set up using kmap_local_page(), which
establishes a virtual mapping strictly limited to a single 4KB page.
However, keys_header_size can reach up to approximately 49.6KB (128 keys
at 388 bytes each).

Could this read past the kmap_local_page window if enough keys are populated
to exceed the 4KB boundary, causing a kernel page fault or copying unrelated
memory into the dm-crypt keys buffer?

> +kunmap:
>  	kunmap_local(keys_header_loaded);
>  	arch_kexec_protect_crashkres();
> -
> -	return 0;
> +	return r;
>  }

[ ... ]

> @@ -432,12 +442,13 @@ int crash_load_dm_crypt_keys(struct kimage *image)
>  	};
>  	int r = 0;
>  
> -	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");
> +			r = 0;
> +			goto out;
> +		}
> +

[Severity: High]
Does this goto out path in crash_load_dm_crypt_keys() unintentionally defeat
the key caching mechanism?

By jumping to the out label where is_dm_key_reused is set to false, it
appears the kexec cleanup function (crash_dm_crypt_cleanup) will now
unconditionally free keys_header since the flag is cleared.

On architectures like ARM64, if kexec segment allocation fails and retries,
the dm-crypt keys may be silently omitted from the new kdump image because
the retry loop will skip loading them (returning success but leaving out the
keys segment).

Could this break the intended caching mechanism for CPU and memory hotplug
events, leading to a silent failure to unlock the encrypted dump target
during a crash?

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

Re: [PATCH v5 1/9] crash_dump: Fix potential double free and UAF of keys_header

[sashiko-bot]

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

New issues:
- [High] Prematurely clearing `is_dm_key_reused` breaks the ARM64 kexec_file_load retry loop and defeats hotplug dm-crypt key reuse.

Pre-existing issues:
- [High] Mapping crash-reserved memory with `kmap_local_page()` causes out-of-bounds reads if dm-crypt keys exceed a single page.
--

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

crash_dump: Fix potential double free and UAF of keys_header

This commit addresses a potential double free and UAF of keys_header when
loading dm-crypt keys for kdump. It establishes an invariant to free
keys_header at the end of kexec_file_load unless reuse=true.

> diff --git a/arch/arm64/kernel/kexec_image.c b/arch/arm64/kernel/kexec_image.c
> index b70f4df15a1ae..1a0ca54adc9a4 100644
> --- a/arch/arm64/kernel/kexec_image.c
> +++ b/arch/arm64/kernel/kexec_image.c
> @@ -110,6 +110,7 @@ static void *image_load(struct kimage *image,
>  		image->nr_segments -= 1;
>  		kbuf.buf_min = kernel_segment->mem + kernel_segment->memsz;
>  		kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> +		crash_dm_crypt_cleanup(image);

[Severity: High]
Does this call to crash_dm_crypt_cleanup() during a retry loop unintentionally
free the keys_header buffer?

Since crash_load_dm_crypt_keys() unconditionally clears the is_dm_key_reused
flag before returning, any mid-retry cleanup performed here when segment
placement fails will see the flag as false and wrongfully free the valid
keys_header buffer.

>  	}
>  
>  	if (ret) {

[ ... ]

> diff --git a/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c
> index c685497cd470e..f638fb4b8554c 100644
> --- a/kernel/crash_dump_dm_crypt.c
> +++ b/kernel/crash_dump_dm_crypt.c
> @@ -84,18 +84,25 @@ static int add_key_to_keyring(struct dm_crypt_key *dm_key,

[ ... ]

> -static void get_keys_from_kdump_reserved_memory(void)
> +static int get_keys_from_kdump_reserved_memory(void)
>  {
>  	struct keys_header *keys_header_loaded;
> +	size_t keys_header_size;
>  
> -	arch_kexec_unprotect_crashkres();
> +	keys_header_size = get_keys_header_size(key_count);
> +	keys_header = kzalloc(keys_header_size, GFP_KERNEL);
> +	if (!keys_header)
> +		return -ENOMEM;
>  
> +	arch_kexec_unprotect_crashkres();
>  	keys_header_loaded = kmap_local_page(pfn_to_page(
>  		kexec_crash_image->dm_crypt_keys_addr >> PAGE_SHIFT));
>  
> -	memcpy(keys_header, keys_header_loaded, get_keys_header_size(key_count));
> +	memcpy(keys_header, keys_header_loaded, keys_header_size);

[Severity: High]
This is a pre-existing issue, but does this copy read out of bounds?

kmap_local_page() guarantees mapping exactly one physical page frame. If user
configuration has enough dm-crypt keys (e.g., > 10 keys, where each struct is
388 bytes), keys_header_size easily exceeds the 4KB page size.

Copying beyond the single-page mapping will cause out-of-bounds reads and
could trigger a kernel panic on systems with HIGHMEM or when
CONFIG_DEBUG_KMAP_LOCAL is enabled. Could memremap() be used instead?

>  	kunmap_local(keys_header_loaded);
>  	arch_kexec_protect_crashkres();
> +
> +	return 0;
>  }

[ ... ]

> @@ -441,18 +458,33 @@ int crash_load_dm_crypt_keys(struct kimage *image)
>  	r = kexec_add_buffer(&kbuf);
>  	if (r) {
>  		pr_err("Failed to call kexec_add_buffer, ret=%d\n", r);
> -		kvfree((void *)kbuf.buffer);
> -		return r;
> +		goto out;
>  	}
> +
>  	image->dm_crypt_keys_addr = kbuf.mem;
>  	image->dm_crypt_keys_sz = kbuf.bufsz;
>  	kexec_dprintk(
>  		"Loaded dm crypt keys to kexec_buffer bufsz=0x%lx memsz=0x%lx\n",
>  		kbuf.bufsz, kbuf.memsz);
>  
> +out:
> +	is_dm_key_reused = false;

[Severity: High]
Does prematurely clearing is_dm_key_reused here destroy the state needed for
the segment placement retry loop?

On ARM64, if the first attempt at segment placement fails, the kexec load
code retries the loop. Since the flag is unconditionally cleared here, the
first iteration will clear it. When the retry cleanup executes in
image_load(), it will see is_dm_key_reused == false, freeing the keys_header
buffer and leaving subsequent retries without the required keys.

>  	return r;
>  }

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

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

[sashiko-bot]

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

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

[Randy Dunlap]

On 9/8/26 5:36 PM, Coiby Xu wrote:
> 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.
> 
> Fixes: 9ebfa8dcaea7 ("crash_dump: reuse saved dm crypt keys for CPU/memory hot-plugging")
> Reviewed-by: Sourabh Jain <sourabhjain@linux.ibm.com>
> Signed-off-by: Coiby Xu <coiby.xu@gmail.com>
> ---
>  Documentation/admin-guide/kdump/kdump.rst | 16 ++++++++++------
>  kernel/crash_dump_dm_crypt.c              | 12 ++++++++----
>  2 files changed, 18 insertions(+), 10 deletions(-)
> 
> diff --git a/Documentation/admin-guide/kdump/kdump.rst b/Documentation/admin-guide/kdump/kdump.rst
> index 7587caadbae1..0bf2eb100a05 100644
> --- a/Documentation/admin-guide/kdump/kdump.rst
> +++ b/Documentation/admin-guide/kdump/kdump.rst
> @@ -577,9 +577,10 @@ with /sys/kernel/config/crash_dm_crypt_keys for setup,
>  
>  1. Tell the first kernel what logon keys are needed to unlock the disk volumes,
>      # Add key #1
> -    mkdir /sys/kernel/config/crash_dm_crypt_keys/7d26b7b4-e342-4d2d-b660-7426b0996720
> +    VOL1_UUID=7d26b7b4-e342-4d2d-b660-7426b0996720
> +    mkdir /sys/kernel/config/crash_dm_crypt_keys/$VOL1_UUID
>      # Add key #1's description
> -    echo cryptsetup:7d26b7b4-e342-4d2d-b660-7426b0996720 > /sys/kernel/config/crash_dm_crypt_keys/description
> +    echo cryptsetup:$VOL1_UUID > /sys/kernel/config/crash_dm_crypt_keys/$VOL1_UUID/description
>  
>      # how many keys do we have now?
>      cat /sys/kernel/config/crash_dm_crypt_keys/count
> @@ -591,15 +592,18 @@ with /sys/kernel/config/crash_dm_crypt_keys for setup,
>      cat /sys/kernel/config/crash_dm_crypt_keys/count
>      2
>  
> -    # To support CPU/memory hot-plugging, reuse keys already saved to reserved
> -    # memory
> -    echo true > /sys/kernel/config/crash_dm_crypt_key/reuse
> -
>  2. Load the dump-capture kernel
>  
>  3. After the dump-capture kerne get booted, restore the keys to user keyring

                             kernel is booted,

>     echo yes > /sys/kernel/crash_dm_crypt_keys/restore
>  
> +For CPU/memory hot-plugging, you can reuse keys already saved to reserved
> +memory before reloading the kdump image,
> +    echo true > /sys/kernel/config/crash_dm_crypt_keys/reuse
> +
> +Note if crash hotplug is supported, this API is totally unnecessary thus will
> +be disabled automatically.
> +
>  Contact
>  =======
>  
-- 
~Randy

Re: [PATCH v5 9/9] Documentation: kdump: Add arm64 and ppc64le to encrypted dump target support list

[Randy Dunlap]

On 9/8/26 5:36 PM, Coiby Xu wrote:
> The encrypted dump target support is now extended to arm64 and ppc64le.
> 
> Fixes: e3a84be1ec2f ("arm64,ppc64le/kdump: pass dm-crypt keys to kdump kernel")
> Reported-by: Sourabh Jain <sourabhjain@linux.ibm.com>
> Reviewed-by: Sourabh Jain <sourabhjain@linux.ibm.com>
> Signed-off-by: Coiby Xu <coiby.xu@gmail.com>
> ---
>  Documentation/admin-guide/kdump/kdump.rst | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/admin-guide/kdump/kdump.rst b/Documentation/admin-guide/kdump/kdump.rst
> index 0bf2eb100a05..36ad6125c670 100644
> --- a/Documentation/admin-guide/kdump/kdump.rst
> +++ b/Documentation/admin-guide/kdump/kdump.rst
> @@ -572,8 +572,8 @@ Write the dump file to encrypted disk volume
>  ============================================
>  
>  CONFIG_CRASH_DM_CRYPT can be enabled to support saving the dump file to an
> -encrypted disk volume (only x86_64 supported for now). User space can interact
> -with /sys/kernel/config/crash_dm_crypt_keys for setup,
> +encrypted disk volume (only x86_64, arm64, ppc64le supported for now). User
> +space can interact with /sys/kernel/config/crash_dm_crypt_keys for setup,

                                                                  for setup.

>  
>  1. Tell the first kernel what logon keys are needed to unlock the disk volumes,
>      # Add key #1

-- 
~Randy

Re: [PATCH v5 1/9] crash_dump: Fix potential double free and UAF of keys_header

[Coiby Xu]

On Wed, Sep 09, 2026 at 12:53:11AM +0000, sashiko-bot@kernel.org wrote:
>Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
>New issues:
>- [High] Prematurely clearing `is_dm_key_reused` breaks the ARM64 kexec_file_load retry loop and defeats hotplug dm-crypt key reuse.

Thanks for catching this issue! I'll send a new version to address this
issue.

>
>Pre-existing issues:
>- [High] Mapping crash-reserved memory with `kmap_local_page()` causes out-of-bounds reads if dm-crypt keys exceed a single page.

x86 is not supported so I think this concern can be dismissed.

>--
>
>commit 9cf0538566bf6406e97a7f29629856abff44a5b0
>Author: Coiby Xu <coiby.xu@gmail.com>
>
>crash_dump: Fix potential double free and UAF of keys_header
>
>This commit addresses a potential double free and UAF of keys_header when
>loading dm-crypt keys for kdump. It establishes an invariant to free
>keys_header at the end of kexec_file_load unless reuse=true.
>
>> diff --git a/arch/arm64/kernel/kexec_image.c b/arch/arm64/kernel/kexec_image.c
>> index b70f4df15a1ae..1a0ca54adc9a4 100644
>> --- a/arch/arm64/kernel/kexec_image.c
>> +++ b/arch/arm64/kernel/kexec_image.c
>> @@ -110,6 +110,7 @@ static void *image_load(struct kimage *image,
>>  		image->nr_segments -= 1;
>>  		kbuf.buf_min = kernel_segment->mem + kernel_segment->memsz;
>>  		kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
>> +		crash_dm_crypt_cleanup(image);
>
>[Severity: High]
>Does this call to crash_dm_crypt_cleanup() during a retry loop unintentionally
>free the keys_header buffer?
>
>Since crash_load_dm_crypt_keys() unconditionally clears the is_dm_key_reused
>flag before returning, any mid-retry cleanup performed here when segment
>placement fails will see the flag as false and wrongfully free the valid
>keys_header buffer.
>
>>  	}
>>
>>  	if (ret) {
>
>[ ... ]
>
>> diff --git a/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c
>> index c685497cd470e..f638fb4b8554c 100644
>> --- a/kernel/crash_dump_dm_crypt.c
>> +++ b/kernel/crash_dump_dm_crypt.c
>> @@ -84,18 +84,25 @@ static int add_key_to_keyring(struct dm_crypt_key *dm_key,
>
>[ ... ]
>
>> -static void get_keys_from_kdump_reserved_memory(void)
>> +static int get_keys_from_kdump_reserved_memory(void)
>>  {
>>  	struct keys_header *keys_header_loaded;
>> +	size_t keys_header_size;
>>
>> -	arch_kexec_unprotect_crashkres();
>> +	keys_header_size = get_keys_header_size(key_count);
>> +	keys_header = kzalloc(keys_header_size, GFP_KERNEL);
>> +	if (!keys_header)
>> +		return -ENOMEM;
>>
>> +	arch_kexec_unprotect_crashkres();
>>  	keys_header_loaded = kmap_local_page(pfn_to_page(
>>  		kexec_crash_image->dm_crypt_keys_addr >> PAGE_SHIFT));
>>
>> -	memcpy(keys_header, keys_header_loaded, get_keys_header_size(key_count));
>> +	memcpy(keys_header, keys_header_loaded, keys_header_size);
>
>[Severity: High]
>This is a pre-existing issue, but does this copy read out of bounds?
>
>kmap_local_page() guarantees mapping exactly one physical page frame. If user
>configuration has enough dm-crypt keys (e.g., > 10 keys, where each struct is
>388 bytes), keys_header_size easily exceeds the 4KB page size.
>
>Copying beyond the single-page mapping will cause out-of-bounds reads and
>could trigger a kernel panic on systems with HIGHMEM or when
>CONFIG_DEBUG_KMAP_LOCAL is enabled. Could memremap() be used instead?
>
>>  	kunmap_local(keys_header_loaded);
>>  	arch_kexec_protect_crashkres();
>> +
>> +	return 0;
>>  }
>
>[ ... ]
>
>> @@ -441,18 +458,33 @@ int crash_load_dm_crypt_keys(struct kimage *image)
>>  	r = kexec_add_buffer(&kbuf);
>>  	if (r) {
>>  		pr_err("Failed to call kexec_add_buffer, ret=%d\n", r);
>> -		kvfree((void *)kbuf.buffer);
>> -		return r;
>> +		goto out;
>>  	}
>> +
>>  	image->dm_crypt_keys_addr = kbuf.mem;
>>  	image->dm_crypt_keys_sz = kbuf.bufsz;
>>  	kexec_dprintk(
>>  		"Loaded dm crypt keys to kexec_buffer bufsz=0x%lx memsz=0x%lx\n",
>>  		kbuf.bufsz, kbuf.memsz);
>>
>> +out:
>> +	is_dm_key_reused = false;
>
>[Severity: High]
>Does prematurely clearing is_dm_key_reused here destroy the state needed for
>the segment placement retry loop?
>
>On ARM64, if the first attempt at segment placement fails, the kexec load
>code retries the loop. Since the flag is unconditionally cleared here, the
>first iteration will clear it. When the retry cleanup executes in
>image_load(), it will see is_dm_key_reused == false, freeing the keys_header
>buffer and leaving subsequent retries without the required keys.
>
>>  	return r;
>>  }
>
>-- 
>Sashiko AI review · https://sashiko.dev/#/patchset/20260909003657.1570544-1-coiby.xu@gmail.com?part=1

-- 
Best regards,
Coiby

Re: [PATCH v5 3/9] crash_dump: Disallow writing to dm-crypt configfs during kexec_file_load syscall

[Coiby Xu]

On Wed, Sep 09, 2026 at 12:47:21AM +0000, sashiko-bot@kernel.org wrote:
>Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
>New issues:
>- [High] Severe locking imbalance in `crash_load_dm_crypt_keys()` leads to a leaked mutex on the error path and unlocking an unacquired mutex on the success path.

Thanks for catching this issue! I'll address it in new version.

>
>Pre-existing issues:
>- [High] Premature and unconditional reset of `is_dm_key_reused` to false breaks ARM64/LoongArch kexec load retries and defeats intended cleanup logic.
>--
>
>commit a9405cf8948ba39213612c61a1f3b10b23f437e4
>Author: Coiby Xu <coiby.xu@gmail.com>
>
>crash_dump: Disallow writing to dm-crypt configfs during kexec_file_load syscall
>
>This commit prevents concurrent writes to the configfs group during the
>kexec_file_load syscall by utilizing config_keys_subsys.su_mutex. It aims to
>avoid buffer overflows, stale keys, or memory leaks caused by changes to
>dm-crypt keys during iteration.
>
>> diff --git a/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c
>> index 3a416933979f8..580534d43b66e 100644
>> --- a/kernel/crash_dump_dm_crypt.c
>> +++ b/kernel/crash_dump_dm_crypt.c
>
>[ ... ]
>
>> @@ -478,6 +490,9 @@ int crash_load_dm_crypt_keys(struct kimage *image)
>>  		"Loaded dm crypt keys to kexec_buffer bufsz=0x%lx memsz=0x%lx\n",
>>  		kbuf.bufsz, kbuf.memsz);
>>
>> +unlock:
>> +	mutex_unlock(&config_keys_subsys.su_mutex);
>> +
>
>[Severity: High]
>Looking at crash_load_dm_crypt_keys(), if kexec_add_buffer() fails earlier
>in the function, it jumps to the out label, bypassing this unlock label.
>Does this leak the config_keys_subsys.su_mutex if the lock was acquired?
>
>Also, on the success path, mutex_unlock() is executed unconditionally. If
>is_dm_key_reused was true at the top of the function, the mutex was never
>acquired. Can this cause a kernel panic or corruption by unlocking an
>unacquired mutex?
>
>>  out:
>> -	return r;
>> +	is_dm_key_reused = false;
>> +	return r;
>>  }
>
>[Severity: High]
>This is a pre-existing issue, but since the error and success paths
>unconditionally reset is_dm_key_reused to false here, does this break
>architectures like ARM64 that might retry segment loading?
>
>If load_other_segments() retries the load sequence, the second pass
>through crash_load_dm_crypt_keys() will see is_dm_key_reused as false,
>find zero keys, and silently load nothing into the kdump kernel.
>
>Additionally, crash_dm_crypt_cleanup() checks !is_dm_key_reused to determine
>whether it should free keys_header. Since the flag is unconditionally cleared
>here first, does this defeat the intended cleanup protection logic?
>
>-- 
>Sashiko AI review · https://sashiko.dev/#/patchset/20260909003657.1570544-1-coiby.xu@gmail.com?part=3

-- 
Best regards,
Coiby

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

[Coiby Xu]

On Tue, Sep 08, 2026 at 10:46:03PM -0700, Randy Dunlap wrote:
>
>
>On 9/8/26 5:36 PM, Coiby Xu wrote:
>> 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.
>>
>> Fixes: 9ebfa8dcaea7 ("crash_dump: reuse saved dm crypt keys for CPU/memory hot-plugging")
>> Reviewed-by: Sourabh Jain <sourabhjain@linux.ibm.com>
>> Signed-off-by: Coiby Xu <coiby.xu@gmail.com>
>> ---
>>  Documentation/admin-guide/kdump/kdump.rst | 16 ++++++++++------
>>  kernel/crash_dump_dm_crypt.c              | 12 ++++++++----
>>  2 files changed, 18 insertions(+), 10 deletions(-)
>>
>> diff --git a/Documentation/admin-guide/kdump/kdump.rst b/Documentation/admin-guide/kdump/kdump.rst
>> index 7587caadbae1..0bf2eb100a05 100644
>> --- a/Documentation/admin-guide/kdump/kdump.rst
>> +++ b/Documentation/admin-guide/kdump/kdump.rst
>> @@ -577,9 +577,10 @@ with /sys/kernel/config/crash_dm_crypt_keys for setup,
>>
>>  1. Tell the first kernel what logon keys are needed to unlock the disk volumes,
>>      # Add key #1
>> -    mkdir /sys/kernel/config/crash_dm_crypt_keys/7d26b7b4-e342-4d2d-b660-7426b0996720
>> +    VOL1_UUID=7d26b7b4-e342-4d2d-b660-7426b0996720
>> +    mkdir /sys/kernel/config/crash_dm_crypt_keys/$VOL1_UUID
>>      # Add key #1's description
>> -    echo cryptsetup:7d26b7b4-e342-4d2d-b660-7426b0996720 > /sys/kernel/config/crash_dm_crypt_keys/description
>> +    echo cryptsetup:$VOL1_UUID > /sys/kernel/config/crash_dm_crypt_keys/$VOL1_UUID/description
>>
>>      # how many keys do we have now?
>>      cat /sys/kernel/config/crash_dm_crypt_keys/count
>> @@ -591,15 +592,18 @@ with /sys/kernel/config/crash_dm_crypt_keys for setup,
>>      cat /sys/kernel/config/crash_dm_crypt_keys/count
>>      2
>>
>> -    # To support CPU/memory hot-plugging, reuse keys already saved to reserved
>> -    # memory
>> -    echo true > /sys/kernel/config/crash_dm_crypt_key/reuse
>> -
>>  2. Load the dump-capture kernel
>>
>>  3. After the dump-capture kerne get booted, restore the keys to user keyring
>
>                             kernel is booted,

Thanks for catching this typo! I'll apply your suggestion to next
version!

[...]

-- 
Best regards,
Coiby

Re: [PATCH v5 9/9] Documentation: kdump: Add arm64 and ppc64le to encrypted dump target support list

[Coiby Xu]

On Tue, Sep 08, 2026 at 10:48:11PM -0700, Randy Dunlap wrote:
>
>
>On 9/8/26 5:36 PM, Coiby Xu wrote:
>> The encrypted dump target support is now extended to arm64 and ppc64le.
>>
>> Fixes: e3a84be1ec2f ("arm64,ppc64le/kdump: pass dm-crypt keys to kdump kernel")
>> Reported-by: Sourabh Jain <sourabhjain@linux.ibm.com>
>> Reviewed-by: Sourabh Jain <sourabhjain@linux.ibm.com>
>> Signed-off-by: Coiby Xu <coiby.xu@gmail.com>
>> ---
>>  Documentation/admin-guide/kdump/kdump.rst | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/admin-guide/kdump/kdump.rst b/Documentation/admin-guide/kdump/kdump.rst
>> index 0bf2eb100a05..36ad6125c670 100644
>> --- a/Documentation/admin-guide/kdump/kdump.rst
>> +++ b/Documentation/admin-guide/kdump/kdump.rst
>> @@ -572,8 +572,8 @@ Write the dump file to encrypted disk volume
>>  ============================================
>>
>>  CONFIG_CRASH_DM_CRYPT can be enabled to support saving the dump file to an
>> -encrypted disk volume (only x86_64 supported for now). User space can interact
>> -with /sys/kernel/config/crash_dm_crypt_keys for setup,
>> +encrypted disk volume (only x86_64, arm64, ppc64le supported for now). User
>> +space can interact with /sys/kernel/config/crash_dm_crypt_keys for setup,
>
>                                                                  for setup.

I'll apply the suggestion, thanks!

[...]

-- 
Best regards,
Coiby