public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/9] crash_dump: Release reference to a keyring at correct time
       [not found] <20260501234342.2518281-1-coiby.xu@gmail.com>
@ 2026-05-01 23:43 ` Coiby Xu
  2026-05-01 23:43 ` [PATCH v2 2/9] crash_dump: Fix potential double free and UAF of keys_header Coiby Xu
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Coiby Xu @ 2026-05-01 23:43 UTC (permalink / raw)
  To: kexec
  Cc: Andrew Morton, Sourabh Jain, Baoquan He, Dave Young,
	Mike Rapoport, Pasha Tatashin, Pratyush Yadav, Coiby Xu,
	open list

It's incorrect to drop the reference only after adding one key to the
specified keyring. If there are many keys to be added, it can lead
"refcount_t: underflow; use-after-free" error and some keys will fail to
be added to the keyring. My testing shows the error can occur when there
are more than five keys.

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

diff --git a/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c
index cb875ddb6ba6..eac4f436a8d4 100644
--- a/kernel/crash_dump_dm_crypt.c
+++ b/kernel/crash_dump_dm_crypt.c
@@ -81,7 +81,6 @@ static int add_key_to_keyring(struct dm_crypt_key *dm_key,
 		kexec_dprintk("Error when adding key");
 	}
 
-	key_ref_put(keyring_ref);
 	return r;
 }
 
@@ -126,8 +125,10 @@ static int restore_dm_crypt_keys_to_thread_keyring(void)
 
 	keys_header_size = get_keys_header_size(key_count);
 	keys_header = kzalloc(keys_header_size, GFP_KERNEL);
-	if (!keys_header)
+	if (!keys_header) {
+		key_ref_put(keyring_ref);
 		return -ENOMEM;
+	}
 
 	dm_crypt_keys_read((char *)keys_header, keys_header_size, &addr);
 
@@ -137,6 +138,7 @@ static int restore_dm_crypt_keys_to_thread_keyring(void)
 		add_key_to_keyring(key, keyring_ref);
 	}
 
+	key_ref_put(keyring_ref);
 	return 0;
 }
 
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 2/9] crash_dump: Fix potential double free and UAF of keys_header
       [not found] <20260501234342.2518281-1-coiby.xu@gmail.com>
  2026-05-01 23:43 ` [PATCH v2 1/9] crash_dump: Release reference to a keyring at correct time Coiby Xu
@ 2026-05-01 23:43 ` Coiby Xu
  2026-05-06 12:28   ` Sourabh Jain
  2026-05-01 23:43 ` [PATCH v2 3/9] crash_dump: Disallow writing to dm-crypt configfs during kexec_file_load syscall Coiby Xu
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 14+ messages in thread
From: Coiby Xu @ 2026-05-01 23:43 UTC (permalink / raw)
  To: kexec
  Cc: Andrew Morton, Sourabh Jain, Baoquan He, Dave Young,
	Mike Rapoport, Pasha Tatashin, Pratyush Yadav, Coiby Xu,
	open list

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 kexec_file_load syscall 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

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>
Signed-off-by: Coiby Xu <coiby.xu@gmail.com>
---
 include/linux/kexec.h        |  6 ++++
 kernel/crash_dump_dm_crypt.c | 65 ++++++++++++++++++++++++++----------
 kernel/kexec_file.c          |  2 ++
 3 files changed, 56 insertions(+), 17 deletions(-)

diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index 8a22bc9b8c6c..91256d7ff434 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 kexec_file_post_load_cleanup_dm_crypt(struct kimage *image);
+#else
+static inline void kexec_file_post_load_cleanup_dm_crypt(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 eac4f436a8d4..4d8a3331bbe7 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)
@@ -283,17 +290,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;
 }
@@ -366,9 +384,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;
@@ -412,7 +427,7 @@ int crash_load_dm_crypt_keys(struct kimage *image)
 		.top_down = false,
 		.random = true,
 	};
-	int r;
+	int r = 0;
 
 
 	if (key_count <= 0) {
@@ -421,14 +436,15 @@ 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 at the end of kexec_file_load syscall
+	 */
 	kbuf.buffer = keys_header;
 	kbuf.bufsz = get_keys_header_size(key_count);
 
@@ -438,18 +454,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 kexec_file_post_load_cleanup_dm_crypt(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 2bfbb2d144e6..0421f1e89791 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -139,6 +139,8 @@ void kimage_file_post_load_cleanup(struct kimage *image)
 	kfree(image->image_loader_data);
 	image->image_loader_data = NULL;
 
+	kexec_file_post_load_cleanup_dm_crypt(image);
+
 	kexec_file_dbg_print = false;
 }
 
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 3/9] crash_dump: Disallow writing to dm-crypt configfs during kexec_file_load syscall
       [not found] <20260501234342.2518281-1-coiby.xu@gmail.com>
  2026-05-01 23:43 ` [PATCH v2 1/9] crash_dump: Release reference to a keyring at correct time Coiby Xu
  2026-05-01 23:43 ` [PATCH v2 2/9] crash_dump: Fix potential double free and UAF of keys_header Coiby Xu
@ 2026-05-01 23:43 ` Coiby Xu
  2026-05-06 13:56   ` Sourabh Jain
  2026-05-01 23:43 ` [PATCH v2 4/9] crash_dump: Read the number of dm-crypt keys from reserved memory Coiby Xu
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 14+ messages in thread
From: Coiby Xu @ 2026-05-01 23:43 UTC (permalink / raw)
  To: kexec
  Cc: Andrew Morton, Sourabh Jain, Baoquan He, Dave Young,
	Mike Rapoport, Pasha Tatashin, Pratyush Yadav, Coiby Xu,
	open list

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.

Fixes: 479e58549b0f ("crash_dump: store dm crypt keys in kdump reserved memory")
Signed-off-by: Coiby Xu <coiby.xu@gmail.com>
---
 kernel/crash_dump_dm_crypt.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c
index 4d8a3331bbe7..6377ee86ec50 100644
--- a/kernel/crash_dump_dm_crypt.c
+++ b/kernel/crash_dump_dm_crypt.c
@@ -429,6 +429,7 @@ int crash_load_dm_crypt_keys(struct kimage *image)
 	};
 	int r = 0;
 
+	mutex_lock(&config_keys_subsys.su_mutex);
 
 	if (key_count <= 0) {
 		kexec_dprintk("No dm-crypt keys\n");
@@ -479,6 +480,9 @@ void kexec_file_post_load_cleanup_dm_crypt(struct kimage *image)
 		kfree_sensitive(keys_header);
 		keys_header = NULL;
 	}
+
+	if (mutex_is_locked(&config_keys_subsys.su_mutex))
+		mutex_unlock(&config_keys_subsys.su_mutex);
 }
 
 static int __init configfs_dmcrypt_keys_init(void)
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 4/9] crash_dump: Read the number of dm-crypt keys from reserved memory
       [not found] <20260501234342.2518281-1-coiby.xu@gmail.com>
                   ` (2 preceding siblings ...)
  2026-05-01 23:43 ` [PATCH v2 3/9] crash_dump: Disallow writing to dm-crypt configfs during kexec_file_load syscall Coiby Xu
@ 2026-05-01 23:43 ` Coiby Xu
  2026-05-06 14:18   ` Sourabh Jain
  2026-05-01 23:43 ` [PATCH v2 5/9] crash_dump: Free temporary dm-crypt keys_header buffer in kdump kernel Coiby Xu
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 14+ messages in thread
From: Coiby Xu @ 2026-05-01 23:43 UTC (permalink / raw)
  To: kexec
  Cc: Andrew Morton, Sourabh Jain, Baoquan He, Dave Young,
	Mike Rapoport, Pasha Tatashin, Pratyush Yadav, Coiby Xu,
	open list

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 | 36 +++++++++++++++++++++++-------------
 1 file changed, 23 insertions(+), 13 deletions(-)

diff --git a/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c
index 6377ee86ec50..a3e460714d23 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)
@@ -431,12 +441,12 @@ int crash_load_dm_crypt_keys(struct kimage *image)
 
 	mutex_lock(&config_keys_subsys.su_mutex);
 
-	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");
+			return 0;
+		}
+
 		r = build_keys_header();
 		if (r)
 			goto out;
@@ -447,7 +457,7 @@ int crash_load_dm_crypt_keys(struct kimage *image)
 	 * cleaned up at the end of kexec_file_load syscall
 	 */
 	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.54.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 5/9] crash_dump: Free temporary dm-crypt keys_header buffer in kdump kernel
       [not found] <20260501234342.2518281-1-coiby.xu@gmail.com>
                   ` (3 preceding siblings ...)
  2026-05-01 23:43 ` [PATCH v2 4/9] crash_dump: Read the number of dm-crypt keys from reserved memory Coiby Xu
@ 2026-05-01 23:43 ` Coiby Xu
  2026-05-01 23:43 ` [PATCH v2 6/9] crash_dump: Only use kexec_dprintk during the kexec_file_load syscall Coiby Xu
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Coiby Xu @ 2026-05-01 23:43 UTC (permalink / raw)
  To: kexec
  Cc: Andrew Morton, Sourabh Jain, Baoquan He, Dave Young,
	Mike Rapoport, Pasha Tatashin, Pratyush Yadav, Coiby Xu,
	open list

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>
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 a3e460714d23..72ae2dffb077 100644
--- a/kernel/crash_dump_dm_crypt.c
+++ b/kernel/crash_dump_dm_crypt.c
@@ -117,6 +117,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.54.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 6/9] crash_dump: Only use kexec_dprintk during the kexec_file_load syscall
       [not found] <20260501234342.2518281-1-coiby.xu@gmail.com>
                   ` (4 preceding siblings ...)
  2026-05-01 23:43 ` [PATCH v2 5/9] crash_dump: Free temporary dm-crypt keys_header buffer in kdump kernel Coiby Xu
@ 2026-05-01 23:43 ` Coiby Xu
  2026-05-01 23:43 ` [PATCH v2 7/9] crash_dump: Improve readability of config_keys_restore_store Coiby Xu
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Coiby Xu @ 2026-05-01 23:43 UTC (permalink / raw)
  To: kexec
  Cc: Andrew Morton, Sourabh Jain, Baoquan He, Dave Young,
	Mike Rapoport, Pasha Tatashin, Pratyush Yadav, Coiby Xu,
	open list

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")
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 72ae2dffb077..545d712428d3 100644
--- a/kernel/crash_dump_dm_crypt.c
+++ b/kernel/crash_dump_dm_crypt.c
@@ -75,10 +75,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", dm_key->key_desc);
 	} else {
 		r = PTR_ERR(key_ref);
-		kexec_dprintk("Error when adding key");
+		pr_warn("Error when adding key");
 	}
 
 	return r;
@@ -127,18 +127,18 @@ 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");
 		return -1;
 	}
 
-	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);
@@ -152,7 +152,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);
 	}
 
@@ -305,8 +305,7 @@ static ssize_t config_keys_reuse_store(struct config_item *item,
 	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");
+		pr_info("dm-crypt keys haven't be saved to crash-reserved memory\n");
 		return -EINVAL;
 	}
 
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 7/9] crash_dump: Improve readability of config_keys_restore_store
       [not found] <20260501234342.2518281-1-coiby.xu@gmail.com>
                   ` (5 preceding siblings ...)
  2026-05-01 23:43 ` [PATCH v2 6/9] crash_dump: Only use kexec_dprintk during the kexec_file_load syscall Coiby Xu
@ 2026-05-01 23:43 ` Coiby Xu
  2026-05-06 14:33   ` Sourabh Jain
  2026-05-01 23:43 ` [PATCH v2 8/9] crash_dump: Disallow configfs/crash_dm_crypt_key/reuse if CONFIG_CRASH_HOTPLUG enabled Coiby Xu
  2026-05-01 23:43 ` [PATCH v2 9/9] Documentation: kdump: Add arm64 and ppc64le to encrypted dump target support list Coiby Xu
  8 siblings, 1 reply; 14+ messages in thread
From: Coiby Xu @ 2026-05-01 23:43 UTC (permalink / raw)
  To: kexec
  Cc: Andrew Morton, Sourabh Jain, Baoquan He, Dave Young,
	Mike Rapoport, Pasha Tatashin, Pratyush Yadav, Coiby Xu,
	open list

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")
Signed-off-by: Coiby Xu <coiby.xu@gmail.com>
---
 kernel/crash_dump_dm_crypt.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c
index 545d712428d3..36e51807d94f 100644
--- a/kernel/crash_dump_dm_crypt.c
+++ b/kernel/crash_dump_dm_crypt.c
@@ -358,12 +358,16 @@ 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;
 
-	if (kstrtobool(page, &restore))
+	if (kstrtobool(page, &val))
 		return -EINVAL;
 
+	if (val && !restore) {
+		if (!restore_dm_crypt_keys_to_thread_keyring())
+			restore = true;
+	}
+
 	return count;
 }
 
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 8/9] crash_dump: Disallow configfs/crash_dm_crypt_key/reuse if CONFIG_CRASH_HOTPLUG enabled
       [not found] <20260501234342.2518281-1-coiby.xu@gmail.com>
                   ` (6 preceding siblings ...)
  2026-05-01 23:43 ` [PATCH v2 7/9] crash_dump: Improve readability of config_keys_restore_store Coiby Xu
@ 2026-05-01 23:43 ` Coiby Xu
  2026-05-06 16:09   ` Sourabh Jain
  2026-05-01 23:43 ` [PATCH v2 9/9] Documentation: kdump: Add arm64 and ppc64le to encrypted dump target support list Coiby Xu
  8 siblings, 1 reply; 14+ messages in thread
From: Coiby Xu @ 2026-05-01 23:43 UTC (permalink / raw)
  To: kexec
  Cc: Andrew Morton, Sourabh Jain, Baoquan He, Dave Young,
	Mike Rapoport, Pasha Tatashin, Pratyush Yadav, Jonathan Corbet,
	Shuah Khan, Coiby Xu, open list:DOCUMENTATION, open list

If CONFIG_CRASH_HOTPLUG is enabled, dm-crypt keys saved to reserved
memory will be took care of automatically. Thus it doesn't make sense
to use configfs/crash_dm_crypt_key/reuse. Reserving
image->dm_crypt_keys_addr is also unnecessary. Currently x86_64 and
ppc64le have implemented CONFIG_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")
Signed-off-by: Coiby Xu <coiby.xu@gmail.com>
---
 Documentation/admin-guide/kdump/kdump.rst |  9 ++++++---
 kernel/crash_dump_dm_crypt.c              | 14 +++++++++++---
 2 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/Documentation/admin-guide/kdump/kdump.rst b/Documentation/admin-guide/kdump/kdump.rst
index 7587caadbae1..73f2e9500c60 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
@@ -593,7 +594,9 @@ with /sys/kernel/config/crash_dm_crypt_keys for setup,
 
     # To support CPU/memory hot-plugging, reuse keys already saved to reserved
     # memory
-    echo true > /sys/kernel/config/crash_dm_crypt_key/reuse
+    # Note if CONFIG_CRASH_HOTPLUG is enabled, this API is totally unnecessary
+    # thus will be disabled.
+    echo true > /sys/kernel/config/crash_dm_crypt_keys/reuse
 
 2. Load the dump-capture kernel
 
diff --git a/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c
index 36e51807d94f..7a7cae17f578 100644
--- a/kernel/crash_dump_dm_crypt.c
+++ b/kernel/crash_dump_dm_crypt.c
@@ -304,6 +304,11 @@ static ssize_t config_keys_reuse_store(struct config_item *item,
 	bool val;
 	int r;
 
+	if (IS_ENABLED(CONFIG_CRASH_HOTPLUG)) {
+		pr_info("CONFIG_CRASH_HOTPLUG already enabled");
+		return -EINVAL;
+	}
+
 	if (!kexec_crash_image || !kexec_crash_image->dm_crypt_keys_addr) {
 		pr_info("dm-crypt keys haven't be saved to crash-reserved memory\n");
 		return -EINVAL;
@@ -486,15 +491,18 @@ int crash_load_dm_crypt_keys(struct kimage *image)
 void kexec_file_post_load_cleanup_dm_crypt(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;
 	}
 
+	if (IS_ENABLED(CONFIG_CRASH_HOTPLUG))
+		image->dm_crypt_keys_addr = 0;
+
 	if (mutex_is_locked(&config_keys_subsys.su_mutex))
 		mutex_unlock(&config_keys_subsys.su_mutex);
 }
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 9/9] Documentation: kdump: Add arm64 and ppc64le to encrypted dump target support list
       [not found] <20260501234342.2518281-1-coiby.xu@gmail.com>
                   ` (7 preceding siblings ...)
  2026-05-01 23:43 ` [PATCH v2 8/9] crash_dump: Disallow configfs/crash_dm_crypt_key/reuse if CONFIG_CRASH_HOTPLUG enabled Coiby Xu
@ 2026-05-01 23:43 ` Coiby Xu
  8 siblings, 0 replies; 14+ messages in thread
From: Coiby Xu @ 2026-05-01 23:43 UTC (permalink / raw)
  To: kexec
  Cc: Andrew Morton, Sourabh Jain, Baoquan He, Dave Young,
	Mike Rapoport, Pasha Tatashin, Pratyush Yadav, Jonathan Corbet,
	Shuah Khan, Coiby Xu, Rob Herring (Arm), open list:DOCUMENTATION,
	open list

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>
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 73f2e9500c60..8708ef394212 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.54.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 2/9] crash_dump: Fix potential double free and UAF of keys_header
  2026-05-01 23:43 ` [PATCH v2 2/9] crash_dump: Fix potential double free and UAF of keys_header Coiby Xu
@ 2026-05-06 12:28   ` Sourabh Jain
  0 siblings, 0 replies; 14+ messages in thread
From: Sourabh Jain @ 2026-05-06 12:28 UTC (permalink / raw)
  To: Coiby Xu, kexec
  Cc: Andrew Morton, Baoquan He, Dave Young, Mike Rapoport,
	Pasha Tatashin, Pratyush Yadav, Coiby Xu, open list

Hello Coiby,

On 02/05/26 05:13, Coiby Xu wrote:
> 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 kexec_file_load syscall 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
>
> 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>
> Signed-off-by: Coiby Xu <coiby.xu@gmail.com>
> ---
>   include/linux/kexec.h        |  6 ++++
>   kernel/crash_dump_dm_crypt.c | 65 ++++++++++++++++++++++++++----------
>   kernel/kexec_file.c          |  2 ++
>   3 files changed, 56 insertions(+), 17 deletions(-)
>
> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> index 8a22bc9b8c6c..91256d7ff434 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 kexec_file_post_load_cleanup_dm_crypt(struct kimage *image);
> +#else
> +static inline void kexec_file_post_load_cleanup_dm_crypt(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 eac4f436a8d4..4d8a3331bbe7 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));

Accessing kexec_crash_image without holding the kexec lock could lead to
undefined behavior.

I think this section of code should be called by holding kexec lock.


>   
> -	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)
> @@ -283,17 +290,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) {

The above check should be performed after acquiring the kexec lock.



>   		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;

Why can’t we allow the user to set is_dm_key_reused = false and free the
key_header? That way, the next kdump kernel load will recreate the 
key_header.

For example, a user may add more keys and want the new keys to be included
in the kdump image from next kdump kernel load.


>   
> -	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;
>   }
> @@ -366,9 +384,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;
> @@ -412,7 +427,7 @@ int crash_load_dm_crypt_keys(struct kimage *image)
>   		.top_down = false,
>   		.random = true,
>   	};
> -	int r;
> +	int r = 0;
>   
>   
>   	if (key_count <= 0) {
> @@ -421,14 +436,15 @@ 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 at the end of kexec_file_load syscall
> +	 */
>   	kbuf.buffer = keys_header;
>   	kbuf.bufsz = get_keys_header_size(key_count);
>   
> @@ -438,18 +454,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 kexec_file_post_load_cleanup_dm_crypt(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;

Since crash_load_dm_crypt_keys() sets is_dm_key_reused = false, 
keys_header will
always be released here, right? Then why is the above free under an if 
condition?

IIUC, for the case where CONFIG_CRASH_HOTPLUG is not enabled, this is 
how key
restore works:

After loading the kdump kernel for the first time, the state of the 
variables is:

is_dm_key_reused = false
keys_header = NULL

For example, if 2 CPUs are hot-removed and kdump is reloaded twice:

Then the sequence of operations needed to ensure the loaded keys can be 
reused is:

Udev rule triggered on the 1st CPU hotplug:
echo true > /sys/kernel/config/crash_dm_crypt_keys/reuse
Restore the key header from the reserved area
Reload the kdump service/kernel

Udev rule triggered on the 2nd CPU hotplug:
echo true > /sys/kernel/config/crash_dm_crypt_keys/reuse
Restore the key header from the reserved area
Reload the kdump service/kernel

What I don’t understand is the need to restore the key header from 
crashkernel
memory for every hotplug operation.


- Sourabh Jain

> +	}
> +}
> +
>   static int __init configfs_dmcrypt_keys_init(void)
>   {
>   	int ret;
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> index 2bfbb2d144e6..0421f1e89791 100644
> --- a/kernel/kexec_file.c
> +++ b/kernel/kexec_file.c
> @@ -139,6 +139,8 @@ void kimage_file_post_load_cleanup(struct kimage *image)
>   	kfree(image->image_loader_data);
>   	image->image_loader_data = NULL;
>   
> +	kexec_file_post_load_cleanup_dm_crypt(image);
> +
>   	kexec_file_dbg_print = false;
>   }
>   


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 3/9] crash_dump: Disallow writing to dm-crypt configfs during kexec_file_load syscall
  2026-05-01 23:43 ` [PATCH v2 3/9] crash_dump: Disallow writing to dm-crypt configfs during kexec_file_load syscall Coiby Xu
@ 2026-05-06 13:56   ` Sourabh Jain
  0 siblings, 0 replies; 14+ messages in thread
From: Sourabh Jain @ 2026-05-06 13:56 UTC (permalink / raw)
  To: Coiby Xu, kexec
  Cc: Andrew Morton, Baoquan He, Dave Young, Mike Rapoport,
	Pasha Tatashin, Pratyush Yadav, Coiby Xu, open list



On 02/05/26 05:13, Coiby Xu wrote:
> 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.


Yes, this will solve many synchronization problems when a  user tries to
perform any operation under our configfs_subsystem while the kernel is
executing the kexec_file_load system call.

Now, regarding the third point about freeing key_header: this will work
only if configfs takes the su_mutex lock before invoking the store callback.
I am not sure whether it actually does.

However, based on my previous comment on (2/9), if we keep 
config_keys_reuse_store()
under the kexec lock, this will be taken care of. This is because the entire
kexec_file_load system call already runs under the kexec lock.


>
> Fixes: 479e58549b0f ("crash_dump: store dm crypt keys in kdump reserved memory")
> Signed-off-by: Coiby Xu <coiby.xu@gmail.com>
> ---
>   kernel/crash_dump_dm_crypt.c | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c
> index 4d8a3331bbe7..6377ee86ec50 100644
> --- a/kernel/crash_dump_dm_crypt.c
> +++ b/kernel/crash_dump_dm_crypt.c
> @@ -429,6 +429,7 @@ int crash_load_dm_crypt_keys(struct kimage *image)
>   	};
>   	int r = 0;
>   
> +	mutex_lock(&config_keys_subsys.su_mutex);
>   
>   	if (key_count <= 0) {
>   		kexec_dprintk("No dm-crypt keys\n");
> @@ -479,6 +480,9 @@ void kexec_file_post_load_cleanup_dm_crypt(struct kimage *image)
>   		kfree_sensitive(keys_header);
>   		keys_header = NULL;
>   	}
> +
> +	if (mutex_is_locked(&config_keys_subsys.su_mutex))
> +		mutex_unlock(&config_keys_subsys.su_mutex);
How about release the lock in crash_load_dm_crypt_keys() itself? Given 
that config_keys_reuse_store() is placed under kexec lock?


- Sourabh Jain

>   }
>   
>   static int __init configfs_dmcrypt_keys_init(void)


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 4/9] crash_dump: Read the number of dm-crypt keys from reserved memory
  2026-05-01 23:43 ` [PATCH v2 4/9] crash_dump: Read the number of dm-crypt keys from reserved memory Coiby Xu
@ 2026-05-06 14:18   ` Sourabh Jain
  0 siblings, 0 replies; 14+ messages in thread
From: Sourabh Jain @ 2026-05-06 14:18 UTC (permalink / raw)
  To: Coiby Xu, kexec
  Cc: Andrew Morton, Baoquan He, Dave Young, Mike Rapoport,
	Pasha Tatashin, Pratyush Yadav, Coiby Xu, open list



On 02/05/26 05:13, Coiby Xu wrote:
> 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 | 36 +++++++++++++++++++++++-------------
>   1 file changed, 23 insertions(+), 13 deletions(-)
>
> diff --git a/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c
> index 6377ee86ec50..a3e460714d23 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;
> +	}

Yes it is good to do a sanity check before using it.

- Sourabh Jain

> +
> +	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)
> @@ -431,12 +441,12 @@ int crash_load_dm_crypt_keys(struct kimage *image)
>   
>   	mutex_lock(&config_keys_subsys.su_mutex);
>   
> -	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");
> +			return 0;
> +		}
> +
>   		r = build_keys_header();
>   		if (r)
>   			goto out;
> @@ -447,7 +457,7 @@ int crash_load_dm_crypt_keys(struct kimage *image)
>   	 * cleaned up at the end of kexec_file_load syscall
>   	 */
>   	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;


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 7/9] crash_dump: Improve readability of config_keys_restore_store
  2026-05-01 23:43 ` [PATCH v2 7/9] crash_dump: Improve readability of config_keys_restore_store Coiby Xu
@ 2026-05-06 14:33   ` Sourabh Jain
  0 siblings, 0 replies; 14+ messages in thread
From: Sourabh Jain @ 2026-05-06 14:33 UTC (permalink / raw)
  To: Coiby Xu, kexec
  Cc: Andrew Morton, Baoquan He, Dave Young, Mike Rapoport,
	Pasha Tatashin, Pratyush Yadav, Coiby Xu, open list



On 02/05/26 05:13, Coiby Xu wrote:
> 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")
> Signed-off-by: Coiby Xu <coiby.xu@gmail.com>
> ---
>   kernel/crash_dump_dm_crypt.c | 10 +++++++---
>   1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c
> index 545d712428d3..36e51807d94f 100644
> --- a/kernel/crash_dump_dm_crypt.c
> +++ b/kernel/crash_dump_dm_crypt.c
> @@ -358,12 +358,16 @@ 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;
>   
> -	if (kstrtobool(page, &restore))
> +	if (kstrtobool(page, &val))
>   		return -EINVAL;
>   
> +	if (val && !restore) {
> +		if (!restore_dm_crypt_keys_to_thread_keyring())
> +			restore = true;
> +	}

Nit:
I think it would be good to warn the user if they try to
restore it when it has already been restored.


- Sourabh Jain
> +
>   	return count;
>   }
>   


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 8/9] crash_dump: Disallow configfs/crash_dm_crypt_key/reuse if CONFIG_CRASH_HOTPLUG enabled
  2026-05-01 23:43 ` [PATCH v2 8/9] crash_dump: Disallow configfs/crash_dm_crypt_key/reuse if CONFIG_CRASH_HOTPLUG enabled Coiby Xu
@ 2026-05-06 16:09   ` Sourabh Jain
  0 siblings, 0 replies; 14+ messages in thread
From: Sourabh Jain @ 2026-05-06 16:09 UTC (permalink / raw)
  To: Coiby Xu, kexec
  Cc: Andrew Morton, Baoquan He, Dave Young, Mike Rapoport,
	Pasha Tatashin, Pratyush Yadav, Jonathan Corbet, Shuah Khan,
	Coiby Xu, open list:DOCUMENTATION, open list



On 02/05/26 05:13, Coiby Xu wrote:
> If CONFIG_CRASH_HOTPLUG is enabled, dm-crypt keys saved to reserved
> memory will be took care of automatically. Thus it doesn't make sense
> to use configfs/crash_dm_crypt_key/reuse. Reserving
> image->dm_crypt_keys_addr is also unnecessary. Currently x86_64 and
> ppc64le have implemented CONFIG_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")
> Signed-off-by: Coiby Xu <coiby.xu@gmail.com>
> ---
>   Documentation/admin-guide/kdump/kdump.rst |  9 ++++++---
>   kernel/crash_dump_dm_crypt.c              | 14 +++++++++++---
>   2 files changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/admin-guide/kdump/kdump.rst b/Documentation/admin-guide/kdump/kdump.rst
> index 7587caadbae1..73f2e9500c60 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
> @@ -593,7 +594,9 @@ with /sys/kernel/config/crash_dm_crypt_keys for setup,
>   
>       # To support CPU/memory hot-plugging, reuse keys already saved to reserved
>       # memory
> -    echo true > /sys/kernel/config/crash_dm_crypt_key/reuse
> +    # Note if CONFIG_CRASH_HOTPLUG is enabled, this API is totally unnecessary
> +    # thus will be disabled.
> +    echo true > /sys/kernel/config/crash_dm_crypt_keys/reuse
>   
>   2. Load the dump-capture kernel
>   
> diff --git a/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c
> index 36e51807d94f..7a7cae17f578 100644
> --- a/kernel/crash_dump_dm_crypt.c
> +++ b/kernel/crash_dump_dm_crypt.c
> @@ -304,6 +304,11 @@ static ssize_t config_keys_reuse_store(struct config_item *item,
>   	bool val;
>   	int r;
>   
> +	if (IS_ENABLED(CONFIG_CRASH_HOTPLUG)) {
> +		pr_info("CONFIG_CRASH_HOTPLUG already enabled");
> +		return -EINVAL;
> +	}
> +

Deciding this solely at compile time can create issues. For example, the 
kernel
may be built with CONFIG_CRASH_HOTPLUG, but if kexec tool loads the kdump
kernel using the kexec_load system call without hotplug support, it can
cause problems. It is rare but possible.

How about this:

#ifdef CONFIG_CRASH_HOTPLUG
     if (kexec_crash_image->hotplug_support) {
pr_info("crash image is loaded with hotplug support\n");return -EINVAL;
     }
#endif

This code should be placed after validating kexec_crash_image.


- Sourabh Jain

>   	if (!kexec_crash_image || !kexec_crash_image->dm_crypt_keys_addr) {
>   		pr_info("dm-crypt keys haven't be saved to crash-reserved memory\n");
>   		return -EINVAL;
> @@ -486,15 +491,18 @@ int crash_load_dm_crypt_keys(struct kimage *image)
>   void kexec_file_post_load_cleanup_dm_crypt(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;
>   	}
>   
> +	if (IS_ENABLED(CONFIG_CRASH_HOTPLUG))
> +		image->dm_crypt_keys_addr = 0;
> +
>   	if (mutex_is_locked(&config_keys_subsys.su_mutex))
>   		mutex_unlock(&config_keys_subsys.su_mutex);
>   }


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2026-05-06 16:10 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20260501234342.2518281-1-coiby.xu@gmail.com>
2026-05-01 23:43 ` [PATCH v2 1/9] crash_dump: Release reference to a keyring at correct time Coiby Xu
2026-05-01 23:43 ` [PATCH v2 2/9] crash_dump: Fix potential double free and UAF of keys_header Coiby Xu
2026-05-06 12:28   ` Sourabh Jain
2026-05-01 23:43 ` [PATCH v2 3/9] crash_dump: Disallow writing to dm-crypt configfs during kexec_file_load syscall Coiby Xu
2026-05-06 13:56   ` Sourabh Jain
2026-05-01 23:43 ` [PATCH v2 4/9] crash_dump: Read the number of dm-crypt keys from reserved memory Coiby Xu
2026-05-06 14:18   ` Sourabh Jain
2026-05-01 23:43 ` [PATCH v2 5/9] crash_dump: Free temporary dm-crypt keys_header buffer in kdump kernel Coiby Xu
2026-05-01 23:43 ` [PATCH v2 6/9] crash_dump: Only use kexec_dprintk during the kexec_file_load syscall Coiby Xu
2026-05-01 23:43 ` [PATCH v2 7/9] crash_dump: Improve readability of config_keys_restore_store Coiby Xu
2026-05-06 14:33   ` Sourabh Jain
2026-05-01 23:43 ` [PATCH v2 8/9] crash_dump: Disallow configfs/crash_dm_crypt_key/reuse if CONFIG_CRASH_HOTPLUG enabled Coiby Xu
2026-05-06 16:09   ` Sourabh Jain
2026-05-01 23:43 ` [PATCH v2 9/9] Documentation: kdump: Add arm64 and ppc64le to encrypted dump target support list Coiby Xu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox