Kexec Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/5] Support kdump with LUKS encryption by reusing LUKS volume key
@ 2024-01-10  7:15 Coiby Xu
  2024-01-10  7:15 ` [PATCH v2 1/5] kexec_file: allow to place kexec_buf randomly Coiby Xu
                   ` (5 more replies)
  0 siblings, 6 replies; 14+ messages in thread
From: Coiby Xu @ 2024-01-10  7:15 UTC (permalink / raw)
  To: kexec
  Cc: Ondrej Kozina, Milan Broz, Thomas Staudt,
	Daniel P . Berrangé, Kairui Song, dm-devel, Jan Pazdziora,
	Pingfan Liu, Baoquan He, Dave Young, linux-kernel, x86,
	Dave Hansen, Vitaly Kuznetsov

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 key 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 key again in kdump kernel which seems to be
redundant work.

This patch set addresses the above issues by reusing the LUKS volume key
in 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 key,

 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 key and then save the volume key 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 the key description to
    /sys/kernel/crash_dm_crypt_key to inform the 1st kernel to save a
    temporary copy of the volume key while building the kdump initramfs

 3. The kexec_file_load syscall saves the temporary copy of the volume
    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 the key
    description to /sys/kernel/crash_dm_crypt_key. 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 key to specified keyring,
whoever takes this should be responsible for the safety of this copy
of key. This key 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.

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 (5):
  kexec_file: allow to place kexec_buf randomly
  crash_dump: save the dm crypt key temporarily
  crash_dump: retrieve dm crypt key in kdump kernel
  x86/crash: pass the dm crypt key to kdump kernel
  x86/crash: make the page that stores the dm crypt key inaccessible

 arch/x86/kernel/crash.c            |  15 +-
 arch/x86/kernel/kexec-bzimage64.c  |   7 +
 arch/x86/kernel/machine_kexec_64.c |  18 +++
 include/linux/crash_core.h         |   7 +-
 include/linux/crash_dump.h         |   2 +
 include/linux/kexec.h              |   6 +
 kernel/Makefile                    |   2 +-
 kernel/crash_dump_dm_crypt.c       | 234 +++++++++++++++++++++++++++++
 kernel/kexec_file.c                |  15 ++
 kernel/ksysfs.c                    |  23 ++-
 10 files changed, 324 insertions(+), 5 deletions(-)
 create mode 100644 kernel/crash_dump_dm_crypt.c

-- 
2.43.0


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

* [PATCH v2 1/5] kexec_file: allow to place kexec_buf randomly
  2024-01-10  7:15 [PATCH v2 0/5] Support kdump with LUKS encryption by reusing LUKS volume key Coiby Xu
@ 2024-01-10  7:15 ` Coiby Xu
  2024-01-10  7:15 ` [PATCH v2 2/5] crash_dump: save the dm crypt key temporarily Coiby Xu
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 14+ messages in thread
From: Coiby Xu @ 2024-01-10  7:15 UTC (permalink / raw)
  To: kexec
  Cc: Ondrej Kozina, Milan Broz, Thomas Staudt,
	Daniel P . Berrangé, Kairui Song, dm-devel, Jan Pazdziora,
	Pingfan Liu, Baoquan He, Dave Young, linux-kernel, x86,
	Dave Hansen, Vitaly Kuznetsov, Eric Biederman

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 |  2 ++
 kernel/kexec_file.c   | 15 +++++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index 8227455192b7..6f4626490ebf 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -169,6 +169,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;
@@ -180,6 +181,7 @@ struct kexec_buf {
 	unsigned long buf_min;
 	unsigned long buf_max;
 	bool top_down;
+	bool random;
 };
 
 int kexec_load_purgatory(struct kimage *image, struct kexec_buf *kbuf);
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index f9a419cd22d4..7abcfc3c8491 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -25,6 +25,7 @@
 #include <linux/elfcore.h>
 #include <linux/kernel.h>
 #include <linux/kernel_read_file.h>
+#include <linux/prandom.h>
 #include <linux/syscalls.h>
 #include <linux/vmalloc.h>
 #include "kexec_internal.h"
@@ -419,6 +420,16 @@ SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, int, initrd_fd,
 	return ret;
 }
 
+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;
+}
+
 static int locate_mem_hole_top_down(unsigned long start, unsigned long end,
 				    struct kexec_buf *kbuf)
 {
@@ -427,6 +438,8 @@ 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;
+	if (kbuf->random)
+		temp_start = kexec_random_start(temp_start, temp_end);
 
 	do {
 		/* align down start */
@@ -464,6 +477,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.43.0


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

* [PATCH v2 2/5] crash_dump: save the dm crypt key temporarily
  2024-01-10  7:15 [PATCH v2 0/5] Support kdump with LUKS encryption by reusing LUKS volume key Coiby Xu
  2024-01-10  7:15 ` [PATCH v2 1/5] kexec_file: allow to place kexec_buf randomly Coiby Xu
@ 2024-01-10  7:15 ` Coiby Xu
  2024-01-13 17:07   ` kernel test robot
                     ` (3 more replies)
  2024-01-10  7:15 ` [PATCH v2 3/5] crash_dump: retrieve dm crypt key in kdump kernel Coiby Xu
                   ` (3 subsequent siblings)
  5 siblings, 4 replies; 14+ messages in thread
From: Coiby Xu @ 2024-01-10  7:15 UTC (permalink / raw)
  To: kexec
  Cc: Ondrej Kozina, Milan Broz, Thomas Staudt,
	Daniel P . Berrangé, Kairui Song, dm-devel, Jan Pazdziora,
	Pingfan Liu, Baoquan He, Dave Young, linux-kernel, x86,
	Dave Hansen, Vitaly Kuznetsov, Vivek Goyal, Eric Biederman

User space is supposed to write the key description to
/sys/kernel/crash_dm_crypt_key so the kernel will read the key and save
a temporary copy for later user. User space has 2 minutes at maximum to
load the kdump initrd before the key gets wiped. And after kdump
retrieves the key, the key will be wiped immediately.

Signed-off-by: Coiby Xu <coxu@redhat.com>
---
 include/linux/crash_core.h   |   7 +-
 include/linux/kexec.h        |   4 ++
 kernel/Makefile              |   2 +-
 kernel/crash_dump_dm_crypt.c | 121 +++++++++++++++++++++++++++++++++++
 kernel/ksysfs.c              |  23 ++++++-
 5 files changed, 153 insertions(+), 4 deletions(-)
 create mode 100644 kernel/crash_dump_dm_crypt.c

diff --git a/include/linux/crash_core.h b/include/linux/crash_core.h
index 5126a4fecb44..7078eda6418d 100644
--- a/include/linux/crash_core.h
+++ b/include/linux/crash_core.h
@@ -125,6 +125,12 @@ static inline void __init reserve_crashkernel_generic(char *cmdline,
 {}
 #endif
 
+struct kimage;
+
+int crash_sysfs_dm_crypt_key_write(const char *key_des, size_t count);
+int crash_pass_temp_dm_crypt_key(void **addr, unsigned long *sz);
+int crash_load_dm_crypt_key(struct kimage *image);
+
 /* Alignment required for elf header segment */
 #define ELF_CORE_HEADER_ALIGN   4096
 
@@ -140,7 +146,6 @@ extern int crash_exclude_mem_range(struct crash_mem *mem,
 extern int crash_prepare_elf64_headers(struct crash_mem *mem, int need_kernel_map,
 				       void **addr, unsigned long *sz);
 
-struct kimage;
 struct kexec_segment;
 
 #define KEXEC_CRASH_HP_NONE			0
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index 6f4626490ebf..bf7ab1e927ef 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -366,6 +366,10 @@ struct kimage {
 	void *elf_headers;
 	unsigned long elf_headers_sz;
 	unsigned long elf_load_addr;
+
+	/* dm crypt key buffer */
+	unsigned long dm_crypt_key_addr;
+	unsigned long dm_crypt_key_sz;
 };
 
 /* kexec interface functions */
diff --git a/kernel/Makefile b/kernel/Makefile
index 3947122d618b..48859bf63db5 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -119,7 +119,7 @@ obj-$(CONFIG_PERF_EVENTS) += events/
 
 obj-$(CONFIG_USER_RETURN_NOTIFIER) += user-return-notifier.o
 obj-$(CONFIG_PADATA) += padata.o
-obj-$(CONFIG_CRASH_DUMP) += crash_dump.o
+obj-$(CONFIG_CRASH_DUMP) += crash_dump.o crash_dump_dm_crypt.o
 obj-$(CONFIG_JUMP_LABEL) += jump_label.o
 obj-$(CONFIG_CONTEXT_TRACKING) += context_tracking.o
 obj-$(CONFIG_TORTURE_TEST) += torture.o
diff --git a/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c
new file mode 100644
index 000000000000..3a0b0b773598
--- /dev/null
+++ b/kernel/crash_dump_dm_crypt.c
@@ -0,0 +1,121 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <keys/user-type.h>
+#include <linux/crash_dump.h>
+
+static u8 *dm_crypt_key;
+static unsigned int dm_crypt_key_size;
+
+void wipe_dm_crypt_key(void)
+{
+	if (dm_crypt_key) {
+		memset(dm_crypt_key, 0, dm_crypt_key_size * sizeof(u8));
+		kfree(dm_crypt_key);
+		dm_crypt_key = NULL;
+	}
+}
+
+static void _wipe_dm_crypt_key(struct work_struct *dummy)
+{
+	wipe_dm_crypt_key();
+}
+
+static DECLARE_DELAYED_WORK(wipe_dm_crypt_key_work, _wipe_dm_crypt_key);
+
+static unsigned __read_mostly wipe_key_delay = 120; /* 2 mins */
+
+static int crash_save_temp_dm_crypt_key(const char *key_desc, size_t count)
+{
+	const struct user_key_payload *ukp;
+	struct key *key;
+
+	if (dm_crypt_key) {
+		memset(dm_crypt_key, 0, dm_crypt_key_size * sizeof(u8));
+		kfree(dm_crypt_key);
+	}
+
+	pr_debug("Requesting key %s", key_desc);
+	key = request_key(&key_type_user, key_desc, NULL);
+
+	if (IS_ERR(key)) {
+		pr_debug("No such key %s", key_desc);
+		return PTR_ERR(key);
+	}
+
+	ukp = user_key_payload_locked(key);
+	if (!ukp)
+		return -EKEYREVOKED;
+
+	dm_crypt_key = kmalloc(ukp->datalen, GFP_KERNEL);
+	if (!dm_crypt_key)
+		return -ENOMEM;
+	memcpy(dm_crypt_key, ukp->data, ukp->datalen);
+	dm_crypt_key_size = ukp->datalen;
+	pr_debug("dm crypt key (size=%u): %8ph\n", dm_crypt_key_size, dm_crypt_key);
+	schedule_delayed_work(&wipe_dm_crypt_key_work,
+			      round_jiffies_relative(wipe_key_delay * HZ));
+	return 0;
+}
+
+int crash_sysfs_dm_crypt_key_write(const char *key_desc, size_t count)
+{
+	if (!is_kdump_kernel())
+		return crash_save_temp_dm_crypt_key(key_desc, count);
+	return -EINVAL;
+}
+EXPORT_SYMBOL(crash_sysfs_dm_crypt_key_write);
+
+int crash_pass_temp_dm_crypt_key(void **addr, unsigned long *sz)
+{
+	unsigned long dm_crypt_key_sz;
+	unsigned char *buf;
+	unsigned int *size_ptr;
+
+	if (!dm_crypt_key)
+		return -EINVAL;
+
+	dm_crypt_key_sz = sizeof(unsigned int) + dm_crypt_key_size * sizeof(u8);
+
+	buf = vzalloc(dm_crypt_key_sz);
+	if (!buf)
+		return -ENOMEM;
+
+	size_ptr = (unsigned int *)buf;
+	memcpy(size_ptr, &dm_crypt_key_size, sizeof(unsigned int));
+	memcpy(size_ptr + 1, dm_crypt_key, dm_crypt_key_size * sizeof(u8));
+	*addr = buf;
+	*sz = dm_crypt_key_sz;
+	wipe_dm_crypt_key();
+	return 0;
+}
+
+int crash_load_dm_crypt_key(struct kimage *image)
+{
+	int ret;
+	struct kexec_buf kbuf = {
+		.image = image,
+		.buf_min = 0,
+		.buf_max = ULONG_MAX,
+		.top_down = false,
+		.random = true,
+	};
+
+	image->dm_crypt_key_addr = 0;
+	ret = crash_pass_temp_dm_crypt_key(&kbuf.buffer, &kbuf.bufsz);
+	if (ret)
+		return ret;
+
+	kbuf.memsz = kbuf.bufsz;
+	kbuf.buf_align = ELF_CORE_HEADER_ALIGN;
+	kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
+	ret = kexec_add_buffer(&kbuf);
+	if (ret) {
+		vfree((void *)kbuf.buffer);
+		return ret;
+	}
+	image->dm_crypt_key_addr = kbuf.mem;
+	image->dm_crypt_key_sz = kbuf.bufsz;
+	pr_debug("Loaded dm crypt key at 0x%lx bufsz=0x%lx memsz=0x%lx\n",
+		 image->dm_crypt_key_addr, kbuf.bufsz, kbuf.bufsz);
+
+	return ret;
+}
diff --git a/kernel/ksysfs.c b/kernel/ksysfs.c
index 1d4bc493b2f4..f3bb6bc6a604 100644
--- a/kernel/ksysfs.c
+++ b/kernel/ksysfs.c
@@ -165,16 +165,34 @@ static ssize_t vmcoreinfo_show(struct kobject *kobj,
 }
 KERNEL_ATTR_RO(vmcoreinfo);
 
+static ssize_t crash_dm_crypt_key_show(struct kobject *kobj,
+					  struct kobj_attribute *attr,
+					  char *buf)
+{
+	return 0;
+}
+
+static ssize_t crash_dm_crypt_key_store(struct kobject *kobj,
+					struct kobj_attribute *attr,
+					   const char *buf, size_t count)
+{
+	int ret;
+
+	ret = crash_sysfs_dm_crypt_key_write(buf, count);
+	return ret < 0 ? ret : count;
+}
+KERNEL_ATTR_RW(crash_dm_crypt_key);
+
 #ifdef CONFIG_CRASH_HOTPLUG
 static ssize_t crash_elfcorehdr_size_show(struct kobject *kobj,
-			       struct kobj_attribute *attr, char *buf)
+					  struct kobj_attribute *attr,
+					  char *buf)
 {
 	unsigned int sz = crash_get_elfcorehdr_size();
 
 	return sysfs_emit(buf, "%u\n", sz);
 }
 KERNEL_ATTR_RO(crash_elfcorehdr_size);
-
 #endif
 
 #endif /* CONFIG_CRASH_CORE */
@@ -267,6 +285,7 @@ static struct attribute * kernel_attrs[] = {
 #endif
 #ifdef CONFIG_CRASH_CORE
 	&vmcoreinfo_attr.attr,
+	&crash_dm_crypt_key_attr.attr,
 #ifdef CONFIG_CRASH_HOTPLUG
 	&crash_elfcorehdr_size_attr.attr,
 #endif
-- 
2.43.0


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

* [PATCH v2 3/5] crash_dump: retrieve dm crypt key in kdump kernel
  2024-01-10  7:15 [PATCH v2 0/5] Support kdump with LUKS encryption by reusing LUKS volume key Coiby Xu
  2024-01-10  7:15 ` [PATCH v2 1/5] kexec_file: allow to place kexec_buf randomly Coiby Xu
  2024-01-10  7:15 ` [PATCH v2 2/5] crash_dump: save the dm crypt key temporarily Coiby Xu
@ 2024-01-10  7:15 ` Coiby Xu
  2024-01-13 18:45   ` kernel test robot
  2024-01-10  7:15 ` [PATCH v2 4/5] x86/crash: pass the dm crypt key to " Coiby Xu
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Coiby Xu @ 2024-01-10  7:15 UTC (permalink / raw)
  To: kexec
  Cc: Ondrej Kozina, Milan Broz, Thomas Staudt,
	Daniel P . Berrangé, Kairui Song, dm-devel, Jan Pazdziora,
	Pingfan Liu, Baoquan He, Dave Young, linux-kernel, x86,
	Dave Hansen, Vitaly Kuznetsov, Vivek Goyal

Crash kernel will retrieve the dm crypt volume key based on the
dmcryptkey command line parameter. When user space writes the key
description to /sys/kernel/crash_dm_crypt_key, the crash kernel will
save the encryption key 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>
---
 include/linux/crash_dump.h   |   2 +
 kernel/crash_dump_dm_crypt.c | 115 ++++++++++++++++++++++++++++++++++-
 2 files changed, 116 insertions(+), 1 deletion(-)

diff --git a/include/linux/crash_dump.h b/include/linux/crash_dump.h
index acc55626afdc..b44adc3962da 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 luks_volume_key_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/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c
index 3a0b0b773598..755017fa5c1b 100644
--- a/kernel/crash_dump_dm_crypt.c
+++ b/kernel/crash_dump_dm_crypt.c
@@ -1,7 +1,82 @@
 // 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>
 
+unsigned long long dm_crypt_key_addr;
+EXPORT_SYMBOL_GPL(dm_crypt_key_addr);
+
+static int __init setup_dmcryptkey(char *arg)
+{
+	char *end;
+
+	if (!arg)
+		return -EINVAL;
+	dm_crypt_key_addr = memparse(arg, &end);
+	if (end > arg)
+		return 0;
+
+	dm_crypt_key_addr = 0;
+	return -EINVAL;
+}
+
+early_param("dmcryptkey", setup_dmcryptkey);
+
+/*
+ * Architectures may override this function to read dm crypt key
+ */
+ssize_t __weak dm_crypt_key_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 retrive_kdump_dm_crypt_key(u8 *buffer, unsigned int *sz)
+{
+	unsigned int key_size;
+	size_t dm_crypt_keybuf_sz;
+	unsigned int *size_ptr;
+	char *dm_crypt_keybuf;
+	u64 addr;
+	int r;
+
+	if (dm_crypt_key_addr == 0) {
+		pr_debug("dm crypt key memory address inaccessible");
+		return -EINVAL;
+	}
+
+	addr = dm_crypt_key_addr;
+
+	/* Read dm crypt key size */
+	r = dm_crypt_key_read((char *)&key_size, sizeof(unsigned int), &addr);
+
+	if (r < 0)
+		return r;
+
+	pr_debug("Retrieve dm crypt key: size=%u\n", key_size);
+	/* Read in dm cryptrkey */
+	dm_crypt_keybuf_sz = sizeof(unsigned int) + key_size * sizeof(u8);
+	dm_crypt_keybuf = (void *)__get_free_pages(GFP_KERNEL | __GFP_ZERO,
+					      get_order(dm_crypt_keybuf_sz));
+	if (!dm_crypt_keybuf)
+		return -ENOMEM;
+
+	addr = dm_crypt_key_addr;
+	r = dm_crypt_key_read((char *)dm_crypt_keybuf, dm_crypt_keybuf_sz, &addr);
+
+	if (r < 0)
+		return r;
+	size_ptr = (unsigned int *)dm_crypt_keybuf;
+	memcpy(buffer, size_ptr + 1, key_size * sizeof(u8));
+	pr_debug("Retrieve dm crypt key (size=%u): %48ph...\n", key_size, buffer);
+	*sz = key_size;
+	return 0;
+}
+
 static u8 *dm_crypt_key;
 static unsigned int dm_crypt_key_size;
 
@@ -23,6 +98,43 @@ static DECLARE_DELAYED_WORK(wipe_dm_crypt_key_work, _wipe_dm_crypt_key);
 
 static unsigned __read_mostly wipe_key_delay = 120; /* 2 mins */
 
+static int retore_dm_crypt_key_to_thread_keyring(const char *key_desc)
+{
+	key_ref_t keyring_ref, key_ref;
+	int ret;
+
+	/* 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)) {
+		pr_alert("Failed to get keyring");
+		return PTR_ERR(keyring_ref);
+	}
+
+	dm_crypt_key = kmalloc(128, GFP_KERNEL);
+	ret = retrive_kdump_dm_crypt_key(dm_crypt_key, &dm_crypt_key_size);
+	if (ret) {
+		kfree(dm_crypt_key);
+		return ret;
+	}
+
+	/* create or update the requested key and add it to the target keyring */
+	key_ref = key_create_or_update(keyring_ref, "user", key_desc,
+				       dm_crypt_key, dm_crypt_key_size,
+				       KEY_USR_ALL, KEY_ALLOC_IN_QUOTA);
+
+	if (!IS_ERR(key_ref)) {
+		ret = key_ref_to_ptr(key_ref)->serial;
+		key_ref_put(key_ref);
+		pr_alert("Success adding key %s", key_desc);
+	} else {
+		ret = PTR_ERR(key_ref);
+		pr_alert("Error when adding key");
+	}
+
+	key_ref_put(keyring_ref);
+	return ret;
+}
+
 static int crash_save_temp_dm_crypt_key(const char *key_desc, size_t count)
 {
 	const struct user_key_payload *ukp;
@@ -60,7 +172,8 @@ int crash_sysfs_dm_crypt_key_write(const char *key_desc, size_t count)
 {
 	if (!is_kdump_kernel())
 		return crash_save_temp_dm_crypt_key(key_desc, count);
-	return -EINVAL;
+	else
+		return retore_dm_crypt_key_to_thread_keyring(key_desc);
 }
 EXPORT_SYMBOL(crash_sysfs_dm_crypt_key_write);
 
-- 
2.43.0


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

* [PATCH v2 4/5] x86/crash: pass the dm crypt key to kdump kernel
  2024-01-10  7:15 [PATCH v2 0/5] Support kdump with LUKS encryption by reusing LUKS volume key Coiby Xu
                   ` (2 preceding siblings ...)
  2024-01-10  7:15 ` [PATCH v2 3/5] crash_dump: retrieve dm crypt key in kdump kernel Coiby Xu
@ 2024-01-10  7:15 ` Coiby Xu
  2024-01-10  7:15 ` [PATCH v2 5/5] x86/crash: make the page that stores the dm crypt key inaccessible Coiby Xu
  2024-01-16 10:37 ` [PATCH v2 0/5] Support kdump with LUKS encryption by reusing LUKS volume key Ondrej Kozina
  5 siblings, 0 replies; 14+ messages in thread
From: Coiby Xu @ 2024-01-10  7:15 UTC (permalink / raw)
  To: kexec
  Cc: Ondrej Kozina, Milan Broz, Thomas Staudt,
	Daniel P . Berrangé, Kairui Song, dm-devel, Jan Pazdziora,
	Pingfan Liu, Baoquan He, Dave Young, linux-kernel, x86,
	Dave Hansen, Vitaly Kuznetsov, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Dave Hansen, H. Peter Anvin

1st kernel will build up the kernel command parameter dmcryptkey 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           | 15 ++++++++++++++-
 arch/x86/kernel/kexec-bzimage64.c |  7 +++++++
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
index c92d88680dbf..69e1090f01bc 100644
--- a/arch/x86/kernel/crash.c
+++ b/arch/x86/kernel/crash.c
@@ -262,6 +262,7 @@ static int memmap_exclude_ranges(struct kimage *image, struct crash_mem *cmem,
 				 unsigned long long mend)
 {
 	unsigned long start, end;
+	int r;
 
 	cmem->ranges[0].start = mstart;
 	cmem->ranges[0].end = mend;
@@ -270,7 +271,19 @@ 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);
+	r = crash_exclude_mem_range(cmem, start, end);
+
+	if (r)
+		return r;
+
+	/* Exclude dm crypt key region */
+	if (image->dm_crypt_key_addr) {
+		start = image->dm_crypt_key_addr;
+		end = start + image->dm_crypt_key_sz - 1;
+		return crash_exclude_mem_range(cmem, start, end);
+	}
+
+	return r;
 }
 
 /* Prepare memory map for crash dump kernel */
diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
index a61c12c01270..6e8adfe0b417 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_key_addr != 0)
+			len += sprintf(cmdline_ptr + len,
+					"dmcryptkey=0x%lx ", image->dm_crypt_key_addr);
 	}
 	memcpy(cmdline_ptr + len, cmdline, cmdline_len);
 	cmdline_len += len;
@@ -433,6 +437,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_key(image);
+		if (ret)
+			pr_debug("Either no dm crypt key or error to retrieve the dm crypt key\n");
 	}
 
 	/*
-- 
2.43.0


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

* [PATCH v2 5/5] x86/crash: make the page that stores the dm crypt key inaccessible
  2024-01-10  7:15 [PATCH v2 0/5] Support kdump with LUKS encryption by reusing LUKS volume key Coiby Xu
                   ` (3 preceding siblings ...)
  2024-01-10  7:15 ` [PATCH v2 4/5] x86/crash: pass the dm crypt key to " Coiby Xu
@ 2024-01-10  7:15 ` Coiby Xu
  2024-01-16 10:37 ` [PATCH v2 0/5] Support kdump with LUKS encryption by reusing LUKS volume key Ondrej Kozina
  5 siblings, 0 replies; 14+ messages in thread
From: Coiby Xu @ 2024-01-10  7:15 UTC (permalink / raw)
  To: kexec
  Cc: Ondrej Kozina, Milan Broz, Thomas Staudt,
	Daniel P . Berrangé, Kairui Song, dm-devel, Jan Pazdziora,
	Pingfan Liu, Baoquan He, Dave Young, linux-kernel, x86,
	Dave Hansen, Vitaly Kuznetsov, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Dave Hansen, H. Peter Anvin

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 | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
index 1a3e2c05a8a5..c9c814b934b8 100644
--- a/arch/x86/kernel/machine_kexec_64.c
+++ b/arch/x86/kernel/machine_kexec_64.c
@@ -546,14 +546,32 @@ static void kexec_mark_crashkres(bool protect)
 	kexec_mark_range(control, crashk_res.end, protect);
 }
 
+static void kexec_mark_dm_crypt_key(bool protect)
+{
+	unsigned long start_paddr, end_paddr;
+	unsigned int nr_pages;
+
+	if (kexec_crash_image->dm_crypt_key_addr) {
+		start_paddr = kexec_crash_image->dm_crypt_key_addr;
+		end_paddr = start_paddr + kexec_crash_image->dm_crypt_key_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_rw((unsigned long)phys_to_virt(start_paddr), nr_pages);
+	}
+}
+
 void arch_kexec_protect_crashkres(void)
 {
 	kexec_mark_crashkres(true);
+	kexec_mark_dm_crypt_key(true);
 }
 
 void arch_kexec_unprotect_crashkres(void)
 {
 	kexec_mark_crashkres(false);
+	kexec_mark_dm_crypt_key(false);
 }
 
 /*
-- 
2.43.0


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

* Re: [PATCH v2 2/5] crash_dump: save the dm crypt key temporarily
  2024-01-10  7:15 ` [PATCH v2 2/5] crash_dump: save the dm crypt key temporarily Coiby Xu
@ 2024-01-13 17:07   ` kernel test robot
  2024-01-13 17:50   ` kernel test robot
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 14+ messages in thread
From: kernel test robot @ 2024-01-13 17:07 UTC (permalink / raw)
  To: Coiby Xu, kexec
  Cc: oe-kbuild-all, Ondrej Kozina, Milan Broz, Thomas Staudt,
	Daniel P . Berrangé, Kairui Song, dm-devel, Jan Pazdziora,
	Pingfan Liu, Baoquan He, Dave Young, linux-kernel, x86,
	Dave Hansen, Vitaly Kuznetsov, Vivek Goyal, Eric Biederman

Hi Coiby,

kernel test robot noticed the following build errors:

[auto build test ERROR on tip/x86/core]
[also build test ERROR on v6.7]
[cannot apply to linus/master next-20240112]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Coiby-Xu/kexec_file-allow-to-place-kexec_buf-randomly/20240110-151859
base:   tip/x86/core
patch link:    https://lore.kernel.org/r/20240110071522.1308935-3-coxu%40redhat.com
patch subject: [PATCH v2 2/5] crash_dump: save the dm crypt key temporarily
config: i386-buildonly-randconfig-003-20240113 (https://download.01.org/0day-ci/archive/20240114/202401140034.Sh6fQaK6-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240114/202401140034.Sh6fQaK6-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/202401140034.Sh6fQaK6-lkp@intel.com/

All error/warnings (new ones prefixed by >>):

>> kernel/crash_dump_dm_crypt.c:8:6: warning: no previous prototype for 'wipe_dm_crypt_key' [-Wmissing-prototypes]
       8 | void wipe_dm_crypt_key(void)
         |      ^~~~~~~~~~~~~~~~~
   kernel/crash_dump_dm_crypt.c: In function 'crash_load_dm_crypt_key':
>> kernel/crash_dump_dm_crypt.c:94:16: error: variable 'kbuf' has initializer but incomplete type
      94 |         struct kexec_buf kbuf = {
         |                ^~~~~~~~~
>> kernel/crash_dump_dm_crypt.c:95:18: error: 'struct kexec_buf' has no member named 'image'
      95 |                 .image = image,
         |                  ^~~~~
>> kernel/crash_dump_dm_crypt.c:95:26: warning: excess elements in struct initializer
      95 |                 .image = image,
         |                          ^~~~~
   kernel/crash_dump_dm_crypt.c:95:26: note: (near initialization for 'kbuf')
>> kernel/crash_dump_dm_crypt.c:96:18: error: 'struct kexec_buf' has no member named 'buf_min'
      96 |                 .buf_min = 0,
         |                  ^~~~~~~
   kernel/crash_dump_dm_crypt.c:96:28: warning: excess elements in struct initializer
      96 |                 .buf_min = 0,
         |                            ^
   kernel/crash_dump_dm_crypt.c:96:28: note: (near initialization for 'kbuf')
>> kernel/crash_dump_dm_crypt.c:97:18: error: 'struct kexec_buf' has no member named 'buf_max'
      97 |                 .buf_max = ULONG_MAX,
         |                  ^~~~~~~
   In file included from include/linux/limits.h:7,
                    from include/linux/kernel.h:17,
                    from arch/x86/include/asm/percpu.h:27,
                    from arch/x86/include/asm/nospec-branch.h:14,
                    from arch/x86/include/asm/irqflags.h:9,
                    from include/linux/irqflags.h:17,
                    from include/linux/rcupdate.h:26,
                    from include/linux/rbtree.h:24,
                    from include/linux/key.h:15,
                    from include/keys/user-type.h:11,
                    from kernel/crash_dump_dm_crypt.c:2:
>> include/vdso/limits.h:13:25: warning: excess elements in struct initializer
      13 | #define ULONG_MAX       (~0UL)
         |                         ^
   kernel/crash_dump_dm_crypt.c:97:28: note: in expansion of macro 'ULONG_MAX'
      97 |                 .buf_max = ULONG_MAX,
         |                            ^~~~~~~~~
   include/vdso/limits.h:13:25: note: (near initialization for 'kbuf')
      13 | #define ULONG_MAX       (~0UL)
         |                         ^
   kernel/crash_dump_dm_crypt.c:97:28: note: in expansion of macro 'ULONG_MAX'
      97 |                 .buf_max = ULONG_MAX,
         |                            ^~~~~~~~~
>> kernel/crash_dump_dm_crypt.c:98:18: error: 'struct kexec_buf' has no member named 'top_down'
      98 |                 .top_down = false,
         |                  ^~~~~~~~
   kernel/crash_dump_dm_crypt.c:98:29: warning: excess elements in struct initializer
      98 |                 .top_down = false,
         |                             ^~~~~
   kernel/crash_dump_dm_crypt.c:98:29: note: (near initialization for 'kbuf')
>> kernel/crash_dump_dm_crypt.c:99:18: error: 'struct kexec_buf' has no member named 'random'
      99 |                 .random = true,
         |                  ^~~~~~
   kernel/crash_dump_dm_crypt.c:99:27: warning: excess elements in struct initializer
      99 |                 .random = true,
         |                           ^~~~
   kernel/crash_dump_dm_crypt.c:99:27: note: (near initialization for 'kbuf')
>> kernel/crash_dump_dm_crypt.c:94:26: error: storage size of 'kbuf' isn't known
      94 |         struct kexec_buf kbuf = {
         |                          ^~~~
>> kernel/crash_dump_dm_crypt.c:109:20: error: 'KEXEC_BUF_MEM_UNKNOWN' undeclared (first use in this function)
     109 |         kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
         |                    ^~~~~~~~~~~~~~~~~~~~~
   kernel/crash_dump_dm_crypt.c:109:20: note: each undeclared identifier is reported only once for each function it appears in
>> kernel/crash_dump_dm_crypt.c:110:15: error: implicit declaration of function 'kexec_add_buffer' [-Werror=implicit-function-declaration]
     110 |         ret = kexec_add_buffer(&kbuf);
         |               ^~~~~~~~~~~~~~~~
>> kernel/crash_dump_dm_crypt.c:94:26: warning: unused variable 'kbuf' [-Wunused-variable]
      94 |         struct kexec_buf kbuf = {
         |                          ^~~~
   cc1: some warnings being treated as errors


vim +/kbuf +94 kernel/crash_dump_dm_crypt.c

     7	
   > 8	void wipe_dm_crypt_key(void)
     9	{
    10		if (dm_crypt_key) {
    11			memset(dm_crypt_key, 0, dm_crypt_key_size * sizeof(u8));
    12			kfree(dm_crypt_key);
    13			dm_crypt_key = NULL;
    14		}
    15	}
    16	
    17	static void _wipe_dm_crypt_key(struct work_struct *dummy)
    18	{
    19		wipe_dm_crypt_key();
    20	}
    21	
    22	static DECLARE_DELAYED_WORK(wipe_dm_crypt_key_work, _wipe_dm_crypt_key);
    23	
    24	static unsigned __read_mostly wipe_key_delay = 120; /* 2 mins */
    25	
    26	static int crash_save_temp_dm_crypt_key(const char *key_desc, size_t count)
    27	{
    28		const struct user_key_payload *ukp;
    29		struct key *key;
    30	
    31		if (dm_crypt_key) {
    32			memset(dm_crypt_key, 0, dm_crypt_key_size * sizeof(u8));
    33			kfree(dm_crypt_key);
    34		}
    35	
    36		pr_debug("Requesting key %s", key_desc);
    37		key = request_key(&key_type_user, key_desc, NULL);
    38	
    39		if (IS_ERR(key)) {
    40			pr_debug("No such key %s", key_desc);
    41			return PTR_ERR(key);
    42		}
    43	
    44		ukp = user_key_payload_locked(key);
    45		if (!ukp)
    46			return -EKEYREVOKED;
    47	
    48		dm_crypt_key = kmalloc(ukp->datalen, GFP_KERNEL);
    49		if (!dm_crypt_key)
    50			return -ENOMEM;
    51		memcpy(dm_crypt_key, ukp->data, ukp->datalen);
    52		dm_crypt_key_size = ukp->datalen;
    53		pr_debug("dm crypt key (size=%u): %8ph\n", dm_crypt_key_size, dm_crypt_key);
    54		schedule_delayed_work(&wipe_dm_crypt_key_work,
    55				      round_jiffies_relative(wipe_key_delay * HZ));
    56		return 0;
    57	}
    58	
    59	int crash_sysfs_dm_crypt_key_write(const char *key_desc, size_t count)
    60	{
    61		if (!is_kdump_kernel())
    62			return crash_save_temp_dm_crypt_key(key_desc, count);
    63		return -EINVAL;
    64	}
    65	EXPORT_SYMBOL(crash_sysfs_dm_crypt_key_write);
    66	
    67	int crash_pass_temp_dm_crypt_key(void **addr, unsigned long *sz)
    68	{
    69		unsigned long dm_crypt_key_sz;
    70		unsigned char *buf;
    71		unsigned int *size_ptr;
    72	
    73		if (!dm_crypt_key)
    74			return -EINVAL;
    75	
    76		dm_crypt_key_sz = sizeof(unsigned int) + dm_crypt_key_size * sizeof(u8);
    77	
    78		buf = vzalloc(dm_crypt_key_sz);
    79		if (!buf)
    80			return -ENOMEM;
    81	
    82		size_ptr = (unsigned int *)buf;
    83		memcpy(size_ptr, &dm_crypt_key_size, sizeof(unsigned int));
    84		memcpy(size_ptr + 1, dm_crypt_key, dm_crypt_key_size * sizeof(u8));
    85		*addr = buf;
    86		*sz = dm_crypt_key_sz;
    87		wipe_dm_crypt_key();
    88		return 0;
    89	}
    90	
    91	int crash_load_dm_crypt_key(struct kimage *image)
    92	{
    93		int ret;
  > 94		struct kexec_buf kbuf = {
  > 95			.image = image,
  > 96			.buf_min = 0,
  > 97			.buf_max = ULONG_MAX,
  > 98			.top_down = false,
  > 99			.random = true,
   100		};
   101	
   102		image->dm_crypt_key_addr = 0;
   103		ret = crash_pass_temp_dm_crypt_key(&kbuf.buffer, &kbuf.bufsz);
   104		if (ret)
   105			return ret;
   106	
   107		kbuf.memsz = kbuf.bufsz;
   108		kbuf.buf_align = ELF_CORE_HEADER_ALIGN;
 > 109		kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
 > 110		ret = kexec_add_buffer(&kbuf);

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

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

* Re: [PATCH v2 2/5] crash_dump: save the dm crypt key temporarily
  2024-01-10  7:15 ` [PATCH v2 2/5] crash_dump: save the dm crypt key temporarily Coiby Xu
  2024-01-13 17:07   ` kernel test robot
@ 2024-01-13 17:50   ` kernel test robot
  2024-01-13 21:43   ` kernel test robot
  2024-01-16 10:40   ` Ondrej Kozina
  3 siblings, 0 replies; 14+ messages in thread
From: kernel test robot @ 2024-01-13 17:50 UTC (permalink / raw)
  To: Coiby Xu, kexec
  Cc: oe-kbuild-all, Ondrej Kozina, Milan Broz, Thomas Staudt,
	Daniel P . Berrangé, Kairui Song, dm-devel, Jan Pazdziora,
	Pingfan Liu, Baoquan He, Dave Young, linux-kernel, x86,
	Dave Hansen, Vitaly Kuznetsov, Vivek Goyal, Eric Biederman

Hi Coiby,

kernel test robot noticed the following build errors:

[auto build test ERROR on tip/x86/core]
[also build test ERROR on v6.7]
[cannot apply to linus/master next-20240112]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Coiby-Xu/kexec_file-allow-to-place-kexec_buf-randomly/20240110-151859
base:   tip/x86/core
patch link:    https://lore.kernel.org/r/20240110071522.1308935-3-coxu%40redhat.com
patch subject: [PATCH v2 2/5] crash_dump: save the dm crypt key temporarily
config: i386-buildonly-randconfig-002-20240113 (https://download.01.org/0day-ci/archive/20240114/202401140128.MBysmTaN-lkp@intel.com/config)
compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240114/202401140128.MBysmTaN-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/202401140128.MBysmTaN-lkp@intel.com/

All errors (new ones prefixed by >>):

   ld: kernel/ksysfs.o: in function `crash_dm_crypt_key_store':
>> ksysfs.c:(.text+0x2d0): undefined reference to `crash_sysfs_dm_crypt_key_write'

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

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

* Re: [PATCH v2 3/5] crash_dump: retrieve dm crypt key in kdump kernel
  2024-01-10  7:15 ` [PATCH v2 3/5] crash_dump: retrieve dm crypt key in kdump kernel Coiby Xu
@ 2024-01-13 18:45   ` kernel test robot
  0 siblings, 0 replies; 14+ messages in thread
From: kernel test robot @ 2024-01-13 18:45 UTC (permalink / raw)
  To: Coiby Xu, kexec
  Cc: oe-kbuild-all, Ondrej Kozina, Milan Broz, Thomas Staudt,
	Daniel P . Berrangé, Kairui Song, dm-devel, Jan Pazdziora,
	Pingfan Liu, Baoquan He, Dave Young, linux-kernel, x86,
	Dave Hansen, Vitaly Kuznetsov, Vivek Goyal

Hi Coiby,

kernel test robot noticed the following build warnings:

[auto build test WARNING on tip/x86/core]
[also build test WARNING on v6.7]
[cannot apply to linus/master next-20240112]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Coiby-Xu/kexec_file-allow-to-place-kexec_buf-randomly/20240110-151859
base:   tip/x86/core
patch link:    https://lore.kernel.org/r/20240110071522.1308935-4-coxu%40redhat.com
patch subject: [PATCH v2 3/5] crash_dump: retrieve dm crypt key in kdump kernel
config: i386-buildonly-randconfig-003-20240113 (https://download.01.org/0day-ci/archive/20240114/202401140240.upb0a18s-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240114/202401140240.upb0a18s-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/202401140240.upb0a18s-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> kernel/crash_dump_dm_crypt.c:29:16: warning: no previous prototype for 'dm_crypt_key_read' [-Wmissing-prototypes]
      29 | ssize_t __weak dm_crypt_key_read(char *buf, size_t count, u64 *ppos)
         |                ^~~~~~~~~~~~~~~~~
   kernel/crash_dump_dm_crypt.c:83:6: warning: no previous prototype for 'wipe_dm_crypt_key' [-Wmissing-prototypes]
      83 | void wipe_dm_crypt_key(void)
         |      ^~~~~~~~~~~~~~~~~
   kernel/crash_dump_dm_crypt.c: In function 'crash_load_dm_crypt_key':
   kernel/crash_dump_dm_crypt.c:207:16: error: variable 'kbuf' has initializer but incomplete type
     207 |         struct kexec_buf kbuf = {
         |                ^~~~~~~~~
   kernel/crash_dump_dm_crypt.c:208:18: error: 'struct kexec_buf' has no member named 'image'
     208 |                 .image = image,
         |                  ^~~~~
   kernel/crash_dump_dm_crypt.c:208:26: warning: excess elements in struct initializer
     208 |                 .image = image,
         |                          ^~~~~
   kernel/crash_dump_dm_crypt.c:208:26: note: (near initialization for 'kbuf')
   kernel/crash_dump_dm_crypt.c:209:18: error: 'struct kexec_buf' has no member named 'buf_min'
     209 |                 .buf_min = 0,
         |                  ^~~~~~~
   kernel/crash_dump_dm_crypt.c:209:28: warning: excess elements in struct initializer
     209 |                 .buf_min = 0,
         |                            ^
   kernel/crash_dump_dm_crypt.c:209:28: note: (near initialization for 'kbuf')
   kernel/crash_dump_dm_crypt.c:210:18: error: 'struct kexec_buf' has no member named 'buf_max'
     210 |                 .buf_max = ULONG_MAX,
         |                  ^~~~~~~
   In file included from include/linux/limits.h:7,
                    from include/linux/kernel.h:17,
                    from arch/x86/include/asm/percpu.h:27,
                    from arch/x86/include/asm/nospec-branch.h:14,
                    from arch/x86/include/asm/irqflags.h:9,
                    from include/linux/irqflags.h:17,
                    from include/linux/rcupdate.h:26,
                    from include/linux/rbtree.h:24,
                    from include/linux/key.h:15,
                    from kernel/crash_dump_dm_crypt.c:2:
   include/vdso/limits.h:13:25: warning: excess elements in struct initializer
      13 | #define ULONG_MAX       (~0UL)
         |                         ^
   kernel/crash_dump_dm_crypt.c:210:28: note: in expansion of macro 'ULONG_MAX'
     210 |                 .buf_max = ULONG_MAX,
         |                            ^~~~~~~~~
   include/vdso/limits.h:13:25: note: (near initialization for 'kbuf')
      13 | #define ULONG_MAX       (~0UL)
         |                         ^
   kernel/crash_dump_dm_crypt.c:210:28: note: in expansion of macro 'ULONG_MAX'
     210 |                 .buf_max = ULONG_MAX,
         |                            ^~~~~~~~~
   kernel/crash_dump_dm_crypt.c:211:18: error: 'struct kexec_buf' has no member named 'top_down'
     211 |                 .top_down = false,
         |                  ^~~~~~~~
   kernel/crash_dump_dm_crypt.c:211:29: warning: excess elements in struct initializer
     211 |                 .top_down = false,
         |                             ^~~~~
   kernel/crash_dump_dm_crypt.c:211:29: note: (near initialization for 'kbuf')
   kernel/crash_dump_dm_crypt.c:212:18: error: 'struct kexec_buf' has no member named 'random'
     212 |                 .random = true,
         |                  ^~~~~~
   kernel/crash_dump_dm_crypt.c:212:27: warning: excess elements in struct initializer
     212 |                 .random = true,
         |                           ^~~~
   kernel/crash_dump_dm_crypt.c:212:27: note: (near initialization for 'kbuf')
   kernel/crash_dump_dm_crypt.c:207:26: error: storage size of 'kbuf' isn't known
     207 |         struct kexec_buf kbuf = {
         |                          ^~~~
   kernel/crash_dump_dm_crypt.c:222:20: error: 'KEXEC_BUF_MEM_UNKNOWN' undeclared (first use in this function)
     222 |         kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
         |                    ^~~~~~~~~~~~~~~~~~~~~
   kernel/crash_dump_dm_crypt.c:222:20: note: each undeclared identifier is reported only once for each function it appears in
   kernel/crash_dump_dm_crypt.c:223:15: error: implicit declaration of function 'kexec_add_buffer' [-Werror=implicit-function-declaration]
     223 |         ret = kexec_add_buffer(&kbuf);
         |               ^~~~~~~~~~~~~~~~
   kernel/crash_dump_dm_crypt.c:207:26: warning: unused variable 'kbuf' [-Wunused-variable]
     207 |         struct kexec_buf kbuf = {
         |                          ^~~~
   cc1: some warnings being treated as errors


vim +/dm_crypt_key_read +29 kernel/crash_dump_dm_crypt.c

    25	
    26	/*
    27	 * Architectures may override this function to read dm crypt key
    28	 */
  > 29	ssize_t __weak dm_crypt_key_read(char *buf, size_t count, u64 *ppos)
    30	{
    31		struct kvec kvec = { .iov_base = buf, .iov_len = count };
    32		struct iov_iter iter;
    33	
    34		iov_iter_kvec(&iter, READ, &kvec, 1, count);
    35		return read_from_oldmem(&iter, count, ppos, false);
    36	}
    37	

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

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

* Re: [PATCH v2 2/5] crash_dump: save the dm crypt key temporarily
  2024-01-10  7:15 ` [PATCH v2 2/5] crash_dump: save the dm crypt key temporarily Coiby Xu
  2024-01-13 17:07   ` kernel test robot
  2024-01-13 17:50   ` kernel test robot
@ 2024-01-13 21:43   ` kernel test robot
  2024-01-16 10:40   ` Ondrej Kozina
  3 siblings, 0 replies; 14+ messages in thread
From: kernel test robot @ 2024-01-13 21:43 UTC (permalink / raw)
  To: Coiby Xu, kexec
  Cc: oe-kbuild-all, Ondrej Kozina, Milan Broz, Thomas Staudt,
	Daniel P . Berrangé, Kairui Song, dm-devel, Jan Pazdziora,
	Pingfan Liu, Baoquan He, Dave Young, linux-kernel, x86,
	Dave Hansen, Vitaly Kuznetsov, Vivek Goyal, Eric Biederman

Hi Coiby,

kernel test robot noticed the following build warnings:

[auto build test WARNING on tip/x86/core]
[also build test WARNING on v6.7]
[cannot apply to linus/master next-20240112]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Coiby-Xu/kexec_file-allow-to-place-kexec_buf-randomly/20240110-151859
base:   tip/x86/core
patch link:    https://lore.kernel.org/r/20240110071522.1308935-3-coxu%40redhat.com
patch subject: [PATCH v2 2/5] crash_dump: save the dm crypt key temporarily
config: x86_64-randconfig-123-20240114 (https://download.01.org/0day-ci/archive/20240114/202401140506.JQu60mgM-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240114/202401140506.JQu60mgM-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/202401140506.JQu60mgM-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> kernel/crash_dump_dm_crypt.c:8:6: sparse: sparse: symbol 'wipe_dm_crypt_key' was not declared. Should it be static?
   kernel/crash_dump_dm_crypt.c: note: in included file (through include/linux/mmzone.h, include/linux/gfp.h, include/linux/mm.h, ...):
   include/linux/page-flags.h:242:46: sparse: sparse: self-comparison always evaluates to false

vim +/wipe_dm_crypt_key +8 kernel/crash_dump_dm_crypt.c

     7	
   > 8	void wipe_dm_crypt_key(void)
     9	{
    10		if (dm_crypt_key) {
    11			memset(dm_crypt_key, 0, dm_crypt_key_size * sizeof(u8));
    12			kfree(dm_crypt_key);
    13			dm_crypt_key = NULL;
    14		}
    15	}
    16	

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

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

* Re: [PATCH v2 0/5] Support kdump with LUKS encryption by reusing LUKS volume key
  2024-01-10  7:15 [PATCH v2 0/5] Support kdump with LUKS encryption by reusing LUKS volume key Coiby Xu
                   ` (4 preceding siblings ...)
  2024-01-10  7:15 ` [PATCH v2 5/5] x86/crash: make the page that stores the dm crypt key inaccessible Coiby Xu
@ 2024-01-16 10:37 ` Ondrej Kozina
  2024-01-17  7:38   ` Coiby Xu
  5 siblings, 1 reply; 14+ messages in thread
From: Ondrej Kozina @ 2024-01-16 10:37 UTC (permalink / raw)
  To: Coiby Xu, kexec
  Cc: Milan Broz, Thomas Staudt, Daniel P . Berrangé, Kairui Song,
	dm-devel, Jan Pazdziora, Pingfan Liu, Baoquan He, Dave Young,
	linux-kernel, x86, Dave Hansen, Vitaly Kuznetsov

Hi Coiby,

I've started working on a patchset for systemd utility. I have one 
question/suggestion:

On 10/01/2024 08:15, 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 key 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 key again in kdump kernel which seems to be
> redundant work.
> 
> This patch set addresses the above issues by reusing the LUKS volume key
> in 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 key,
> 
>   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 key and then save the volume key 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 the key description to
>      /sys/kernel/crash_dm_crypt_key to inform the 1st kernel to save a
>      temporary copy of the volume key while building the kdump initramfs

So this volume key copy cached by systemd utility in 1st kernel does not 
have to be readable from userspace.

> 
>   3. The kexec_file_load syscall saves the temporary copy of the volume
>      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 the key
>      description to /sys/kernel/crash_dm_crypt_key. Then the LUKS
>      encrypted devide is unlocked with libcryptsetup's
>      --volume-key-keyring API.

Unlike here where it has to readable from uspace so that libcryptsetup 
can verify the volume key.

Is it correct?
O.

> 
>   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 key to specified keyring,
> whoever takes this should be responsible for the safety of this copy
> of key. This key 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.
> 


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

* Re: [PATCH v2 2/5] crash_dump: save the dm crypt key temporarily
  2024-01-10  7:15 ` [PATCH v2 2/5] crash_dump: save the dm crypt key temporarily Coiby Xu
                     ` (2 preceding siblings ...)
  2024-01-13 21:43   ` kernel test robot
@ 2024-01-16 10:40   ` Ondrej Kozina
  2024-01-17  7:39     ` Coiby Xu
  3 siblings, 1 reply; 14+ messages in thread
From: Ondrej Kozina @ 2024-01-16 10:40 UTC (permalink / raw)
  To: Coiby Xu, kexec
  Cc: Milan Broz, Thomas Staudt, Daniel P . Berrangé, Kairui Song,
	dm-devel, Jan Pazdziora, Pingfan Liu, Baoquan He, Dave Young,
	linux-kernel, x86, Dave Hansen, Vitaly Kuznetsov, Vivek Goyal,
	Eric Biederman

On 10/01/2024 08:15, Coiby Xu wrote:
> User space is supposed to write the key description to
> /sys/kernel/crash_dm_crypt_key so the kernel will read the key and save
> a temporary copy for later user. User space has 2 minutes at maximum to
> load the kdump initrd before the key gets wiped. And after kdump
> retrieves the key, the key will be wiped immediately.
> 
> Signed-off-by: Coiby Xu <coxu@redhat.com>
> ---
>   include/linux/crash_core.h   |   7 +-
>   include/linux/kexec.h        |   4 ++
>   kernel/Makefile              |   2 +-
>   kernel/crash_dump_dm_crypt.c | 121 +++++++++++++++++++++++++++++++++++
>   kernel/ksysfs.c              |  23 ++++++-
>   5 files changed, 153 insertions(+), 4 deletions(-)
>   create mode 100644 kernel/crash_dump_dm_crypt.c
> 
> diff --git a/include/linux/crash_core.h b/include/linux/crash_core.h
> index 5126a4fecb44..7078eda6418d 100644
> --- a/include/linux/crash_core.h
> +++ b/include/linux/crash_core.h
> @@ -125,6 +125,12 @@ static inline void __init reserve_crashkernel_generic(char *cmdline,
>   {}
>   #endif
>   
> +struct kimage;
> +
> +int crash_sysfs_dm_crypt_key_write(const char *key_des, size_t count);
> +int crash_pass_temp_dm_crypt_key(void **addr, unsigned long *sz);
> +int crash_load_dm_crypt_key(struct kimage *image);
> +
>   /* Alignment required for elf header segment */
>   #define ELF_CORE_HEADER_ALIGN   4096
>   
> @@ -140,7 +146,6 @@ extern int crash_exclude_mem_range(struct crash_mem *mem,
>   extern int crash_prepare_elf64_headers(struct crash_mem *mem, int need_kernel_map,
>   				       void **addr, unsigned long *sz);
>   
> -struct kimage;
>   struct kexec_segment;
>   
>   #define KEXEC_CRASH_HP_NONE			0
> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> index 6f4626490ebf..bf7ab1e927ef 100644
> --- a/include/linux/kexec.h
> +++ b/include/linux/kexec.h
> @@ -366,6 +366,10 @@ struct kimage {
>   	void *elf_headers;
>   	unsigned long elf_headers_sz;
>   	unsigned long elf_load_addr;
> +
> +	/* dm crypt key buffer */
> +	unsigned long dm_crypt_key_addr;
> +	unsigned long dm_crypt_key_sz;
>   };
>   
>   /* kexec interface functions */
> diff --git a/kernel/Makefile b/kernel/Makefile
> index 3947122d618b..48859bf63db5 100644
> --- a/kernel/Makefile
> +++ b/kernel/Makefile
> @@ -119,7 +119,7 @@ obj-$(CONFIG_PERF_EVENTS) += events/
>   
>   obj-$(CONFIG_USER_RETURN_NOTIFIER) += user-return-notifier.o
>   obj-$(CONFIG_PADATA) += padata.o
> -obj-$(CONFIG_CRASH_DUMP) += crash_dump.o
> +obj-$(CONFIG_CRASH_DUMP) += crash_dump.o crash_dump_dm_crypt.o
>   obj-$(CONFIG_JUMP_LABEL) += jump_label.o
>   obj-$(CONFIG_CONTEXT_TRACKING) += context_tracking.o
>   obj-$(CONFIG_TORTURE_TEST) += torture.o
> diff --git a/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c
> new file mode 100644
> index 000000000000..3a0b0b773598
> --- /dev/null
> +++ b/kernel/crash_dump_dm_crypt.c
> @@ -0,0 +1,121 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +#include <keys/user-type.h>
> +#include <linux/crash_dump.h>
> +
> +static u8 *dm_crypt_key;
> +static unsigned int dm_crypt_key_size;
> +
> +void wipe_dm_crypt_key(void)
> +{
> +	if (dm_crypt_key) {
> +		memset(dm_crypt_key, 0, dm_crypt_key_size * sizeof(u8));
> +		kfree(dm_crypt_key);
> +		dm_crypt_key = NULL;
> +	}
> +}
> +
> +static void _wipe_dm_crypt_key(struct work_struct *dummy)
> +{
> +	wipe_dm_crypt_key();
> +}
> +
> +static DECLARE_DELAYED_WORK(wipe_dm_crypt_key_work, _wipe_dm_crypt_key);
> +
> +static unsigned __read_mostly wipe_key_delay = 120; /* 2 mins */
> +
> +static int crash_save_temp_dm_crypt_key(const char *key_desc, size_t count)
> +{
> +	const struct user_key_payload *ukp;
> +	struct key *key;
> +
> +	if (dm_crypt_key) {
> +		memset(dm_crypt_key, 0, dm_crypt_key_size * sizeof(u8));
> +		kfree(dm_crypt_key);
> +	}
> +
> +	pr_debug("Requesting key %s", key_desc);
> +	key = request_key(&key_type_user, key_desc, NULL);

If we don't read the key copy form userspace (my reply to top level 
message) you could use key_type_logon here.

> +
> +	if (IS_ERR(key)) {
> +		pr_debug("No such key %s", key_desc);
> +		return PTR_ERR(key);
> +	}
> +
> +	ukp = user_key_payload_locked(key);
> +	if (!ukp)
> +		return -EKEYREVOKED;
> +
> +	dm_crypt_key = kmalloc(ukp->datalen, GFP_KERNEL);
> +	if (!dm_crypt_key)
> +		return -ENOMEM;
> +	memcpy(dm_crypt_key, ukp->data, ukp->datalen);
> +	dm_crypt_key_size = ukp->datalen;
> +	pr_debug("dm crypt key (size=%u): %8ph\n", dm_crypt_key_size, dm_crypt_key);
> +	schedule_delayed_work(&wipe_dm_crypt_key_work,
> +			      round_jiffies_relative(wipe_key_delay * HZ));
> +	return 0;
> +}
> +
> +int crash_sysfs_dm_crypt_key_write(const char *key_desc, size_t count)
> +{
> +	if (!is_kdump_kernel())
> +		return crash_save_temp_dm_crypt_key(key_desc, count);
> +	return -EINVAL;
> +}
> +EXPORT_SYMBOL(crash_sysfs_dm_crypt_key_write);
> +
> +int crash_pass_temp_dm_crypt_key(void **addr, unsigned long *sz)
> +{
> +	unsigned long dm_crypt_key_sz;
> +	unsigned char *buf;
> +	unsigned int *size_ptr;
> +
> +	if (!dm_crypt_key)
> +		return -EINVAL;
> +
> +	dm_crypt_key_sz = sizeof(unsigned int) + dm_crypt_key_size * sizeof(u8);
> +
> +	buf = vzalloc(dm_crypt_key_sz);
> +	if (!buf)
> +		return -ENOMEM;
> +
> +	size_ptr = (unsigned int *)buf;
> +	memcpy(size_ptr, &dm_crypt_key_size, sizeof(unsigned int));
> +	memcpy(size_ptr + 1, dm_crypt_key, dm_crypt_key_size * sizeof(u8));
> +	*addr = buf;
> +	*sz = dm_crypt_key_sz;
> +	wipe_dm_crypt_key();
> +	return 0;
> +}
> +
> +int crash_load_dm_crypt_key(struct kimage *image)
> +{
> +	int ret;
> +	struct kexec_buf kbuf = {
> +		.image = image,
> +		.buf_min = 0,
> +		.buf_max = ULONG_MAX,
> +		.top_down = false,
> +		.random = true,
> +	};
> +
> +	image->dm_crypt_key_addr = 0;
> +	ret = crash_pass_temp_dm_crypt_key(&kbuf.buffer, &kbuf.bufsz);
> +	if (ret)
> +		return ret;
> +
> +	kbuf.memsz = kbuf.bufsz;
> +	kbuf.buf_align = ELF_CORE_HEADER_ALIGN;
> +	kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> +	ret = kexec_add_buffer(&kbuf);
> +	if (ret) {
> +		vfree((void *)kbuf.buffer);
> +		return ret;
> +	}
> +	image->dm_crypt_key_addr = kbuf.mem;
> +	image->dm_crypt_key_sz = kbuf.bufsz;
> +	pr_debug("Loaded dm crypt key at 0x%lx bufsz=0x%lx memsz=0x%lx\n",
> +		 image->dm_crypt_key_addr, kbuf.bufsz, kbuf.bufsz);
> +
> +	return ret;
> +}
> diff --git a/kernel/ksysfs.c b/kernel/ksysfs.c
> index 1d4bc493b2f4..f3bb6bc6a604 100644
> --- a/kernel/ksysfs.c
> +++ b/kernel/ksysfs.c
> @@ -165,16 +165,34 @@ static ssize_t vmcoreinfo_show(struct kobject *kobj,
>   }
>   KERNEL_ATTR_RO(vmcoreinfo);
>   
> +static ssize_t crash_dm_crypt_key_show(struct kobject *kobj,
> +					  struct kobj_attribute *attr,
> +					  char *buf)
> +{
> +	return 0;
> +}
> +
> +static ssize_t crash_dm_crypt_key_store(struct kobject *kobj,
> +					struct kobj_attribute *attr,
> +					   const char *buf, size_t count)
> +{
> +	int ret;
> +
> +	ret = crash_sysfs_dm_crypt_key_write(buf, count);
> +	return ret < 0 ? ret : count;
> +}
> +KERNEL_ATTR_RW(crash_dm_crypt_key);
> +
>   #ifdef CONFIG_CRASH_HOTPLUG
>   static ssize_t crash_elfcorehdr_size_show(struct kobject *kobj,
> -			       struct kobj_attribute *attr, char *buf)
> +					  struct kobj_attribute *attr,
> +					  char *buf)
>   {
>   	unsigned int sz = crash_get_elfcorehdr_size();
>   
>   	return sysfs_emit(buf, "%u\n", sz);
>   }
>   KERNEL_ATTR_RO(crash_elfcorehdr_size);
> -
>   #endif
>   
>   #endif /* CONFIG_CRASH_CORE */
> @@ -267,6 +285,7 @@ static struct attribute * kernel_attrs[] = {
>   #endif
>   #ifdef CONFIG_CRASH_CORE
>   	&vmcoreinfo_attr.attr,
> +	&crash_dm_crypt_key_attr.attr,
>   #ifdef CONFIG_CRASH_HOTPLUG
>   	&crash_elfcorehdr_size_attr.attr,
>   #endif


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

* Re: Re: [PATCH v2 0/5] Support kdump with LUKS encryption by reusing LUKS volume key
  2024-01-16 10:37 ` [PATCH v2 0/5] Support kdump with LUKS encryption by reusing LUKS volume key Ondrej Kozina
@ 2024-01-17  7:38   ` Coiby Xu
  0 siblings, 0 replies; 14+ messages in thread
From: Coiby Xu @ 2024-01-17  7:38 UTC (permalink / raw)
  To: Ondrej Kozina
  Cc: kexec, Milan Broz, Thomas Staudt, Daniel P . Berrangé,
	Kairui Song, dm-devel, Jan Pazdziora, Pingfan Liu, Baoquan He,
	Dave Young, linux-kernel, x86, Dave Hansen, Vitaly Kuznetsov

On Tue, Jan 16, 2024 at 11:37:10AM +0100, Ondrej Kozina wrote:
>Hi Coiby,

Hi Ondrej,

>
>I've started working on a patchset for systemd utility. I have one 
>question/suggestion:
>
>On 10/01/2024 08:15, 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 key 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 key again in kdump kernel which seems to be
>>redundant work.
>>
>>This patch set addresses the above issues by reusing the LUKS volume key
>>in 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 key,
>>
>>  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 key and then save the volume key 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 the key description to
>>     /sys/kernel/crash_dm_crypt_key to inform the 1st kernel to save a
>>     temporary copy of the volume key while building the kdump initramfs
>
>So this volume key copy cached by systemd utility in 1st kernel does 
>not have to be readable from userspace.
>
>>
>>  3. The kexec_file_load syscall saves the temporary copy of the volume
>>     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 the key
>>     description to /sys/kernel/crash_dm_crypt_key. Then the LUKS
>>     encrypted devide is unlocked with libcryptsetup's
>>     --volume-key-keyring API.
>
>Unlike here where it has to readable from uspace so that libcryptsetup 
>can verify the volume key.
>
>Is it correct?
>O.

Oh, my assumed --link-vk-to-keyring only support user key which must be
wrong. So yes, you are absolutely correct that "volume key copy cached
by systemd utility in 1st kernel does not have to be readable from
userspace". Thanks for the suggestion!


-- 
Best regards,
Coiby


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

* Re: Re: [PATCH v2 2/5] crash_dump: save the dm crypt key temporarily
  2024-01-16 10:40   ` Ondrej Kozina
@ 2024-01-17  7:39     ` Coiby Xu
  0 siblings, 0 replies; 14+ messages in thread
From: Coiby Xu @ 2024-01-17  7:39 UTC (permalink / raw)
  To: Ondrej Kozina
  Cc: kexec, Milan Broz, Thomas Staudt, Daniel P . Berrangé,
	Kairui Song, dm-devel, Jan Pazdziora, Pingfan Liu, Baoquan He,
	Dave Young, linux-kernel, x86, Dave Hansen, Vitaly Kuznetsov,
	Vivek Goyal, Eric Biederman

On Tue, Jan 16, 2024 at 11:40:53AM +0100, Ondrej Kozina wrote:
>On 10/01/2024 08:15, Coiby Xu wrote:
>>User space is supposed to write the key description to
>>/sys/kernel/crash_dm_crypt_key so the kernel will read the key and save
>>a temporary copy for later user. User space has 2 minutes at maximum to
>>load the kdump initrd before the key gets wiped. And after kdump
>>retrieves the key, the key will be wiped immediately.
>>
>>Signed-off-by: Coiby Xu <coxu@redhat.com>
>>---
>>  include/linux/crash_core.h   |   7 +-
>>  include/linux/kexec.h        |   4 ++
>>  kernel/Makefile              |   2 +-
>>  kernel/crash_dump_dm_crypt.c | 121 +++++++++++++++++++++++++++++++++++
>>  kernel/ksysfs.c              |  23 ++++++-
>>  5 files changed, 153 insertions(+), 4 deletions(-)
>>  create mode 100644 kernel/crash_dump_dm_crypt.c
>>
>>diff --git a/include/linux/crash_core.h b/include/linux/crash_core.h
>>index 5126a4fecb44..7078eda6418d 100644
>>--- a/include/linux/crash_core.h
>>+++ b/include/linux/crash_core.h
>>@@ -125,6 +125,12 @@ static inline void __init reserve_crashkernel_generic(char *cmdline,
>>  {}
>>  #endif
>>+struct kimage;
>>+
>>+int crash_sysfs_dm_crypt_key_write(const char *key_des, size_t count);
>>+int crash_pass_temp_dm_crypt_key(void **addr, unsigned long *sz);
>>+int crash_load_dm_crypt_key(struct kimage *image);
>>+
>>  /* Alignment required for elf header segment */
>>  #define ELF_CORE_HEADER_ALIGN   4096
>>@@ -140,7 +146,6 @@ extern int crash_exclude_mem_range(struct crash_mem *mem,
>>  extern int crash_prepare_elf64_headers(struct crash_mem *mem, int need_kernel_map,
>>  				       void **addr, unsigned long *sz);
>>-struct kimage;
>>  struct kexec_segment;
>>  #define KEXEC_CRASH_HP_NONE			0
>>diff --git a/include/linux/kexec.h b/include/linux/kexec.h
>>index 6f4626490ebf..bf7ab1e927ef 100644
>>--- a/include/linux/kexec.h
>>+++ b/include/linux/kexec.h
>>@@ -366,6 +366,10 @@ struct kimage {
>>  	void *elf_headers;
>>  	unsigned long elf_headers_sz;
>>  	unsigned long elf_load_addr;
>>+
>>+	/* dm crypt key buffer */
>>+	unsigned long dm_crypt_key_addr;
>>+	unsigned long dm_crypt_key_sz;
>>  };
>>  /* kexec interface functions */
>>diff --git a/kernel/Makefile b/kernel/Makefile
>>index 3947122d618b..48859bf63db5 100644
>>--- a/kernel/Makefile
>>+++ b/kernel/Makefile
>>@@ -119,7 +119,7 @@ obj-$(CONFIG_PERF_EVENTS) += events/
>>  obj-$(CONFIG_USER_RETURN_NOTIFIER) += user-return-notifier.o
>>  obj-$(CONFIG_PADATA) += padata.o
>>-obj-$(CONFIG_CRASH_DUMP) += crash_dump.o
>>+obj-$(CONFIG_CRASH_DUMP) += crash_dump.o crash_dump_dm_crypt.o
>>  obj-$(CONFIG_JUMP_LABEL) += jump_label.o
>>  obj-$(CONFIG_CONTEXT_TRACKING) += context_tracking.o
>>  obj-$(CONFIG_TORTURE_TEST) += torture.o
>>diff --git a/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c
>>new file mode 100644
>>index 000000000000..3a0b0b773598
>>--- /dev/null
>>+++ b/kernel/crash_dump_dm_crypt.c
>>@@ -0,0 +1,121 @@
>>+// SPDX-License-Identifier: GPL-2.0-only
>>+#include <keys/user-type.h>
>>+#include <linux/crash_dump.h>
>>+
>>+static u8 *dm_crypt_key;
>>+static unsigned int dm_crypt_key_size;
>>+
>>+void wipe_dm_crypt_key(void)
>>+{
>>+	if (dm_crypt_key) {
>>+		memset(dm_crypt_key, 0, dm_crypt_key_size * sizeof(u8));
>>+		kfree(dm_crypt_key);
>>+		dm_crypt_key = NULL;
>>+	}
>>+}
>>+
>>+static void _wipe_dm_crypt_key(struct work_struct *dummy)
>>+{
>>+	wipe_dm_crypt_key();
>>+}
>>+
>>+static DECLARE_DELAYED_WORK(wipe_dm_crypt_key_work, _wipe_dm_crypt_key);
>>+
>>+static unsigned __read_mostly wipe_key_delay = 120; /* 2 mins */
>>+
>>+static int crash_save_temp_dm_crypt_key(const char *key_desc, size_t count)
>>+{
>>+	const struct user_key_payload *ukp;
>>+	struct key *key;
>>+
>>+	if (dm_crypt_key) {
>>+		memset(dm_crypt_key, 0, dm_crypt_key_size * sizeof(u8));
>>+		kfree(dm_crypt_key);
>>+	}
>>+
>>+	pr_debug("Requesting key %s", key_desc);
>>+	key = request_key(&key_type_user, key_desc, NULL);
>
>If we don't read the key copy form userspace (my reply to top level 
>message) you could use key_type_logon here.

I'll use key_type_logon, thanks!

-- 
Best regards,
Coiby


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

end of thread, other threads:[~2024-01-17  7:45 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-01-10  7:15 [PATCH v2 0/5] Support kdump with LUKS encryption by reusing LUKS volume key Coiby Xu
2024-01-10  7:15 ` [PATCH v2 1/5] kexec_file: allow to place kexec_buf randomly Coiby Xu
2024-01-10  7:15 ` [PATCH v2 2/5] crash_dump: save the dm crypt key temporarily Coiby Xu
2024-01-13 17:07   ` kernel test robot
2024-01-13 17:50   ` kernel test robot
2024-01-13 21:43   ` kernel test robot
2024-01-16 10:40   ` Ondrej Kozina
2024-01-17  7:39     ` Coiby Xu
2024-01-10  7:15 ` [PATCH v2 3/5] crash_dump: retrieve dm crypt key in kdump kernel Coiby Xu
2024-01-13 18:45   ` kernel test robot
2024-01-10  7:15 ` [PATCH v2 4/5] x86/crash: pass the dm crypt key to " Coiby Xu
2024-01-10  7:15 ` [PATCH v2 5/5] x86/crash: make the page that stores the dm crypt key inaccessible Coiby Xu
2024-01-16 10:37 ` [PATCH v2 0/5] Support kdump with LUKS encryption by reusing LUKS volume key Ondrej Kozina
2024-01-17  7:38   ` Coiby Xu

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