[PATCH v4 0/7] Support kdump with LUKS encryption by reusing LUKS volume keys

[PATCH v4 0/7] Support kdump with LUKS encryption by reusing LUKS volume keys

[Coiby Xu]

LUKS is the standard for Linux disk encryption. Many users choose LUKS
and in some use cases like Confidential VM it's mandated. With kdump
enabled, when the 1st kernel crashes, the system could boot into the
kdump/crash kernel and dump the memory image i.e. /proc/vmcore to a
specified target. Currently, when dumping vmcore to a LUKS
encrypted device, there are two problems,

 - Kdump kernel may not be able to decrypt the LUKS partition. For some
   machines, a system administrator may not have a chance to enter the
   password to decrypt the device in kdump initramfs after the 1st kernel
   crashes; For cloud confidential VMs, depending on the policy the
   kdump kernel may not be able to unseal the keys with TPM and the
   console virtual keyboard is untrusted.

 - LUKS2 by default use the memory-hard Argon2 key derivation function
   which is quite memory-consuming compared to the limited memory reserved
   for kdump. Take Fedora example, by default, only 256M is reserved for
   systems having memory between 4G-64G. With LUKS enabled, ~1300M needs
   to be reserved for kdump. Note if the memory reserved for kdump can't
   be used by 1st kernel i.e. an user sees ~1300M memory missing in the
   1st kernel.

Besides users (at least for Fedora) usually expect kdump to work out of
the box i.e. no manual password input is needed. And it doesn't make
sense to derivate the keys again in kdump kernel which seems to be
redundant work.

This patch set addresses the above issues by make the LUKS volume keys
persistent for kdump kernel with the help of cryptsetup's new APIs
(--link-vk-to-keyring/--volume-key-keyring). Here is the life cycle of
this kdump copy of LUKS volume keys,

 1. After the 1st kernel loads the initramfs during boot, systemd
    use an user-input passphrase or TPM-sealed key to de-crypt the LUKS
    volume keys and then save the volume keys to specified keyring
    (using the --link-vk-to-keyring API) and the key will expire within
    specified time.

 2. A user space tool (kdump initramfs builder) writes a key description to
    /sys/kernel/crash_dm_crypt_keys to inform the 1st kernel to record the
    key while building the kdump initramfs

 3. The kexec_file_load syscall read the volume keys by recorded key
    descriptions and then save them key to kdump reserved memory and wipe the
    copy.

 4. When the 1st kernel crashes and the kdump initramfs is booted, the kdump
    initramfs asks the kdump kernel to create a user key using the key stored in
    kdump reserved memory by writing to to /sys/kernel/crash_dm_crypt_keys. Then
    the LUKS encrypted devide is unlocked with libcryptsetup's
    --volume-key-keyring API.

 5. The system gets rebooted to the 1st kernel after dumping vmcore to
    the LUKS encrypted device is finished

After libcryptsetup saving the LUKS volume keys to specified keyring,
whoever takes this should be responsible for the safety of these copies
of keys. The keys will be saved in the memory area exclusively reserved
for kdump where even the 1st kernel has no direct access. And further
more, two additional protections are added,
 - save the copy randomly in kdump reserved memory as suggested by Jan
 - clear the _PAGE_PRESENT flag of the page that stores the copy as
   suggested by Pingfan

This patch set only supports x86. There will be patches to support other
architectures once this patch set gets merged.

v5
 - Baoquan
   - limit the feature of placing kexec_buf randomly to kdump (CONFIG_CRASH_DUMP)
   - add documentation for added sysfs API 
   - allow to re-send init command to support the case of user switching to
     a different LUKS-encrypted target
   - make CONFIG_CRASH_DM_CRYPT depends on CONFIG_DM_CRYPT
   - check if the number of keys exceed KEY_NUM_MAX
   - rename (struct keys_header).key_count as (struct
     keys_header).total_keys to improve code readiblity
   - improve commit message
   - fix the failure of calling crash_exclude_mem_range (there is a split
     of mem_range)
   - use ret instead of r as return code
 
 - Greg
   - add documentation for added sysfs API 
   - avoid spamming kernel logs 
   - fix a buffer overflow issue
   - keep the state enums synced up with the string values
   - use sysfs_emit other than sprintf
   - explain KEY_NUM_MAX and KEY_SIZE_MAX
   - s/EXPORT_SYMBOL_GPL/EXPORT_SYMBOL/g
   - improve code readability
 
 - Rebase onto latest Linus tree


v4
- rebase onto latest Linus tree so Baoquan can apply the patches for
  code review
- fix kernel test robot warnings

v3
 - Support CPU/memory hot-plugging [Baoquan]
 - Don't save the keys temporarily to simplify the implementation [Baoquan]
 - Support multiple LUKS encrypted volumes
 - Read logon key instead of user key to improve security [Ondrej]
 - A kernel config option CRASH_DM_CRYPT for this feature (disabled by default)
 - Fix warnings found by kernel test robot
 - Rebase the code onto 6.9.0-rc5+

v2
 - work together with libscryptsetup's --link-vk-to-keyring/--volume-key-keyring APIs [Milan and Ondrej]
 - add the case where console virtual keyboard is untrusted for confidential VM
 - use dm_crypt_key instead of LUKS volume key [Milan and Eric]
 - fix some code format issues
 - don't move "struct kexec_segment" declaration
 - Rebase the code onto latest Linus tree (6.7.0)

v1
 - "Put the luks key handling related to crash_dump out into a separate
   file kernel/crash_dump_luks.c" [Baoquan]
 - Put the generic luks handling code before the x86 specific code to
   make it easier for other arches to follow suit [Baoquan]
 - Use phys_to_virt instead of "pfn -> page -> vaddr" [Dave Hansen]
 - Drop the RFC prefix [Dave Young]
 - Rebase the code onto latest Linus tree (6.4.0-rc4)

RFC v2
 - libcryptsetup interacts with the kernel via sysfs instead of "hacking"
   dm-crypt
   - to save a kdump copy of the LUKS volume key in 1st kernel
   - to add a logon key using the copy for libcryptsetup in kdump kernel [Milan]
   - to avoid the incorrect usage of LUKS master key in dm-crypt [Milan]
 - save the kdump copy of LUKS volume key randomly [Jan]
 - mark the kdump copy inaccessible [Pingfan]
 - Miscellaneous
   - explain when operations related to the LUKS volume key happen [Jan]
   - s/master key/volume key/g
   - use crash_ instead of kexec_ as function prefix
   - fix commit subject prefixes e.g. "x86, kdump" to x86/crash

Coiby Xu (7):
  kexec_file: allow to place kexec_buf randomly
  crash_dump: make dm crypt keys persist for the kdump kernel
  crash_dump: store dm crypt keys in kdump reserved memory
  crash_dump: reuse saved dm crypt keys for CPU/memory hot-plugging
  crash_dump: retrieve dm crypt keys in kdump kernel
  x86/crash: pass dm crypt keys to kdump kernel
  x86/crash: make the page that stores the dm crypt keys inaccessible

 Documentation/ABI/testing/crash_dm_crypt_keys |  35 ++
 arch/x86/kernel/crash.c                       |  20 +-
 arch/x86/kernel/kexec-bzimage64.c             |   7 +
 arch/x86/kernel/machine_kexec_64.c            |  22 ++
 include/linux/crash_core.h                    |   9 +-
 include/linux/crash_dump.h                    |   2 +
 include/linux/kexec.h                         |   8 +
 kernel/Kconfig.kexec                          |   9 +
 kernel/Makefile                               |   1 +
 kernel/crash_dump_dm_crypt.c                  | 338 ++++++++++++++++++
 kernel/kexec_file.c                           |  21 ++
 kernel/ksysfs.c                               |  24 ++
 12 files changed, 493 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/ABI/testing/crash_dm_crypt_keys
 create mode 100644 kernel/crash_dump_dm_crypt.c


base-commit: 8a92980606e3585d72d510a03b59906e96755b8a
-- 
2.45.1

[PATCH v5 1/7] kexec_file: allow to place kexec_buf randomly

[Coiby Xu]

Currently, kexec_buf is placed in order which means for the same
machine, the info in the kexec_buf is always located at the same
position each time the machine is booted. This may cause a risk for
sensitive information like LUKS volume key. Now struct kexec_buf has a
new field random which indicates it's supposed to be placed in a random
position.

Suggested-by: Jan Pazdziora <jpazdziora@redhat.com>
Signed-off-by: Coiby Xu <coxu@redhat.com>
---
 include/linux/kexec.h |  4 ++++
 kernel/kexec_file.c   | 21 +++++++++++++++++++++
 2 files changed, 25 insertions(+)

diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index f0e9f8eda7a3..c45bfc727737 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -171,6 +171,7 @@ int kexec_image_post_load_cleanup_default(struct kimage *image);
  * @buf_min:	The buffer can't be placed below this address.
  * @buf_max:	The buffer can't be placed above this address.
  * @top_down:	Allocate from top of memory.
+ * @random:	Place the buffer at a random position.
  */
 struct kexec_buf {
 	struct kimage *image;
@@ -182,6 +183,9 @@ struct kexec_buf {
 	unsigned long buf_min;
 	unsigned long buf_max;
 	bool top_down;
+#ifdef CONFIG_CRASH_DUMP
+	bool random;
+#endif
 };
 
 int kexec_load_purgatory(struct kimage *image, struct kexec_buf *kbuf);
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 3d64290d24c9..f7538d8f67e0 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -25,6 +25,9 @@
 #include <linux/elfcore.h>
 #include <linux/kernel.h>
 #include <linux/kernel_read_file.h>
+#ifdef CONFIG_CRASH_DUMP
+#include <linux/prandom.h>
+#endif
 #include <linux/syscalls.h>
 #include <linux/vmalloc.h>
 #include "kexec_internal.h"
@@ -437,6 +440,18 @@ SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, int, initrd_fd,
 	return ret;
 }
 
+#ifdef CONFIG_CRASH_DUMP
+static unsigned long kexec_random_start(unsigned long start, unsigned long end)
+{
+	unsigned long temp_start;
+	unsigned short i;
+
+	get_random_bytes(&i, sizeof(unsigned short));
+	temp_start = start + (end - start) / USHRT_MAX * i;
+	return temp_start;
+}
+#endif
+
 static int locate_mem_hole_top_down(unsigned long start, unsigned long end,
 				    struct kexec_buf *kbuf)
 {
@@ -445,6 +460,10 @@ static int locate_mem_hole_top_down(unsigned long start, unsigned long end,
 
 	temp_end = min(end, kbuf->buf_max);
 	temp_start = temp_end - kbuf->memsz + 1;
+#ifdef CONFIG_CRASH_DUMP
+	if (kbuf->random)
+		temp_start = kexec_random_start(temp_start, temp_end);
+#endif
 
 	do {
 		/* align down start */
@@ -482,6 +501,8 @@ static int locate_mem_hole_bottom_up(unsigned long start, unsigned long end,
 	unsigned long temp_start, temp_end;
 
 	temp_start = max(start, kbuf->buf_min);
+	if (kbuf->random)
+		temp_start = kexec_random_start(temp_start, end);
 
 	do {
 		temp_start = ALIGN(temp_start, kbuf->buf_align);
-- 
2.45.1

[PATCH v5 2/7] crash_dump: make dm crypt keys persist for the kdump kernel

[Coiby Xu]

A sysfs /sys/kernel/crash_dm_crypt_keys is provided for user space to
make the dm crypt keys persist for the kdump kernel. Take the case of
dumping to a LUKS-encrypted target as an example, here is the life cycle
of this kdump copy of LUKS volume keys,

 1. After the 1st kernel loads the initramfs during boot, systemd
    use an user-input passphrase or TPM-sealed key to de-crypt the LUKS
    volume keys and then save the volume keys to specified keyring
    (using the --link-vk-to-keyring API) and the key will expire within
    specified time.

 2. A user space tool (kdump initramfs builder) writes a key description to
    /sys/kernel/crash_dm_crypt_keys to inform the 1st kernel to record the
    key while building the kdump initramfs

 3. The kexec_file_load syscall read the volume keys by recorded key
    descriptions and then save them key to kdump reserved memory and wipe the
    copy.

 4. When the 1st kernel crashes and the kdump initramfs is booted, the kdump
    initramfs asks the kdump kernel to create a user key using the key stored in
    kdump reserved memory by writing to to /sys/kernel/crash_dm_crypt_keys. Then
    the LUKS encrypted devide is unlocked with libcryptsetup's
    --volume-key-keyring API.

 5. The system gets rebooted to the 1st kernel after dumping vmcore to
    the LUKS encrypted device is finished

I assume 1) there are 128 LUKS devices at maximum to be unlocked thus
MAX_KEY_NUM=128 2) and a key won't exceed 256 bytes thus
MAX_KEY_SIZE=256 according to "cryptsetup benchmark".

For details on usage of the API, please check the new doc file
Documentation/ABI/testing/crash_dm_crypt_keys.

Cc: Greg KH <gregkh@linuxfoundation.org>
Signed-off-by: Coiby Xu <coxu@redhat.com>
---
 Documentation/ABI/testing/crash_dm_crypt_keys |  30 ++++
 include/linux/crash_core.h                    |   5 +-
 kernel/Kconfig.kexec                          |   9 ++
 kernel/Makefile                               |   1 +
 kernel/crash_dump_dm_crypt.c                  | 130 ++++++++++++++++++
 kernel/ksysfs.c                               |  24 ++++
 6 files changed, 198 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ABI/testing/crash_dm_crypt_keys
 create mode 100644 kernel/crash_dump_dm_crypt.c

diff --git a/Documentation/ABI/testing/crash_dm_crypt_keys b/Documentation/ABI/testing/crash_dm_crypt_keys
new file mode 100644
index 000000000000..e6a6f6be5a9e
--- /dev/null
+++ b/Documentation/ABI/testing/crash_dm_crypt_keys
@@ -0,0 +1,30 @@
+What:		/sys/kernel/crash_dm_crypt_keys
+Date:		Jun 2024
+KernelVersion:	6.11
+Contact:	kexec@lists.infradead.org
+Description:	read/write
+    Make dm crypt keys persistent for the kdump kernel.
+
+    Assume the key size won't exceed 256 bytes and the maximum number of keys is 128.
+
+    You can write the following commands before kexec'ing the kdump kernel,
+      - "init KEY_NUM"
+        Let the kernel know the number of dm crypt keys so it will initialize
+        needed structures. KEY_NUM=128 dm crypt keys at maximum.
+        - "record KEY_DESC"
+        Record a key description. For security reason, the key must be a logon
+        key whose payload can't be read by user space. For details, please refer
+        to security/keys/core.rst.
+
+    And you can also read this API to know the command eructation status,
+      - fresh
+        Waiting for a command
+      - initialized
+        The "init KEY_NUM" command has been executed successfully
+      - recorded
+        Specified number of keys have been recorded
+      - loaded
+        the kdump kernel has been loaded with the dm crypt keys stored to kdump
+        reserved memory
+
+User:		Kdump service
diff --git a/include/linux/crash_core.h b/include/linux/crash_core.h
index 44305336314e..6bff1c24efa3 100644
--- a/include/linux/crash_core.h
+++ b/include/linux/crash_core.h
@@ -34,7 +34,10 @@ static inline void arch_kexec_protect_crashkres(void) { }
 static inline void arch_kexec_unprotect_crashkres(void) { }
 #endif
 
-
+#ifdef CONFIG_CRASH_DM_CRYPT
+int crash_sysfs_dm_crypt_keys_read(char *buf);
+int crash_sysfs_dm_crypt_keys_write(const char *buf, size_t count);
+#endif
 
 #ifndef arch_crash_handle_hotplug_event
 static inline void arch_crash_handle_hotplug_event(struct kimage *image, void *arg) { }
diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
index 6c34e63c88ff..d067ba252163 100644
--- a/kernel/Kconfig.kexec
+++ b/kernel/Kconfig.kexec
@@ -116,6 +116,15 @@ config CRASH_DUMP
 	  For s390, this option also enables zfcpdump.
 	  See also <file:Documentation/arch/s390/zfcpdump.rst>
 
+config CRASH_DM_CRYPT
+	bool "Support saving crash dump to dm-crypt encrypted volume"
+	depends on CRASH_DUMP
+	depends on DM_CRYPT
+	help
+	  With this option enabled, user space can intereact with
+	  /sys/kernel/crash_dm_crypt_keys to make the dm crypt keys
+	  persistent for the crash dump kernel.
+
 config CRASH_HOTPLUG
 	bool "Update the crash elfcorehdr on system configuration changes"
 	default y
diff --git a/kernel/Makefile b/kernel/Makefile
index 3c13240dfc9f..f2e5b3e86d12 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -72,6 +72,7 @@ obj-$(CONFIG_VMCORE_INFO) += vmcore_info.o elfcorehdr.o
 obj-$(CONFIG_CRASH_RESERVE) += crash_reserve.o
 obj-$(CONFIG_KEXEC_CORE) += kexec_core.o
 obj-$(CONFIG_CRASH_DUMP) += crash_core.o
+obj-$(CONFIG_CRASH_DM_CRYPT) += crash_dump_dm_crypt.o
 obj-$(CONFIG_KEXEC) += kexec.o
 obj-$(CONFIG_KEXEC_FILE) += kexec_file.o
 obj-$(CONFIG_KEXEC_ELF) += kexec_elf.o
diff --git a/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c
new file mode 100644
index 000000000000..608bde3aaa8e
--- /dev/null
+++ b/kernel/crash_dump_dm_crypt.c
@@ -0,0 +1,130 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <keys/user-type.h>
+#include <linux/crash_dump.h>
+
+#define KEY_NUM_MAX 128   /* maximum dm crypt keys */
+#define KEY_SIZE_MAX 256  /* maximum dm crypt key size */
+
+// The key scription has the format: cryptsetup:UUID 11+36+1(NULL)=48
+#define KEY_DESC_LEN 48
+
+static enum STATE_ENUM {
+	FRESH = 0,
+	INITIALIZED,
+	RECORDED,
+	LOADED,
+} state;
+
+static const char * const STATE_STR[] = {
+	[FRESH] = "fresh",
+	[INITIALIZED] = "initialized",
+	[RECORDED] = "recorded",
+	[LOADED] = "loaded"
+};
+
+static unsigned int key_count;
+static size_t keys_header_size;
+
+struct dm_crypt_key {
+	unsigned int key_size;
+	char key_desc[KEY_DESC_LEN];
+	u8 data[KEY_SIZE_MAX];
+};
+
+static struct keys_header {
+	unsigned int total_keys;
+	struct dm_crypt_key keys[] __counted_by(total_keys);
+} *keys_header;
+
+static size_t get_keys_header_size(struct keys_header *keys_header,
+				   size_t total_keys)
+{
+	return struct_size(keys_header, keys, total_keys);
+}
+
+/*
+ * Let the kernel know the number of dm crypt keys and allocate memory to
+ * initialize related structures.
+ */
+static int init(const char *buf)
+{
+	unsigned int total_keys;
+
+	if (sscanf(buf, "init %u", &total_keys) != 1)
+		return -EINVAL;
+
+	if (total_keys > KEY_NUM_MAX) {
+		kexec_dprintk(
+			"Exceed the maximum number of keys (KEY_NUM_MAX=%u)\n",
+			KEY_NUM_MAX);
+		return -EINVAL;
+	}
+
+	keys_header_size = get_keys_header_size(keys_header, total_keys);
+	key_count = 0;
+
+	if (keys_header != NULL)
+		kvfree(keys_header);
+
+	keys_header = kzalloc(keys_header_size, GFP_KERNEL);
+	if (!keys_header)
+		return -ENOMEM;
+
+	keys_header->total_keys = total_keys;
+	state = INITIALIZED;
+	return 0;
+}
+
+/*
+ * Record the key description of a dm crypt key.
+ */
+static int record_key_desc(const char *buf, struct dm_crypt_key *dm_key)
+{
+	char key_desc[KEY_DESC_LEN];
+
+	if (state != INITIALIZED) {
+		kexec_dprintk("Please send the cmd 'init <KEY_NUM>' first\n");
+		return -EINVAL;
+	}
+
+	if (sscanf(buf, "record %s", key_desc) != 1)
+		return -EINVAL;
+
+	if (key_count >= keys_header->total_keys) {
+		kexec_dprintk("Already have %u keys", key_count);
+		return -EINVAL;
+	}
+
+	strscpy(dm_key->key_desc, key_desc, KEY_DESC_LEN);
+	kexec_dprintk("Key%d (%s) recorded\n", key_count, dm_key->key_desc);
+	key_count++;
+
+	if (key_count == keys_header->total_keys)
+		state = RECORDED;
+
+	return 0;
+}
+
+static int process_cmd(const char *buf, size_t count)
+{
+	if (strncmp(buf, "init ", 5) == 0)
+		return init(buf);
+	else if (strncmp(buf, "record ", 7) == 0 && count == KEY_DESC_LEN + 6)
+		return record_key_desc(buf, &keys_header->keys[key_count]);
+
+	return -EINVAL;
+}
+
+int crash_sysfs_dm_crypt_keys_write(const char *buf, size_t count)
+{
+	if (!is_kdump_kernel())
+		return process_cmd(buf, count);
+	return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(crash_sysfs_dm_crypt_keys_write);
+
+int crash_sysfs_dm_crypt_keys_read(char *buf)
+{
+	return sysfs_emit(buf, "%s\n", STATE_STR[state]);
+}
+EXPORT_SYMBOL_GPL(crash_sysfs_dm_crypt_keys_read);
diff --git a/kernel/ksysfs.c b/kernel/ksysfs.c
index 07fb5987b42b..ff7caef30f51 100644
--- a/kernel/ksysfs.c
+++ b/kernel/ksysfs.c
@@ -167,6 +167,27 @@ static ssize_t vmcoreinfo_show(struct kobject *kobj,
 }
 KERNEL_ATTR_RO(vmcoreinfo);
 
+#ifdef CONFIG_CRASH_DM_CRYPT
+static ssize_t crash_dm_crypt_keys_show(struct kobject *kobj,
+					struct kobj_attribute *attr, char *buf)
+{
+	return crash_sysfs_dm_crypt_keys_read(buf);
+}
+
+static ssize_t crash_dm_crypt_keys_store(struct kobject *kobj,
+					 struct kobj_attribute *attr,
+					 const char *buf, size_t count)
+{
+	int ret;
+
+	ret = crash_sysfs_dm_crypt_keys_write(buf, count);
+	if (ret < 0)
+		return ret;
+	return count;
+}
+KERNEL_ATTR_RW(crash_dm_crypt_keys);
+#endif /* CONFIG_CRASH_DM_CRYPT */
+
 #ifdef CONFIG_CRASH_HOTPLUG
 static ssize_t crash_elfcorehdr_size_show(struct kobject *kobj,
 			       struct kobj_attribute *attr, char *buf)
@@ -271,6 +292,9 @@ static struct attribute * kernel_attrs[] = {
 #endif
 #ifdef CONFIG_VMCORE_INFO
 	&vmcoreinfo_attr.attr,
+#ifdef CONFIG_CRASH_DM_CRYPT
+	&crash_dm_crypt_keys_attr.attr,
+#endif
 #ifdef CONFIG_CRASH_HOTPLUG
 	&crash_elfcorehdr_size_attr.attr,
 #endif
-- 
2.45.1

[PATCH v5 3/7] crash_dump: store dm crypt keys in kdump reserved memory

[Coiby Xu]

When the kdump kernel image and initrd are loaded, the dm crypts keys
will be read from keyring and then stored in kdump reserved memory.

Signed-off-by: Coiby Xu <coxu@redhat.com>
---
 include/linux/crash_core.h   |  3 ++
 include/linux/crash_dump.h   |  2 +
 include/linux/kexec.h        |  4 ++
 kernel/crash_dump_dm_crypt.c | 87 ++++++++++++++++++++++++++++++++++++
 4 files changed, 96 insertions(+)

diff --git a/include/linux/crash_core.h b/include/linux/crash_core.h
index 6bff1c24efa3..ab20829d0bc9 100644
--- a/include/linux/crash_core.h
+++ b/include/linux/crash_core.h
@@ -37,6 +37,9 @@ static inline void arch_kexec_unprotect_crashkres(void) { }
 #ifdef CONFIG_CRASH_DM_CRYPT
 int crash_sysfs_dm_crypt_keys_read(char *buf);
 int crash_sysfs_dm_crypt_keys_write(const char *buf, size_t count);
+int crash_load_dm_crypt_keys(struct kimage *image);
+#else
+static inline int crash_load_dm_crypt_keys(struct kimage *image) {return 0; }
 #endif
 
 #ifndef arch_crash_handle_hotplug_event
diff --git a/include/linux/crash_dump.h b/include/linux/crash_dump.h
index acc55626afdc..dfd8e4fe6129 100644
--- a/include/linux/crash_dump.h
+++ b/include/linux/crash_dump.h
@@ -15,6 +15,8 @@
 extern unsigned long long elfcorehdr_addr;
 extern unsigned long long elfcorehdr_size;
 
+extern unsigned long long dm_crypt_keys_addr;
+
 #ifdef CONFIG_CRASH_DUMP
 extern int elfcorehdr_alloc(unsigned long long *addr, unsigned long long *size);
 extern void elfcorehdr_free(unsigned long long addr);
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index c45bfc727737..cb6275928fbd 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -372,6 +372,10 @@ struct kimage {
 	void *elf_headers;
 	unsigned long elf_headers_sz;
 	unsigned long elf_load_addr;
+
+	/* dm crypt keys buffer */
+	unsigned long dm_crypt_keys_addr;
+	unsigned long dm_crypt_keys_sz;
 };
 
 /* kexec interface functions */
diff --git a/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c
index 608bde3aaa8e..0033152668ae 100644
--- a/kernel/crash_dump_dm_crypt.c
+++ b/kernel/crash_dump_dm_crypt.c
@@ -1,4 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-only
+#include <linux/key.h>
+#include <linux/keyctl.h>
 #include <keys/user-type.h>
 #include <linux/crash_dump.h>
 
@@ -128,3 +130,88 @@ int crash_sysfs_dm_crypt_keys_read(char *buf)
 	return sysfs_emit(buf, "%s\n", STATE_STR[state]);
 }
 EXPORT_SYMBOL_GPL(crash_sysfs_dm_crypt_keys_read);
+
+static int read_key_from_user_keying(struct dm_crypt_key *dm_key)
+{
+	const struct user_key_payload *ukp;
+	struct key *key;
+
+	kexec_dprintk("Requesting key %s", dm_key->key_desc);
+	key = request_key(&key_type_logon, dm_key->key_desc, NULL);
+
+	if (IS_ERR(key)) {
+		pr_warn("No such key %s\n", dm_key->key_desc);
+		return PTR_ERR(key);
+	}
+
+	ukp = user_key_payload_locked(key);
+	if (!ukp)
+		return -EKEYREVOKED;
+
+	memcpy(dm_key->data, ukp->data, ukp->datalen);
+	dm_key->key_size = ukp->datalen;
+	kexec_dprintk("Get dm crypt key (size=%u) %s: %8ph\n", dm_key->key_size,
+		      dm_key->key_desc, dm_key->data);
+	return 0;
+}
+
+static int build_keys_header(void)
+{
+	int i, r;
+
+	for (i = 0; i < key_count; i++) {
+		r = read_key_from_user_keying(&keys_header->keys[i]);
+		if (r != 0) {
+			pr_err("Failed to read key %s\n", keys_header->keys[i].key_desc);
+			return r;
+		}
+	}
+
+	return 0;
+}
+
+int crash_load_dm_crypt_keys(struct kimage *image)
+{
+	struct kexec_buf kbuf = {
+		.image = image,
+		.buf_min = 0,
+		.buf_max = ULONG_MAX,
+		.top_down = false,
+		.random = true,
+	};
+
+	int r;
+
+	if (state == FRESH)
+		return 0;
+
+	if (key_count != keys_header->total_keys) {
+		kexec_dprintk("Only record %u keys (%u in total)\n", key_count,
+			      keys_header->total_keys);
+		return -EINVAL;
+	}
+
+	image->dm_crypt_keys_addr = 0;
+	r = build_keys_header();
+	if (r)
+		return r;
+
+	kbuf.buffer = keys_header;
+	kbuf.bufsz = keys_header_size;
+
+	kbuf.memsz = kbuf.bufsz;
+	kbuf.buf_align = ELF_CORE_HEADER_ALIGN;
+	kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
+	r = kexec_add_buffer(&kbuf);
+	if (r) {
+		kvfree((void *)kbuf.buffer);
+		return r;
+	}
+	state = LOADED;
+	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.bufsz);
+
+	return r;
+}
-- 
2.45.1

[PATCH v5 4/7] crash_dump: reuse saved dm crypt keys for CPU/memory hot-plugging

[Coiby Xu]

When there are CPU and memory hot un/plugs, the crash elfcorehdr which
describes CPUs and memory in the system needs to be updated for the
kdump kernel.

Currently, there are two solutions to support this case. One is to
utilizes udev to instruct user space to reload the kdump kernel image
and initrd, elfcorehdr and etc. again. The other is to only update the
elfcorehdr segment. For the 1st solution, the dm crypt keys need to be
reloaded again. The user space can write the "reuse" command to
/sys/kernel/crash_dm_crypt_key so the stored keys can be re-saved again.

Note only x86 (commit ea53ad9cf73b ("x86/crash: add x86 crash
hotplug support")) and ppc (WIP) supports the new infrastructure
(commit 247262756121 ("crash: add generic infrastructure for crash
hotplug support")). If the new infrastructure get extended to all arches,
this patch can be dropped.

Signed-off-by: Coiby Xu <coxu@redhat.com>
---
 Documentation/ABI/testing/crash_dm_crypt_keys |  2 ++
 kernel/crash_dump_dm_crypt.c                  | 32 ++++++++++++++++---
 2 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/Documentation/ABI/testing/crash_dm_crypt_keys b/Documentation/ABI/testing/crash_dm_crypt_keys
index e6a6f6be5a9e..7426c9d8de97 100644
--- a/Documentation/ABI/testing/crash_dm_crypt_keys
+++ b/Documentation/ABI/testing/crash_dm_crypt_keys
@@ -15,6 +15,8 @@ Description:	read/write
         Record a key description. For security reason, the key must be a logon
         key whose payload can't be read by user space. For details, please refer
         to security/keys/core.rst.
+      - "reuse"
+        Reuse the dm crypt keys stored in kdump reserved memory.
 
     And you can also read this API to know the command eructation status,
       - fresh
diff --git a/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c
index 0033152668ae..9a6bd39adf76 100644
--- a/kernel/crash_dump_dm_crypt.c
+++ b/kernel/crash_dump_dm_crypt.c
@@ -15,13 +15,15 @@ static enum STATE_ENUM {
 	INITIALIZED,
 	RECORDED,
 	LOADED,
+	REUSE,
 } state;
 
 static const char * const STATE_STR[] = {
 	[FRESH] = "fresh",
 	[INITIALIZED] = "initialized",
 	[RECORDED] = "recorded",
-	[LOADED] = "loaded"
+	[LOADED] = "loaded",
+	[REUSE] = "reuse"
 };
 
 static unsigned int key_count;
@@ -107,12 +109,32 @@ static int record_key_desc(const char *buf, struct dm_crypt_key *dm_key)
 	return 0;
 }
 
+static void get_keys_from_kdump_reserved_memory(void)
+{
+	struct keys_header *keys_header_loaded;
+
+	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, keys_header_size);
+	kunmap_local(keys_header_loaded);
+	state = RECORDED;
+	arch_kexec_protect_crashkres();
+}
+
 static int process_cmd(const char *buf, size_t count)
 {
 	if (strncmp(buf, "init ", 5) == 0)
 		return init(buf);
 	else if (strncmp(buf, "record ", 7) == 0 && count == KEY_DESC_LEN + 6)
 		return record_key_desc(buf, &keys_header->keys[key_count]);
+	else if (!strcmp(buf, "reuse")) {
+		state = REUSE;
+		get_keys_from_kdump_reserved_memory();
+		return 0;
+	}
 
 	return -EINVAL;
 }
@@ -192,9 +214,11 @@ int crash_load_dm_crypt_keys(struct kimage *image)
 	}
 
 	image->dm_crypt_keys_addr = 0;
-	r = build_keys_header();
-	if (r)
-		return r;
+	if (state != REUSE) {
+		r = build_keys_header();
+		if (r)
+			return r;
+	}
 
 	kbuf.buffer = keys_header;
 	kbuf.bufsz = keys_header_size;
-- 
2.45.1

[PATCH v5 5/7] crash_dump: retrieve dm crypt keys in kdump kernel

[Coiby Xu]

Crash kernel will retrieve the dm crypt keys based on the dmcryptkeys
command line parameter. When user space writes the key description to
/sys/kernel/crash_dm_crypt_key, the crash kernel will save the
encryption keys to the user keyring. Then user space e.g. cryptsetup's
--volume-key-keyring API can use it to unlock the encrypted device.

Signed-off-by: Coiby Xu <coxu@redhat.com>
---
 Documentation/ABI/testing/crash_dm_crypt_keys |   3 +
 include/linux/crash_core.h                    |   1 +
 kernel/crash_dump_dm_crypt.c                  | 101 +++++++++++++++++-
 3 files changed, 103 insertions(+), 2 deletions(-)

diff --git a/Documentation/ABI/testing/crash_dm_crypt_keys b/Documentation/ABI/testing/crash_dm_crypt_keys
index 7426c9d8de97..56a4b878a4dd 100644
--- a/Documentation/ABI/testing/crash_dm_crypt_keys
+++ b/Documentation/ABI/testing/crash_dm_crypt_keys
@@ -29,4 +29,7 @@ Description:	read/write
         the kdump kernel has been loaded with the dm crypt keys stored to kdump
         reserved memory
 
+    After the kdump kernel gets booted, user space can write anything to this API
+    so the dm crypt keys can be restored to the keyring.
+
 User:		Kdump service
diff --git a/include/linux/crash_core.h b/include/linux/crash_core.h
index ab20829d0bc9..d7308b6e83f4 100644
--- a/include/linux/crash_core.h
+++ b/include/linux/crash_core.h
@@ -38,6 +38,7 @@ static inline void arch_kexec_unprotect_crashkres(void) { }
 int crash_sysfs_dm_crypt_keys_read(char *buf);
 int crash_sysfs_dm_crypt_keys_write(const char *buf, size_t count);
 int crash_load_dm_crypt_keys(struct kimage *image);
+ssize_t dm_crypt_keys_read(char *buf, size_t count, u64 *ppos);
 #else
 static inline int crash_load_dm_crypt_keys(struct kimage *image) {return 0; }
 #endif
diff --git a/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c
index 9a6bd39adf76..00223343473b 100644
--- a/kernel/crash_dump_dm_crypt.c
+++ b/kernel/crash_dump_dm_crypt.c
@@ -40,12 +40,67 @@ static struct keys_header {
 	struct dm_crypt_key keys[] __counted_by(total_keys);
 } *keys_header;
 
+unsigned long long dm_crypt_keys_addr;
+EXPORT_SYMBOL_GPL(dm_crypt_keys_addr);
+
+static int __init setup_dmcryptkeys(char *arg)
+{
+	char *end;
+
+	if (!arg)
+		return -EINVAL;
+	dm_crypt_keys_addr = memparse(arg, &end);
+	if (end > arg)
+		return 0;
+
+	dm_crypt_keys_addr = 0;
+	return -EINVAL;
+}
+
+early_param("dmcryptkeys", setup_dmcryptkeys);
+
 static size_t get_keys_header_size(struct keys_header *keys_header,
 				   size_t total_keys)
 {
 	return struct_size(keys_header, keys, total_keys);
 }
 
+/*
+ * Architectures may override this function to read dm crypt keys
+ */
+ssize_t __weak dm_crypt_keys_read(char *buf, size_t count, u64 *ppos)
+{
+	struct kvec kvec = { .iov_base = buf, .iov_len = count };
+	struct iov_iter iter;
+
+	iov_iter_kvec(&iter, READ, &kvec, 1, count);
+	return read_from_oldmem(&iter, count, ppos, false);
+}
+
+static int add_key_to_keyring(struct dm_crypt_key *dm_key,
+			      key_ref_t keyring_ref)
+{
+	key_ref_t key_ref;
+	int r;
+
+	/* 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,
+				       dm_key->data, dm_key->key_size,
+				       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);
+		kexec_dprintk("Success adding key %s", dm_key->key_desc);
+	} else {
+		r = PTR_ERR(key_ref);
+		kexec_dprintk("Error when adding key");
+	}
+
+	key_ref_put(keyring_ref);
+	return r;
+}
+
 /*
  * Let the kernel know the number of dm crypt keys and allocate memory to
  * initialize related structures.
@@ -139,11 +194,53 @@ static int process_cmd(const char *buf, size_t count)
 	return -EINVAL;
 }
 
+static int restore_dm_crypt_keys_to_thread_keyring(const char *key_desc)
+{
+	struct dm_crypt_key *key;
+	key_ref_t keyring_ref;
+	u64 addr;
+
+	/* find the target keyring (which must be writable) */
+	keyring_ref =
+		lookup_user_key(KEY_SPEC_USER_KEYRING, 0x01, KEY_NEED_WRITE);
+	if (IS_ERR(keyring_ref)) {
+		kexec_dprintk("Failed to get keyring %s\n", key_desc);
+		return PTR_ERR(keyring_ref);
+	}
+
+	addr = dm_crypt_keys_addr;
+	dm_crypt_keys_read((char *)&key_count, sizeof(key_count), &addr);
+	if (key_count < 0 || key_count > KEY_NUM_MAX) {
+		pr_info("Failed to read the number of dm_crypt keys\n");
+		return -1;
+	}
+
+	kexec_dprintk("There are %u keys\n", key_count);
+	addr = dm_crypt_keys_addr;
+
+	keys_header_size = get_keys_header_size(keys_header, key_count);
+
+	keys_header = kzalloc(keys_header_size, GFP_KERNEL);
+	if (!keys_header)
+		return -ENOMEM;
+
+	dm_crypt_keys_read((char *)keys_header, keys_header_size, &addr);
+
+	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);
+		add_key_to_keyring(key, keyring_ref);
+	}
+
+	return 0;
+}
+
 int crash_sysfs_dm_crypt_keys_write(const char *buf, size_t count)
 {
 	if (!is_kdump_kernel())
 		return process_cmd(buf, count);
-	return -EINVAL;
+	else
+		return restore_dm_crypt_keys_to_thread_keyring(buf);
 }
 EXPORT_SYMBOL_GPL(crash_sysfs_dm_crypt_keys_write);
 
@@ -184,7 +281,7 @@ static int build_keys_header(void)
 	for (i = 0; i < key_count; i++) {
 		r = read_key_from_user_keying(&keys_header->keys[i]);
 		if (r != 0) {
-			pr_err("Failed to read key %s\n", keys_header->keys[i].key_desc);
+			kexec_dprintk("Failed to read key %s\n", keys_header->keys[i].key_desc);
 			return r;
 		}
 	}
-- 
2.45.1

[PATCH v5 6/7] x86/crash: pass dm crypt keys to kdump kernel

[Coiby Xu]

1st kernel will build up the kernel command parameter dmcryptkeys as
similar to elfcorehdr to pass the memory address of the stored info of
dm crypt key to kdump kernel.

Signed-off-by: Coiby Xu <coxu@redhat.com>
---
 arch/x86/kernel/crash.c           | 20 ++++++++++++++++++--
 arch/x86/kernel/kexec-bzimage64.c |  7 +++++++
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
index f06501445cd9..ca7f7d44a433 100644
--- a/arch/x86/kernel/crash.c
+++ b/arch/x86/kernel/crash.c
@@ -266,6 +266,7 @@ static int memmap_exclude_ranges(struct kimage *image, struct crash_mem *cmem,
 				 unsigned long long mend)
 {
 	unsigned long start, end;
+	int ret;
 
 	cmem->ranges[0].start = mstart;
 	cmem->ranges[0].end = mend;
@@ -274,22 +275,37 @@ static int memmap_exclude_ranges(struct kimage *image, struct crash_mem *cmem,
 	/* Exclude elf header region */
 	start = image->elf_load_addr;
 	end = start + image->elf_headers_sz - 1;
-	return crash_exclude_mem_range(cmem, start, end);
+	ret = crash_exclude_mem_range(cmem, start, end);
+
+	if (ret)
+		return ret;
+
+	/* Exclude dm crypt keys region */
+	if (image->dm_crypt_keys_addr) {
+		start = image->dm_crypt_keys_addr;
+		end = start + image->dm_crypt_keys_sz - 1;
+		return crash_exclude_mem_range(cmem, start, end);
+	}
+
+	return ret;
 }
 
 /* Prepare memory map for crash dump kernel */
 int crash_setup_memmap_entries(struct kimage *image, struct boot_params *params)
 {
+	unsigned int max_nr_ranges = 3;
 	int i, ret = 0;
 	unsigned long flags;
 	struct e820_entry ei;
 	struct crash_memmap_data cmd;
 	struct crash_mem *cmem;
 
-	cmem = vzalloc(struct_size(cmem, ranges, 1));
+	cmem = vzalloc(struct_size(cmem, ranges, max_nr_ranges));
 	if (!cmem)
 		return -ENOMEM;
 
+	cmem->max_nr_ranges = max_nr_ranges;
+
 	memset(&cmd, 0, sizeof(struct crash_memmap_data));
 	cmd.params = params;
 
diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
index 68530fad05f7..9c94428927bd 100644
--- a/arch/x86/kernel/kexec-bzimage64.c
+++ b/arch/x86/kernel/kexec-bzimage64.c
@@ -76,6 +76,10 @@ static int setup_cmdline(struct kimage *image, struct boot_params *params,
 	if (image->type == KEXEC_TYPE_CRASH) {
 		len = sprintf(cmdline_ptr,
 			"elfcorehdr=0x%lx ", image->elf_load_addr);
+
+		if (image->dm_crypt_keys_addr != 0)
+			len += sprintf(cmdline_ptr + len,
+					"dmcryptkeys=0x%lx ", image->dm_crypt_keys_addr);
 	}
 	memcpy(cmdline_ptr + len, cmdline, cmdline_len);
 	cmdline_len += len;
@@ -441,6 +445,9 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
 		ret = crash_load_segments(image);
 		if (ret)
 			return ERR_PTR(ret);
+		ret = crash_load_dm_crypt_keys(image);
+		if (ret)
+			pr_debug("Either no dm crypt key or error to retrieve the dm crypt key\n");
 	}
 #endif
 
-- 
2.45.1

[PATCH v5 7/7] x86/crash: make the page that stores the dm crypt keys inaccessible

[Coiby Xu]

This adds an addition layer of protection for the saved copy of dm
crypt key. Trying to access the saved copy will cause page fault.

Suggested-by: Pingfan Liu <kernelfans@gmail.com>
Signed-off-by: Coiby Xu <coxu@redhat.com>
---
 arch/x86/kernel/machine_kexec_64.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
index b180d8e497c3..aba50ec641e6 100644
--- a/arch/x86/kernel/machine_kexec_64.c
+++ b/arch/x86/kernel/machine_kexec_64.c
@@ -545,13 +545,35 @@ static void kexec_mark_crashkres(bool protect)
 	kexec_mark_range(control, crashk_res.end, protect);
 }
 
+/* make the memory storing dm crypt keys in/accessible */
+static void kexec_mark_dm_crypt_keys(bool protect)
+{
+	unsigned long start_paddr, end_paddr;
+	unsigned int nr_pages;
+
+	if (kexec_crash_image->dm_crypt_keys_addr) {
+		start_paddr = kexec_crash_image->dm_crypt_keys_addr;
+		end_paddr = start_paddr + kexec_crash_image->dm_crypt_keys_sz - 1;
+		nr_pages = (PAGE_ALIGN(end_paddr) - PAGE_ALIGN_DOWN(start_paddr))/PAGE_SIZE;
+		if (protect)
+			set_memory_np((unsigned long)phys_to_virt(start_paddr), nr_pages);
+		else
+			__set_memory_prot(
+				(unsigned long)phys_to_virt(start_paddr),
+				nr_pages,
+				__pgprot(_PAGE_PRESENT | _PAGE_NX | _PAGE_RW));
+	}
+}
+
 void arch_kexec_protect_crashkres(void)
 {
 	kexec_mark_crashkres(true);
+	kexec_mark_dm_crypt_keys(true);
 }
 
 void arch_kexec_unprotect_crashkres(void)
 {
+	kexec_mark_dm_crypt_keys(false);
 	kexec_mark_crashkres(false);
 }
 #endif
-- 
2.45.1

Re: [PATCH v4 0/7] Support kdump with LUKS encryption by reusing LUKS volume keys

[Coiby Xu]

The subject prefix should be "[PATCH v5 0/7]". I'm sorry if it causes
any confusion.

On Fri, Jun 07, 2024 at 08:26:10PM +0800, Coiby Xu wrote:
>LUKS is the standard for Linux disk encryption. Many users choose LUKS
>and in some use cases like Confidential VM it's mandated. With kdump
>enabled, when the 1st kernel crashes, the system could boot into the
>kdump/crash kernel and dump the memory image i.e. /proc/vmcore to a
>specified target. Currently, when dumping vmcore to a LUKS
>encrypted device, there are two problems,
>
> - Kdump kernel may not be able to decrypt the LUKS partition. For some
>   machines, a system administrator may not have a chance to enter the
>   password to decrypt the device in kdump initramfs after the 1st kernel
>   crashes; For cloud confidential VMs, depending on the policy the
>   kdump kernel may not be able to unseal the keys with TPM and the
>   console virtual keyboard is untrusted.
>
> - LUKS2 by default use the memory-hard Argon2 key derivation function
>   which is quite memory-consuming compared to the limited memory reserved
>   for kdump. Take Fedora example, by default, only 256M is reserved for
>   systems having memory between 4G-64G. With LUKS enabled, ~1300M needs
>   to be reserved for kdump. Note if the memory reserved for kdump can't
>   be used by 1st kernel i.e. an user sees ~1300M memory missing in the
>   1st kernel.
>
>Besides users (at least for Fedora) usually expect kdump to work out of
>the box i.e. no manual password input is needed. And it doesn't make
>sense to derivate the keys again in kdump kernel which seems to be
>redundant work.
>
>This patch set addresses the above issues by make the LUKS volume keys
>persistent for kdump kernel with the help of cryptsetup's new APIs
>(--link-vk-to-keyring/--volume-key-keyring). Here is the life cycle of
>this kdump copy of LUKS volume keys,
>
> 1. After the 1st kernel loads the initramfs during boot, systemd
>    use an user-input passphrase or TPM-sealed key to de-crypt the LUKS
>    volume keys and then save the volume keys to specified keyring
>    (using the --link-vk-to-keyring API) and the key will expire within
>    specified time.
>
> 2. A user space tool (kdump initramfs builder) writes a key description to
>    /sys/kernel/crash_dm_crypt_keys to inform the 1st kernel to record the
>    key while building the kdump initramfs
>
> 3. The kexec_file_load syscall read the volume keys by recorded key
>    descriptions and then save them key to kdump reserved memory and wipe the
>    copy.
>
> 4. When the 1st kernel crashes and the kdump initramfs is booted, the kdump
>    initramfs asks the kdump kernel to create a user key using the key stored in
>    kdump reserved memory by writing to to /sys/kernel/crash_dm_crypt_keys. Then
>    the LUKS encrypted devide is unlocked with libcryptsetup's
>    --volume-key-keyring API.
>
> 5. The system gets rebooted to the 1st kernel after dumping vmcore to
>    the LUKS encrypted device is finished
>
>After libcryptsetup saving the LUKS volume keys to specified keyring,
>whoever takes this should be responsible for the safety of these copies
>of keys. The keys will be saved in the memory area exclusively reserved
>for kdump where even the 1st kernel has no direct access. And further
>more, two additional protections are added,
> - save the copy randomly in kdump reserved memory as suggested by Jan
> - clear the _PAGE_PRESENT flag of the page that stores the copy as
>   suggested by Pingfan
>
>This patch set only supports x86. There will be patches to support other
>architectures once this patch set gets merged.
>
>v5
> - Baoquan
>   - limit the feature of placing kexec_buf randomly to kdump (CONFIG_CRASH_DUMP)
>   - add documentation for added sysfs API
>   - allow to re-send init command to support the case of user switching to
>     a different LUKS-encrypted target
>   - make CONFIG_CRASH_DM_CRYPT depends on CONFIG_DM_CRYPT
>   - check if the number of keys exceed KEY_NUM_MAX
>   - rename (struct keys_header).key_count as (struct
>     keys_header).total_keys to improve code readiblity
>   - improve commit message
>   - fix the failure of calling crash_exclude_mem_range (there is a split
>     of mem_range)
>   - use ret instead of r as return code
>
> - Greg
>   - add documentation for added sysfs API
>   - avoid spamming kernel logs
>   - fix a buffer overflow issue
>   - keep the state enums synced up with the string values
>   - use sysfs_emit other than sprintf
>   - explain KEY_NUM_MAX and KEY_SIZE_MAX
>   - s/EXPORT_SYMBOL_GPL/EXPORT_SYMBOL/g
>   - improve code readability
>
> - Rebase onto latest Linus tree
>
>
>v4
>- rebase onto latest Linus tree so Baoquan can apply the patches for
>  code review
>- fix kernel test robot warnings
>
>v3
> - Support CPU/memory hot-plugging [Baoquan]
> - Don't save the keys temporarily to simplify the implementation [Baoquan]
> - Support multiple LUKS encrypted volumes
> - Read logon key instead of user key to improve security [Ondrej]
> - A kernel config option CRASH_DM_CRYPT for this feature (disabled by default)
> - Fix warnings found by kernel test robot
> - Rebase the code onto 6.9.0-rc5+
>
>v2
> - work together with libscryptsetup's --link-vk-to-keyring/--volume-key-keyring APIs [Milan and Ondrej]
> - add the case where console virtual keyboard is untrusted for confidential VM
> - use dm_crypt_key instead of LUKS volume key [Milan and Eric]
> - fix some code format issues
> - don't move "struct kexec_segment" declaration
> - Rebase the code onto latest Linus tree (6.7.0)
>
>v1
> - "Put the luks key handling related to crash_dump out into a separate
>   file kernel/crash_dump_luks.c" [Baoquan]
> - Put the generic luks handling code before the x86 specific code to
>   make it easier for other arches to follow suit [Baoquan]
> - Use phys_to_virt instead of "pfn -> page -> vaddr" [Dave Hansen]
> - Drop the RFC prefix [Dave Young]
> - Rebase the code onto latest Linus tree (6.4.0-rc4)
>
>RFC v2
> - libcryptsetup interacts with the kernel via sysfs instead of "hacking"
>   dm-crypt
>   - to save a kdump copy of the LUKS volume key in 1st kernel
>   - to add a logon key using the copy for libcryptsetup in kdump kernel [Milan]
>   - to avoid the incorrect usage of LUKS master key in dm-crypt [Milan]
> - save the kdump copy of LUKS volume key randomly [Jan]
> - mark the kdump copy inaccessible [Pingfan]
> - Miscellaneous
>   - explain when operations related to the LUKS volume key happen [Jan]
>   - s/master key/volume key/g
>   - use crash_ instead of kexec_ as function prefix
>   - fix commit subject prefixes e.g. "x86, kdump" to x86/crash
>
>Coiby Xu (7):
>  kexec_file: allow to place kexec_buf randomly
>  crash_dump: make dm crypt keys persist for the kdump kernel
>  crash_dump: store dm crypt keys in kdump reserved memory
>  crash_dump: reuse saved dm crypt keys for CPU/memory hot-plugging
>  crash_dump: retrieve dm crypt keys in kdump kernel
>  x86/crash: pass dm crypt keys to kdump kernel
>  x86/crash: make the page that stores the dm crypt keys inaccessible
>
> Documentation/ABI/testing/crash_dm_crypt_keys |  35 ++
> arch/x86/kernel/crash.c                       |  20 +-
> arch/x86/kernel/kexec-bzimage64.c             |   7 +
> arch/x86/kernel/machine_kexec_64.c            |  22 ++
> include/linux/crash_core.h                    |   9 +-
> include/linux/crash_dump.h                    |   2 +
> include/linux/kexec.h                         |   8 +
> kernel/Kconfig.kexec                          |   9 +
> kernel/Makefile                               |   1 +
> kernel/crash_dump_dm_crypt.c                  | 338 ++++++++++++++++++
> kernel/kexec_file.c                           |  21 ++
> kernel/ksysfs.c                               |  24 ++
> 12 files changed, 493 insertions(+), 3 deletions(-)
> create mode 100644 Documentation/ABI/testing/crash_dm_crypt_keys
> create mode 100644 kernel/crash_dump_dm_crypt.c
>
>
>base-commit: 8a92980606e3585d72d510a03b59906e96755b8a
>-- 
>2.45.1
>

-- 
Best regards,
Coiby

Re: [PATCH v5 1/7] kexec_file: allow to place kexec_buf randomly

[kernel test robot]

Hi Coiby,

kernel test robot noticed the following build errors:

[auto build test ERROR on 8a92980606e3585d72d510a03b59906e96755b8a]

url:    https://github.com/intel-lab-lkp/linux/commits/Coiby-Xu/kexec_file-allow-to-place-kexec_buf-randomly/20240607-203154
base:   8a92980606e3585d72d510a03b59906e96755b8a
patch link:    https://lore.kernel.org/r/20240607122622.167228-2-coxu%40redhat.com
patch subject: [PATCH v5 1/7] kexec_file: allow to place kexec_buf randomly
config: parisc-allyesconfig (https://download.01.org/0day-ci/archive/20240608/202406081124.hYZYvBL0-lkp@intel.com/config)
compiler: hppa-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240608/202406081124.hYZYvBL0-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202406081124.hYZYvBL0-lkp@intel.com/

All errors (new ones prefixed by >>):

   kernel/kexec_file.c: In function 'locate_mem_hole_bottom_up':
>> kernel/kexec_file.c:504:17: error: 'struct kexec_buf' has no member named 'random'
     504 |         if (kbuf->random)
         |                 ^~
>> kernel/kexec_file.c:505:30: error: implicit declaration of function 'kexec_random_start' [-Werror=implicit-function-declaration]
     505 |                 temp_start = kexec_random_start(temp_start, end);
         |                              ^~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors


vim +504 kernel/kexec_file.c

   496	
   497	static int locate_mem_hole_bottom_up(unsigned long start, unsigned long end,
   498					     struct kexec_buf *kbuf)
   499	{
   500		struct kimage *image = kbuf->image;
   501		unsigned long temp_start, temp_end;
   502	
   503		temp_start = max(start, kbuf->buf_min);
 > 504		if (kbuf->random)
 > 505			temp_start = kexec_random_start(temp_start, end);
   506	
   507		do {
   508			temp_start = ALIGN(temp_start, kbuf->buf_align);
   509			temp_end = temp_start + kbuf->memsz - 1;
   510	
   511			if (temp_end > end || temp_end > kbuf->buf_max)
   512				return 0;
   513			/*
   514			 * Make sure this does not conflict with any of existing
   515			 * segments
   516			 */
   517			if (kimage_is_destination_range(image, temp_start, temp_end)) {
   518				temp_start = temp_start + PAGE_SIZE;
   519				continue;
   520			}
   521	
   522			/* We found a suitable memory range */
   523			break;
   524		} while (1);
   525	
   526		/* If we are here, we found a suitable memory range */
   527		kbuf->mem = temp_start;
   528	
   529		/* Success, stop navigating through remaining System RAM ranges */
   530		return 1;
   531	}
   532	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v5 1/7] kexec_file: allow to place kexec_buf randomly

[kernel test robot]

Hi Coiby,

kernel test robot noticed the following build errors:

[auto build test ERROR on 8a92980606e3585d72d510a03b59906e96755b8a]

url:    https://github.com/intel-lab-lkp/linux/commits/Coiby-Xu/kexec_file-allow-to-place-kexec_buf-randomly/20240607-203154
base:   8a92980606e3585d72d510a03b59906e96755b8a
patch link:    https://lore.kernel.org/r/20240607122622.167228-2-coxu%40redhat.com
patch subject: [PATCH v5 1/7] kexec_file: allow to place kexec_buf randomly
config: x86_64-kexec (https://download.01.org/0day-ci/archive/20240608/202406081214.kNbwlJWP-lkp@intel.com/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240608/202406081214.kNbwlJWP-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202406081214.kNbwlJWP-lkp@intel.com/

All errors (new ones prefixed by >>):

>> kernel/kexec_file.c:504:12: error: no member named 'random' in 'struct kexec_buf'
     504 |         if (kbuf->random)
         |             ~~~~  ^
>> kernel/kexec_file.c:505:16: error: call to undeclared function 'kexec_random_start'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     505 |                 temp_start = kexec_random_start(temp_start, end);
         |                              ^
   2 errors generated.


vim +504 kernel/kexec_file.c

   496	
   497	static int locate_mem_hole_bottom_up(unsigned long start, unsigned long end,
   498					     struct kexec_buf *kbuf)
   499	{
   500		struct kimage *image = kbuf->image;
   501		unsigned long temp_start, temp_end;
   502	
   503		temp_start = max(start, kbuf->buf_min);
 > 504		if (kbuf->random)
 > 505			temp_start = kexec_random_start(temp_start, end);
   506	
   507		do {
   508			temp_start = ALIGN(temp_start, kbuf->buf_align);
   509			temp_end = temp_start + kbuf->memsz - 1;
   510	
   511			if (temp_end > end || temp_end > kbuf->buf_max)
   512				return 0;
   513			/*
   514			 * Make sure this does not conflict with any of existing
   515			 * segments
   516			 */
   517			if (kimage_is_destination_range(image, temp_start, temp_end)) {
   518				temp_start = temp_start + PAGE_SIZE;
   519				continue;
   520			}
   521	
   522			/* We found a suitable memory range */
   523			break;
   524		} while (1);
   525	
   526		/* If we are here, we found a suitable memory range */
   527		kbuf->mem = temp_start;
   528	
   529		/* Success, stop navigating through remaining System RAM ranges */
   530		return 1;
   531	}
   532	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v4 0/7] Support kdump with LUKS encryption by reusing LUKS volume keys

[Greg KH]

On Sat, Jun 08, 2024 at 09:26:43AM +0800, Coiby Xu wrote:
> The subject prefix should be "[PATCH v5 0/7]". I'm sorry if it causes
> any confusion.

It will probably break our tools (b4) that want to look this up, so you
might want to fix that for the next version you send out, given that
this one has some 0-day bot issues.

thanks,

greg k-h

Re: [PATCH v5 1/7] kexec_file: allow to place kexec_buf randomly

[Greg KH]

On Fri, Jun 07, 2024 at 08:26:11PM +0800, Coiby Xu wrote:
> Currently, kexec_buf is placed in order which means for the same
> machine, the info in the kexec_buf is always located at the same
> position each time the machine is booted. This may cause a risk for
> sensitive information like LUKS volume key. Now struct kexec_buf has a
> new field random which indicates it's supposed to be placed in a random
> position.
> 
> Suggested-by: Jan Pazdziora <jpazdziora@redhat.com>
> Signed-off-by: Coiby Xu <coxu@redhat.com>
> ---
>  include/linux/kexec.h |  4 ++++
>  kernel/kexec_file.c   | 21 +++++++++++++++++++++
>  2 files changed, 25 insertions(+)
> 
> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> index f0e9f8eda7a3..c45bfc727737 100644
> --- a/include/linux/kexec.h
> +++ b/include/linux/kexec.h
> @@ -171,6 +171,7 @@ int kexec_image_post_load_cleanup_default(struct kimage *image);
>   * @buf_min:	The buffer can't be placed below this address.
>   * @buf_max:	The buffer can't be placed above this address.
>   * @top_down:	Allocate from top of memory.
> + * @random:	Place the buffer at a random position.
>   */
>  struct kexec_buf {
>  	struct kimage *image;
> @@ -182,6 +183,9 @@ struct kexec_buf {
>  	unsigned long buf_min;
>  	unsigned long buf_max;
>  	bool top_down;
> +#ifdef CONFIG_CRASH_DUMP
> +	bool random;
> +#endif

Why is the ifdef needed?

>  };
>  
>  int kexec_load_purgatory(struct kimage *image, struct kexec_buf *kbuf);
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> index 3d64290d24c9..f7538d8f67e0 100644
> --- a/kernel/kexec_file.c
> +++ b/kernel/kexec_file.c
> @@ -25,6 +25,9 @@
>  #include <linux/elfcore.h>
>  #include <linux/kernel.h>
>  #include <linux/kernel_read_file.h>
> +#ifdef CONFIG_CRASH_DUMP
> +#include <linux/prandom.h>
> +#endif

No ifdef in .c files please.  This should not be an issue.

>  #include <linux/syscalls.h>
>  #include <linux/vmalloc.h>
>  #include "kexec_internal.h"
> @@ -437,6 +440,18 @@ SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, int, initrd_fd,
>  	return ret;
>  }
>  
> +#ifdef CONFIG_CRASH_DUMP
> +static unsigned long kexec_random_start(unsigned long start, unsigned long end)
> +{
> +	unsigned long temp_start;
> +	unsigned short i;
> +
> +	get_random_bytes(&i, sizeof(unsigned short));
> +	temp_start = start + (end - start) / USHRT_MAX * i;
> +	return temp_start;
> +}
> +#endif

This #ifdef should be handled properly in the .h file.

> +
>  static int locate_mem_hole_top_down(unsigned long start, unsigned long end,
>  				    struct kexec_buf *kbuf)
>  {
> @@ -445,6 +460,10 @@ static int locate_mem_hole_top_down(unsigned long start, unsigned long end,
>  
>  	temp_end = min(end, kbuf->buf_max);
>  	temp_start = temp_end - kbuf->memsz + 1;
> +#ifdef CONFIG_CRASH_DUMP
> +	if (kbuf->random)
> +		temp_start = kexec_random_start(temp_start, temp_end);
> +#endif

Same with this.

And why do you need the boolean at all, why not just have
kexec_random_start() handle this properly for you?

thanks,

greg k-h

Re: [PATCH v5 2/7] crash_dump: make dm crypt keys persist for the kdump kernel

[Greg KH]

On Fri, Jun 07, 2024 at 08:26:12PM +0800, Coiby Xu wrote:
> A sysfs /sys/kernel/crash_dm_crypt_keys is provided for user space to
> make the dm crypt keys persist for the kdump kernel. Take the case of
> dumping to a LUKS-encrypted target as an example, here is the life cycle
> of this kdump copy of LUKS volume keys,
> 
>  1. After the 1st kernel loads the initramfs during boot, systemd
>     use an user-input passphrase or TPM-sealed key to de-crypt the LUKS
>     volume keys and then save the volume keys to specified keyring
>     (using the --link-vk-to-keyring API) and the key will expire within
>     specified time.
> 
>  2. A user space tool (kdump initramfs builder) writes a key description to
>     /sys/kernel/crash_dm_crypt_keys to inform the 1st kernel to record the
>     key while building the kdump initramfs
> 
>  3. The kexec_file_load syscall read the volume keys by recorded key
>     descriptions and then save them key to kdump reserved memory and wipe the
>     copy.
> 
>  4. When the 1st kernel crashes and the kdump initramfs is booted, the kdump
>     initramfs asks the kdump kernel to create a user key using the key stored in
>     kdump reserved memory by writing to to /sys/kernel/crash_dm_crypt_keys. Then
>     the LUKS encrypted devide is unlocked with libcryptsetup's
>     --volume-key-keyring API.
> 
>  5. The system gets rebooted to the 1st kernel after dumping vmcore to
>     the LUKS encrypted device is finished
> 
> I assume 1) there are 128 LUKS devices at maximum to be unlocked thus
> MAX_KEY_NUM=128 2) and a key won't exceed 256 bytes thus
> MAX_KEY_SIZE=256 according to "cryptsetup benchmark".
> 
> For details on usage of the API, please check the new doc file
> Documentation/ABI/testing/crash_dm_crypt_keys.
> 
> Cc: Greg KH <gregkh@linuxfoundation.org>
> Signed-off-by: Coiby Xu <coxu@redhat.com>
> ---
>  Documentation/ABI/testing/crash_dm_crypt_keys |  30 ++++
>  include/linux/crash_core.h                    |   5 +-
>  kernel/Kconfig.kexec                          |   9 ++
>  kernel/Makefile                               |   1 +
>  kernel/crash_dump_dm_crypt.c                  | 130 ++++++++++++++++++
>  kernel/ksysfs.c                               |  24 ++++
>  6 files changed, 198 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/ABI/testing/crash_dm_crypt_keys
>  create mode 100644 kernel/crash_dump_dm_crypt.c
> 
> diff --git a/Documentation/ABI/testing/crash_dm_crypt_keys b/Documentation/ABI/testing/crash_dm_crypt_keys
> new file mode 100644
> index 000000000000..e6a6f6be5a9e
> --- /dev/null
> +++ b/Documentation/ABI/testing/crash_dm_crypt_keys
> @@ -0,0 +1,30 @@
> +What:		/sys/kernel/crash_dm_crypt_keys
> +Date:		Jun 2024
> +KernelVersion:	6.11
> +Contact:	kexec@lists.infradead.org
> +Description:	read/write
> +    Make dm crypt keys persistent for the kdump kernel.

Tabs please.

> +
> +    Assume the key size won't exceed 256 bytes and the maximum number of keys is 128.

Please wrap the documentation lines.

And who makes this assumption?  The kernel?  Userspace?  Something else?
Who enforces it?  What happens if it is not true?

> +
> +    You can write the following commands before kexec'ing the kdump kernel,
> +      - "init KEY_NUM"
> +        Let the kernel know the number of dm crypt keys so it will initialize
> +        needed structures. KEY_NUM=128 dm crypt keys at maximum.
> +        - "record KEY_DESC"
> +        Record a key description. For security reason, the key must be a logon
> +        key whose payload can't be read by user space. For details, please refer
> +        to security/keys/core.rst.
> +
> +    And you can also read this API to know the command eructation status,
> +      - fresh
> +        Waiting for a command
> +      - initialized
> +        The "init KEY_NUM" command has been executed successfully
> +      - recorded
> +        Specified number of keys have been recorded
> +      - loaded
> +        the kdump kernel has been loaded with the dm crypt keys stored to kdump
> +        reserved memory

I do not understand, this is a command that you write to this file?
That's not how sysfs files work at all.

And this isn't really very well documented, I do not understand if this
is read/write or just read or just write or a mix?  And what are the
valid write values?  Valid read?  Why "init" vs. "initialized"?



> +
> +User:		Kdump service
> diff --git a/include/linux/crash_core.h b/include/linux/crash_core.h
> index 44305336314e..6bff1c24efa3 100644
> --- a/include/linux/crash_core.h
> +++ b/include/linux/crash_core.h
> @@ -34,7 +34,10 @@ static inline void arch_kexec_protect_crashkres(void) { }
>  static inline void arch_kexec_unprotect_crashkres(void) { }
>  #endif
>  
> -
> +#ifdef CONFIG_CRASH_DM_CRYPT
> +int crash_sysfs_dm_crypt_keys_read(char *buf);
> +int crash_sysfs_dm_crypt_keys_write(const char *buf, size_t count);
> +#endif

No versions for when this is not enabled?

>  
>  #ifndef arch_crash_handle_hotplug_event
>  static inline void arch_crash_handle_hotplug_event(struct kimage *image, void *arg) { }
> diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
> index 6c34e63c88ff..d067ba252163 100644
> --- a/kernel/Kconfig.kexec
> +++ b/kernel/Kconfig.kexec
> @@ -116,6 +116,15 @@ config CRASH_DUMP
>  	  For s390, this option also enables zfcpdump.
>  	  See also <file:Documentation/arch/s390/zfcpdump.rst>
>  
> +config CRASH_DM_CRYPT
> +	bool "Support saving crash dump to dm-crypt encrypted volume"
> +	depends on CRASH_DUMP
> +	depends on DM_CRYPT
> +	help
> +	  With this option enabled, user space can intereact with
> +	  /sys/kernel/crash_dm_crypt_keys to make the dm crypt keys
> +	  persistent for the crash dump kernel.
> +
>  config CRASH_HOTPLUG
>  	bool "Update the crash elfcorehdr on system configuration changes"
>  	default y
> diff --git a/kernel/Makefile b/kernel/Makefile
> index 3c13240dfc9f..f2e5b3e86d12 100644
> --- a/kernel/Makefile
> +++ b/kernel/Makefile
> @@ -72,6 +72,7 @@ obj-$(CONFIG_VMCORE_INFO) += vmcore_info.o elfcorehdr.o
>  obj-$(CONFIG_CRASH_RESERVE) += crash_reserve.o
>  obj-$(CONFIG_KEXEC_CORE) += kexec_core.o
>  obj-$(CONFIG_CRASH_DUMP) += crash_core.o
> +obj-$(CONFIG_CRASH_DM_CRYPT) += crash_dump_dm_crypt.o
>  obj-$(CONFIG_KEXEC) += kexec.o
>  obj-$(CONFIG_KEXEC_FILE) += kexec_file.o
>  obj-$(CONFIG_KEXEC_ELF) += kexec_elf.o
> diff --git a/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c
> new file mode 100644
> index 000000000000..608bde3aaa8e
> --- /dev/null
> +++ b/kernel/crash_dump_dm_crypt.c
> @@ -0,0 +1,130 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +#include <keys/user-type.h>
> +#include <linux/crash_dump.h>
> +
> +#define KEY_NUM_MAX 128   /* maximum dm crypt keys */
> +#define KEY_SIZE_MAX 256  /* maximum dm crypt key size */

Tabs please.


> +
> +// The key scription has the format: cryptsetup:UUID 11+36+1(NULL)=48
> +#define KEY_DESC_LEN 48

"scription"?  And is this a string length?  Why not use the in-kernel
uuid code if you are dealing with uuid values?

> +
> +static enum STATE_ENUM {
> +	FRESH = 0,
> +	INITIALIZED,
> +	RECORDED,
> +	LOADED,
> +} state;
> +
> +static const char * const STATE_STR[] = {
> +	[FRESH] = "fresh",
> +	[INITIALIZED] = "initialized",
> +	[RECORDED] = "recorded",
> +	[LOADED] = "loaded"
> +};
> +
> +static unsigned int key_count;
> +static size_t keys_header_size;

So all of the keys have a single header size?

> +
> +struct dm_crypt_key {
> +	unsigned int key_size;
> +	char key_desc[KEY_DESC_LEN];
> +	u8 data[KEY_SIZE_MAX];
> +};
> +
> +static struct keys_header {
> +	unsigned int total_keys;
> +	struct dm_crypt_key keys[] __counted_by(total_keys);
> +} *keys_header;
> +
> +static size_t get_keys_header_size(struct keys_header *keys_header,
> +				   size_t total_keys)
> +{
> +	return struct_size(keys_header, keys, total_keys);
> +}
> +
> +/*
> + * Let the kernel know the number of dm crypt keys and allocate memory to
> + * initialize related structures.
> + */
> +static int init(const char *buf)
> +{
> +	unsigned int total_keys;
> +
> +	if (sscanf(buf, "init %u", &total_keys) != 1)
> +		return -EINVAL;
> +
> +	if (total_keys > KEY_NUM_MAX) {
> +		kexec_dprintk(
> +			"Exceed the maximum number of keys (KEY_NUM_MAX=%u)\n",
> +			KEY_NUM_MAX);
> +		return -EINVAL;
> +	}
> +
> +	keys_header_size = get_keys_header_size(keys_header, total_keys);
> +	key_count = 0;
> +
> +	if (keys_header != NULL)
> +		kvfree(keys_header);
> +
> +	keys_header = kzalloc(keys_header_size, GFP_KERNEL);
> +	if (!keys_header)
> +		return -ENOMEM;
> +
> +	keys_header->total_keys = total_keys;
> +	state = INITIALIZED;
> +	return 0;
> +}
> +
> +/*
> + * Record the key description of a dm crypt key.
> + */
> +static int record_key_desc(const char *buf, struct dm_crypt_key *dm_key)
> +{
> +	char key_desc[KEY_DESC_LEN];
> +
> +	if (state != INITIALIZED) {
> +		kexec_dprintk("Please send the cmd 'init <KEY_NUM>' first\n");
> +		return -EINVAL;
> +	}
> +
> +	if (sscanf(buf, "record %s", key_desc) != 1)
> +		return -EINVAL;
> +
> +	if (key_count >= keys_header->total_keys) {
> +		kexec_dprintk("Already have %u keys", key_count);
> +		return -EINVAL;
> +	}
> +
> +	strscpy(dm_key->key_desc, key_desc, KEY_DESC_LEN);
> +	kexec_dprintk("Key%d (%s) recorded\n", key_count, dm_key->key_desc);
> +	key_count++;
> +
> +	if (key_count == keys_header->total_keys)
> +		state = RECORDED;
> +
> +	return 0;
> +}
> +
> +static int process_cmd(const char *buf, size_t count)
> +{
> +	if (strncmp(buf, "init ", 5) == 0)
> +		return init(buf);
> +	else if (strncmp(buf, "record ", 7) == 0 && count == KEY_DESC_LEN + 6)
> +		return record_key_desc(buf, &keys_header->keys[key_count]);

So you are parsing the "init" and "record" strings twice?  That feels
wrong, and might get out of sync easily.

But again, having a sysfs file that does "actions" like this is odd too,
as you have multiple things in the same string.  sysfs is "one value per
file", why aren't you using configfs instead?  That's meant for
configuring stuff, not sysfs.


> +
> +	return -EINVAL;
> +}
> +
> +int crash_sysfs_dm_crypt_keys_write(const char *buf, size_t count)
> +{
> +	if (!is_kdump_kernel())
> +		return process_cmd(buf, count);
> +	return -EINVAL;
> +}
> +EXPORT_SYMBOL_GPL(crash_sysfs_dm_crypt_keys_write);
> +
> +int crash_sysfs_dm_crypt_keys_read(char *buf)
> +{
> +	return sysfs_emit(buf, "%s\n", STATE_STR[state]);
> +}
> +EXPORT_SYMBOL_GPL(crash_sysfs_dm_crypt_keys_read);
> diff --git a/kernel/ksysfs.c b/kernel/ksysfs.c
> index 07fb5987b42b..ff7caef30f51 100644
> --- a/kernel/ksysfs.c
> +++ b/kernel/ksysfs.c
> @@ -167,6 +167,27 @@ static ssize_t vmcoreinfo_show(struct kobject *kobj,
>  }
>  KERNEL_ATTR_RO(vmcoreinfo);
>  
> +#ifdef CONFIG_CRASH_DM_CRYPT
> +static ssize_t crash_dm_crypt_keys_show(struct kobject *kobj,
> +					struct kobj_attribute *attr, char *buf)
> +{
> +	return crash_sysfs_dm_crypt_keys_read(buf);
> +}
> +
> +static ssize_t crash_dm_crypt_keys_store(struct kobject *kobj,
> +					 struct kobj_attribute *attr,
> +					 const char *buf, size_t count)
> +{
> +	int ret;
> +
> +	ret = crash_sysfs_dm_crypt_keys_write(buf, count);
> +	if (ret < 0)
> +		return ret;
> +	return count;

Why doesn't crash_sysfs_dm_crypt_keys_write just return the count value
processed?  No need to check the return value again.

thanks

greg k-h