* [PATCH v9 0/8] Support kdump with LUKS encryption by reusing LUKS volume keys
@ 2025-05-02 1:12 Coiby Xu
2025-05-02 1:12 ` [PATCH v9 1/8] kexec_file: allow to place kexec_buf randomly Coiby Xu
` (7 more replies)
0 siblings, 8 replies; 16+ messages in thread
From: Coiby Xu @ 2025-05-02 1:12 UTC (permalink / raw)
To: kexec
Cc: Ondrej Kozina, Milan Broz, Thomas Staudt,
Daniel P . Berrangé, Kairui Song, Pingfan Liu, Baoquan He,
Dave Young, linux-kernel, x86, Dave Hansen, Vitaly Kuznetsov,
Arnaud Lefebvre
LUKS is the standard for Linux disk encryption, widely adopted by users,
and in some cases, such as Confidential VMs, it is a requirement. With
kdump enabled, when the first kernel crashes, the system can boot into
the kdump/crash kernel to dump the memory image (i.e., /proc/vmcore)
to a specified target. However, there are two challenges when dumping
vmcore to a LUKS-encrypted device:
- 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 or custom crashkernel value 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 making 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
the kdump copies of LUKS volume keys,
1. After the 1st kernel loads the initramfs during boot, systemd
use an user-input passphrase to de-crypt the LUKS volume keys
or TPM-sealed key 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 loader like kdump-utils) create
key items inside /sys/kernel/config/crash_dm_crypt_keys to inform
the 1st kernel which keys are needed.
3. When the kdump initramfs is loaded by the kexec_file_load
syscall, the 1st kernel will iterate created key items, save the
keys to kdump reserved memory.
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 yes to
/sys/kernel/crash_dm_crypt_keys/restore. Then the LUKS encrypted
device 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.
v9
- check whether key description is set before copying it to resolve the
issue where the kernel crashes because user space tries to load kdump
kernel/initrams before writing key description(s) as found by Arnaud
- take the new kernel parameter dmcryptkeys into consideration when
checking cmlind buffer size [Arnaud]
- make CONFIG_CRASH_DM_CRYPT depends on CONFIGFS_FS [Arnaud]
- update documentation and message logs to indicate logon keys other
than user keys are accepted
- bring back __set_memory_prot
- add Acked-by tag from Baoquan
- rebase onto 6.15.0-rc4+
v8
- improve documentation [Randy]
- rebase onto 6.14.0-rc1
v7
- Baoquan
- differentiate between failing to get dm crypt keys and no dm crypt keys
- add code comments, change function name and etc. to improve code readability
- add documentation for configfs API [Dave]
- fix building error found by kernel test robot
v6
- Baoquan
- support AMD SEV
- drop uncessary keys_header_size
- improve commit message of [PATCH 4/7]
- Greg
- switch to configfs
- move ifdef from .c to .h files and rework kexec_random_start
- use tab instead of space for appended code comment
- Process key description in a more flexible way to address problems
found by Ondrej
- improve cover letter
- fix an compilation error as found by kernel test robot
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 readability
- 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 (8):
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
Revert "x86/mm: Remove unused __set_memory_prot()"
x86/crash: pass dm crypt keys to kdump kernel
x86/crash: make the page that stores the dm crypt keys inaccessible
Documentation/admin-guide/kdump/kdump.rst | 32 ++
arch/x86/include/asm/set_memory.h | 1 +
arch/x86/kernel/crash.c | 26 +-
arch/x86/kernel/kexec-bzimage64.c | 21 +
arch/x86/kernel/machine_kexec_64.c | 22 +
arch/x86/mm/pat/set_memory.c | 13 +
include/linux/crash_core.h | 7 +-
include/linux/crash_dump.h | 2 +
include/linux/kexec.h | 34 ++
kernel/Kconfig.kexec | 11 +
kernel/Makefile | 1 +
kernel/crash_dump_dm_crypt.c | 464 ++++++++++++++++++++++
kernel/kexec_file.c | 3 +
13 files changed, 634 insertions(+), 3 deletions(-)
create mode 100644 kernel/crash_dump_dm_crypt.c
base-commit: b4432656b36e5cc1d50a1f2dc15357543add530e
--
2.49.0
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v9 1/8] kexec_file: allow to place kexec_buf randomly
2025-05-02 1:12 [PATCH v9 0/8] Support kdump with LUKS encryption by reusing LUKS volume keys Coiby Xu
@ 2025-05-02 1:12 ` Coiby Xu
2025-08-21 11:15 ` Breno Leitao
2025-05-02 1:12 ` [PATCH v9 2/8] crash_dump: make dm crypt keys persist for the kdump kernel Coiby Xu
` (6 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Coiby Xu @ 2025-05-02 1:12 UTC (permalink / raw)
To: kexec
Cc: Ondrej Kozina, Milan Broz, Thomas Staudt,
Daniel P . Berrangé, Kairui Song, Pingfan Liu, Baoquan He,
Dave Young, linux-kernel, x86, Dave Hansen, Vitaly Kuznetsov,
Arnaud Lefebvre, Jan Pazdziora, Andrew Morton
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.
Note this feature is enabled only when CONFIG_CRASH_DUMP is enabled. So
it only takes effect for kdump and won't impact kexec reboot.
Suggested-by: Jan Pazdziora <jpazdziora@redhat.com>
Signed-off-by: Coiby Xu <coxu@redhat.com>
Acked-by: Baoquan He <bhe@redhat.com>
---
include/linux/kexec.h | 30 ++++++++++++++++++++++++++++++
kernel/kexec_file.c | 3 +++
2 files changed, 33 insertions(+)
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index c8971861521a..1871eaa95432 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -25,6 +25,10 @@
extern note_buf_t __percpu *crash_notes;
+#ifdef CONFIG_CRASH_DUMP
+#include <linux/prandom.h>
+#endif
+
#ifdef CONFIG_KEXEC_CORE
#include <linux/list.h>
#include <linux/compat.h>
@@ -169,6 +173,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,8 +185,33 @@ struct kexec_buf {
unsigned long buf_min;
unsigned long buf_max;
bool top_down;
+#ifdef CONFIG_CRASH_DUMP
+ bool random;
+#endif
};
+
+#ifdef CONFIG_CRASH_DUMP
+static inline void kexec_random_range_start(unsigned long start,
+ unsigned long end,
+ struct kexec_buf *kbuf,
+ unsigned long *temp_start)
+{
+ unsigned short i;
+
+ if (kbuf->random) {
+ get_random_bytes(&i, sizeof(unsigned short));
+ *temp_start = start + (end - start) / USHRT_MAX * i;
+ }
+}
+#else
+static inline void kexec_random_range_start(unsigned long start,
+ unsigned long end,
+ struct kexec_buf *kbuf,
+ unsigned long *temp_start)
+{}
+#endif
+
int kexec_load_purgatory(struct kimage *image, struct kexec_buf *kbuf);
int kexec_purgatory_get_set_symbol(struct kimage *image, const char *name,
void *buf, unsigned int size,
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index fba686487e3b..1180c0aa73f6 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -445,6 +445,7 @@ 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;
+ kexec_random_range_start(temp_start, temp_end, kbuf, &temp_start);
do {
/* align down start */
@@ -489,6 +490,8 @@ static int locate_mem_hole_bottom_up(unsigned long start, unsigned long end,
temp_start = max(start, kbuf->buf_min);
+ kexec_random_range_start(temp_start, end, kbuf, &temp_start);
+
do {
temp_start = ALIGN(temp_start, kbuf->buf_align);
temp_end = temp_start + kbuf->memsz - 1;
--
2.49.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v9 2/8] crash_dump: make dm crypt keys persist for the kdump kernel
2025-05-02 1:12 [PATCH v9 0/8] Support kdump with LUKS encryption by reusing LUKS volume keys Coiby Xu
2025-05-02 1:12 ` [PATCH v9 1/8] kexec_file: allow to place kexec_buf randomly Coiby Xu
@ 2025-05-02 1:12 ` Coiby Xu
2025-05-02 1:12 ` [PATCH v9 3/8] crash_dump: store dm crypt keys in kdump reserved memory Coiby Xu
` (5 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: Coiby Xu @ 2025-05-02 1:12 UTC (permalink / raw)
To: kexec
Cc: Ondrej Kozina, Milan Broz, Thomas Staudt,
Daniel P . Berrangé, Kairui Song, Pingfan Liu, Baoquan He,
Dave Young, linux-kernel, x86, Dave Hansen, Vitaly Kuznetsov,
Arnaud Lefebvre, Andrew Morton, Vivek Goyal, Jonathan Corbet,
open list:DOCUMENTATION
A configfs /sys/kernel/config/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 the kdump copies of LUKS volume keys,
1. After the 1st kernel loads the initramfs during boot, systemd uses
an user-input passphrase to de-crypt the LUKS volume keys or simply
TPM-sealed volume keys and then save the volume keys to specified
keyring (using the --link-vk-to-keyring API) and the keys will expire
within specified time.
2. A user space tool (kdump initramfs loader like kdump-utils) create
key items inside /sys/kernel/config/crash_dm_crypt_keys to inform
the 1st kernel which keys are needed.
3. When the kdump initramfs is loaded by the kexec_file_load
syscall, the 1st kernel will iterate created key items, save the
keys to kdump reserved memory.
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 yes to
/sys/kernel/crash_dm_crypt_keys/restore. Then the LUKS encrypted
device 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
Eventually the keys have to stay in the kdump reserved memory for the
kdump kernel to unlock encrypted volumes. During this process, some
measures like letting the keys expire within specified time are
desirable to reduce security risk.
This patch assumes,
1) there are 128 LUKS devices at maximum to be unlocked thus
MAX_KEY_NUM=128.
2) a key description won't exceed 128 bytes thus KEY_DESC_MAX_LEN=128.
And here is a demo on how to interact with
/sys/kernel/config/crash_dm_crypt_keys,
# Add key #1
mkdir /sys/kernel/config/crash_dm_crypt_keys/7d26b7b4-e342-4d2d-b660-7426b0996720
# Add key #1's description
echo cryptsetup:7d26b7b4-e342-4d2d-b660-7426b0996720 > /sys/kernel/config/crash_dm_crypt_keys/description
# how many keys do we have now?
cat /sys/kernel/config/crash_dm_crypt_keys/count
1
# Add key# 2 in the same way
# how many keys do we have now?
cat /sys/kernel/config/crash_dm_crypt_keys/count
2
# the tree structure of /crash_dm_crypt_keys configfs
tree /sys/kernel/config/crash_dm_crypt_keys/
/sys/kernel/config/crash_dm_crypt_keys/
├── 7d26b7b4-e342-4d2d-b660-7426b0996720
│ └── description
├── count
├── fce2cd38-4d59-4317-8ce2-1fd24d52c46a
│ └── description
Signed-off-by: Coiby Xu <coxu@redhat.com>
Acked-by: Baoquan He <bhe@redhat.com>
---
Documentation/admin-guide/kdump/kdump.rst | 28 ++++
kernel/Kconfig.kexec | 11 ++
kernel/Makefile | 1 +
kernel/crash_dump_dm_crypt.c | 154 ++++++++++++++++++++++
4 files changed, 194 insertions(+)
create mode 100644 kernel/crash_dump_dm_crypt.c
diff --git a/Documentation/admin-guide/kdump/kdump.rst b/Documentation/admin-guide/kdump/kdump.rst
index 1f7f14c6e184..b74d3bed8fff 100644
--- a/Documentation/admin-guide/kdump/kdump.rst
+++ b/Documentation/admin-guide/kdump/kdump.rst
@@ -547,6 +547,34 @@ from within add_taint() whenever the value set in this bitmask matches with the
bit flag being set by add_taint().
This will cause a kdump to occur at the add_taint()->panic() call.
+Write the dump file to encrypted disk volume
+============================================
+
+CONFIG_CRASH_DM_CRYPT can be enabled to support saving the dump file to an
+encrypted disk volume. User space can interact with
+/sys/kernel/config/crash_dm_crypt_keys for setup,
+
+1. Tell the first kernel what logon keys are needed to unlock the disk volumes,
+ # Add key #1
+ mkdir /sys/kernel/config/crash_dm_crypt_keys/7d26b7b4-e342-4d2d-b660-7426b0996720
+ # Add key #1's description
+ echo cryptsetup:7d26b7b4-e342-4d2d-b660-7426b0996720 > /sys/kernel/config/crash_dm_crypt_keys/description
+
+ # how many keys do we have now?
+ cat /sys/kernel/config/crash_dm_crypt_keys/count
+ 1
+
+ # Add key #2 in the same way
+
+ # how many keys do we have now?
+ cat /sys/kernel/config/crash_dm_crypt_keys/count
+ 2
+
+2. Load the dump-capture kernel
+
+3. After the dump-capture kerne get booted, restore the keys to user keyring
+ echo yes > /sys/kernel/crash_dm_crypt_keys/restore
+
Contact
=======
diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
index 4d111f871951..0a8fafd247d1 100644
--- a/kernel/Kconfig.kexec
+++ b/kernel/Kconfig.kexec
@@ -116,6 +116,17 @@ 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 KEXEC_FILE
+ depends on CRASH_DUMP
+ depends on DM_CRYPT
+ depends on CONFIGFS_FS
+ help
+ With this option enabled, user space can intereact with
+ /sys/kernel/config/crash_dm_crypt_keys to make the dm crypt keys
+ persistent for the dump-capture kernel.
+
config CRASH_HOTPLUG
bool "Update the crash elfcorehdr on system configuration changes"
default y
diff --git a/kernel/Makefile b/kernel/Makefile
index 434929de17ef..c7d3793107e5 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -77,6 +77,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..62a3c47d8b3b
--- /dev/null
+++ b/kernel/crash_dump_dm_crypt.c
@@ -0,0 +1,154 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <keys/user-type.h>
+#include <linux/crash_dump.h>
+#include <linux/configfs.h>
+#include <linux/module.h>
+
+#define KEY_NUM_MAX 128 /* maximum dm crypt keys */
+#define KEY_DESC_MAX_LEN 128 /* maximum dm crypt key description size */
+
+static unsigned int key_count;
+
+struct config_key {
+ struct config_item item;
+ const char *description;
+};
+
+static inline struct config_key *to_config_key(struct config_item *item)
+{
+ return container_of(item, struct config_key, item);
+}
+
+static ssize_t config_key_description_show(struct config_item *item, char *page)
+{
+ return sprintf(page, "%s\n", to_config_key(item)->description);
+}
+
+static ssize_t config_key_description_store(struct config_item *item,
+ const char *page, size_t count)
+{
+ struct config_key *config_key = to_config_key(item);
+ size_t len;
+ int ret;
+
+ ret = -EINVAL;
+ len = strcspn(page, "\n");
+
+ if (len > KEY_DESC_MAX_LEN) {
+ pr_err("The key description shouldn't exceed %u characters", KEY_DESC_MAX_LEN);
+ return ret;
+ }
+
+ if (!len)
+ return ret;
+
+ kfree(config_key->description);
+ ret = -ENOMEM;
+ config_key->description = kmemdup_nul(page, len, GFP_KERNEL);
+ if (!config_key->description)
+ return ret;
+
+ return count;
+}
+
+CONFIGFS_ATTR(config_key_, description);
+
+static struct configfs_attribute *config_key_attrs[] = {
+ &config_key_attr_description,
+ NULL,
+};
+
+static void config_key_release(struct config_item *item)
+{
+ kfree(to_config_key(item));
+ key_count--;
+}
+
+static struct configfs_item_operations config_key_item_ops = {
+ .release = config_key_release,
+};
+
+static const struct config_item_type config_key_type = {
+ .ct_item_ops = &config_key_item_ops,
+ .ct_attrs = config_key_attrs,
+ .ct_owner = THIS_MODULE,
+};
+
+static struct config_item *config_keys_make_item(struct config_group *group,
+ const char *name)
+{
+ struct config_key *config_key;
+
+ if (key_count > KEY_NUM_MAX) {
+ pr_err("Only %u keys at maximum to be created\n", KEY_NUM_MAX);
+ return ERR_PTR(-EINVAL);
+ }
+
+ config_key = kzalloc(sizeof(struct config_key), GFP_KERNEL);
+ if (!config_key)
+ return ERR_PTR(-ENOMEM);
+
+ config_item_init_type_name(&config_key->item, name, &config_key_type);
+
+ key_count++;
+
+ return &config_key->item;
+}
+
+static ssize_t config_keys_count_show(struct config_item *item, char *page)
+{
+ return sprintf(page, "%d\n", key_count);
+}
+
+CONFIGFS_ATTR_RO(config_keys_, count);
+
+static struct configfs_attribute *config_keys_attrs[] = {
+ &config_keys_attr_count,
+ NULL,
+};
+
+/*
+ * Note that, since no extra work is required on ->drop_item(),
+ * no ->drop_item() is provided.
+ */
+static struct configfs_group_operations config_keys_group_ops = {
+ .make_item = config_keys_make_item,
+};
+
+static const struct config_item_type config_keys_type = {
+ .ct_group_ops = &config_keys_group_ops,
+ .ct_attrs = config_keys_attrs,
+ .ct_owner = THIS_MODULE,
+};
+
+static struct configfs_subsystem config_keys_subsys = {
+ .su_group = {
+ .cg_item = {
+ .ci_namebuf = "crash_dm_crypt_keys",
+ .ci_type = &config_keys_type,
+ },
+ },
+};
+
+static int __init configfs_dmcrypt_keys_init(void)
+{
+ int ret;
+
+ config_group_init(&config_keys_subsys.su_group);
+ mutex_init(&config_keys_subsys.su_mutex);
+ ret = configfs_register_subsystem(&config_keys_subsys);
+ if (ret) {
+ pr_err("Error %d while registering subsystem %s\n", ret,
+ config_keys_subsys.su_group.cg_item.ci_namebuf);
+ goto out_unregister;
+ }
+
+ return 0;
+
+out_unregister:
+ configfs_unregister_subsystem(&config_keys_subsys);
+
+ return ret;
+}
+
+module_init(configfs_dmcrypt_keys_init);
--
2.49.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v9 3/8] crash_dump: store dm crypt keys in kdump reserved memory
2025-05-02 1:12 [PATCH v9 0/8] Support kdump with LUKS encryption by reusing LUKS volume keys Coiby Xu
2025-05-02 1:12 ` [PATCH v9 1/8] kexec_file: allow to place kexec_buf randomly Coiby Xu
2025-05-02 1:12 ` [PATCH v9 2/8] crash_dump: make dm crypt keys persist for the kdump kernel Coiby Xu
@ 2025-05-02 1:12 ` Coiby Xu
2025-05-02 1:12 ` [PATCH v9 4/8] crash_dump: reuse saved dm crypt keys for CPU/memory hot-plugging Coiby Xu
` (4 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: Coiby Xu @ 2025-05-02 1:12 UTC (permalink / raw)
To: kexec
Cc: Ondrej Kozina, Milan Broz, Thomas Staudt,
Daniel P . Berrangé, Kairui Song, Pingfan Liu, Baoquan He,
Dave Young, linux-kernel, x86, Dave Hansen, Vitaly Kuznetsov,
Arnaud Lefebvre, Andrew Morton, Vivek Goyal, Kees Cook,
Gustavo A. R. Silva,
open list:KERNEL HARDENING (not covered by other areas):Keyword:b__counted_by(_le|_be)?b
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.
Assume a key won't exceed 256 bytes thus MAX_KEY_SIZE=256 according to
"cryptsetup benchmark".
Signed-off-by: Coiby Xu <coxu@redhat.com>
Acked-by: Baoquan He <bhe@redhat.com>
---
include/linux/crash_core.h | 6 +-
include/linux/kexec.h | 4 ++
kernel/crash_dump_dm_crypt.c | 133 +++++++++++++++++++++++++++++++++++
3 files changed, 142 insertions(+), 1 deletion(-)
diff --git a/include/linux/crash_core.h b/include/linux/crash_core.h
index 44305336314e..2e6782239034 100644
--- a/include/linux/crash_core.h
+++ b/include/linux/crash_core.h
@@ -34,7 +34,11 @@ static inline void arch_kexec_protect_crashkres(void) { }
static inline void arch_kexec_unprotect_crashkres(void) { }
#endif
-
+#ifdef CONFIG_CRASH_DM_CRYPT
+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
static inline void arch_crash_handle_hotplug_event(struct kimage *image, void *arg) { }
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index 1871eaa95432..6e688c5d8e4d 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -405,6 +405,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 62a3c47d8b3b..fb25f55f1512 100644
--- a/kernel/crash_dump_dm_crypt.c
+++ b/kernel/crash_dump_dm_crypt.c
@@ -1,14 +1,62 @@
// 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>
#include <linux/configfs.h>
#include <linux/module.h>
#define KEY_NUM_MAX 128 /* maximum dm crypt keys */
+#define KEY_SIZE_MAX 256 /* maximum dm crypt key size */
#define KEY_DESC_MAX_LEN 128 /* maximum dm crypt key description size */
static unsigned int key_count;
+struct dm_crypt_key {
+ unsigned int key_size;
+ char key_desc[KEY_DESC_MAX_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(size_t total_keys)
+{
+ return struct_size(keys_header, keys, total_keys);
+}
+
+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 logon 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 logon key %s\n", dm_key->key_desc);
+ return PTR_ERR(key);
+ }
+
+ ukp = user_key_payload_locked(key);
+ if (!ukp)
+ return -EKEYREVOKED;
+
+ if (ukp->datalen > KEY_SIZE_MAX) {
+ pr_err("Key size %u exceeds maximum (%u)\n", ukp->datalen, KEY_SIZE_MAX);
+ return -EINVAL;
+ }
+
+ 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;
+}
+
struct config_key {
struct config_item item;
const char *description;
@@ -130,6 +178,91 @@ static struct configfs_subsystem config_keys_subsys = {
},
};
+static int build_keys_header(void)
+{
+ struct config_item *item = NULL;
+ struct config_key *key;
+ int i, r;
+
+ if (keys_header != NULL)
+ kvfree(keys_header);
+
+ keys_header = kzalloc(get_keys_header_size(key_count), GFP_KERNEL);
+ if (!keys_header)
+ return -ENOMEM;
+
+ keys_header->total_keys = key_count;
+
+ i = 0;
+ list_for_each_entry(item, &config_keys_subsys.su_group.cg_children,
+ ci_entry) {
+ if (item->ci_type != &config_key_type)
+ continue;
+
+ key = to_config_key(item);
+
+ if (!key->description) {
+ pr_warn("No key description for key %s\n", item->ci_name);
+ return -EINVAL;
+ }
+
+ strscpy(keys_header->keys[i].key_desc, key->description,
+ KEY_DESC_MAX_LEN);
+ r = read_key_from_user_keying(&keys_header->keys[i]);
+ if (r != 0) {
+ kexec_dprintk("Failed to read key %s\n",
+ keys_header->keys[i].key_desc);
+ return r;
+ }
+ i++;
+ kexec_dprintk("Found key: %s\n", item->ci_name);
+ }
+
+ 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 (key_count <= 0) {
+ kexec_dprintk("No dm-crypt keys\n");
+ return -ENOENT;
+ }
+
+ image->dm_crypt_keys_addr = 0;
+ r = build_keys_header();
+ if (r)
+ return r;
+
+ kbuf.buffer = keys_header;
+ kbuf.bufsz = get_keys_header_size(key_count);
+
+ 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;
+ }
+ image->dm_crypt_keys_addr = kbuf.mem;
+ image->dm_crypt_keys_sz = kbuf.bufsz;
+ kexec_dprintk(
+ "Loaded dm crypt keys to kexec_buffer bufsz=0x%lx memsz=0x%lx\n",
+ kbuf.bufsz, kbuf.memsz);
+
+ return r;
+}
+
static int __init configfs_dmcrypt_keys_init(void)
{
int ret;
--
2.49.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v9 4/8] crash_dump: reuse saved dm crypt keys for CPU/memory hot-plugging
2025-05-02 1:12 [PATCH v9 0/8] Support kdump with LUKS encryption by reusing LUKS volume keys Coiby Xu
` (2 preceding siblings ...)
2025-05-02 1:12 ` [PATCH v9 3/8] crash_dump: store dm crypt keys in kdump reserved memory Coiby Xu
@ 2025-05-02 1:12 ` Coiby Xu
2025-05-02 1:12 ` [PATCH v9 5/8] crash_dump: retrieve dm crypt keys in kdump kernel Coiby Xu
` (3 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: Coiby Xu @ 2025-05-02 1:12 UTC (permalink / raw)
To: kexec
Cc: Ondrej Kozina, Milan Broz, Thomas Staudt,
Daniel P . Berrangé, Kairui Song, Pingfan Liu, Baoquan He,
Dave Young, linux-kernel, x86, Dave Hansen, Vitaly Kuznetsov,
Arnaud Lefebvre, Andrew Morton, Vivek Goyal, Jonathan Corbet,
open list:DOCUMENTATION
When there are CPU and memory hot un/plugs, the dm crypt keys may need
to be reloaded again depending on the solution for crash hotplug
support. Currently, there are two solutions. 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 introduced in commit 247262756121 ("crash:
add generic infrastructure for crash hotplug support").
For the 1st solution, the dm crypt keys need to be reloaded again. The
user space can write true to
/sys/kernel/config/crash_dm_crypt_key/reuse so the stored keys can be
re-used.
For the 2nd solution, the dm crypt keys don't need to be reloaded.
Currently, only x86 supports the 2nd solution. If the 2nd solution
gets extended to all arches, this patch can be dropped.
Signed-off-by: Coiby Xu <coxu@redhat.com>
Acked-by: Baoquan He <bhe@redhat.com>
---
Documentation/admin-guide/kdump/kdump.rst | 4 ++
kernel/crash_dump_dm_crypt.c | 52 +++++++++++++++++++++--
2 files changed, 52 insertions(+), 4 deletions(-)
diff --git a/Documentation/admin-guide/kdump/kdump.rst b/Documentation/admin-guide/kdump/kdump.rst
index b74d3bed8fff..e25edaa8e533 100644
--- a/Documentation/admin-guide/kdump/kdump.rst
+++ b/Documentation/admin-guide/kdump/kdump.rst
@@ -570,6 +570,10 @@ encrypted disk volume. User space can interact with
cat /sys/kernel/config/crash_dm_crypt_keys/count
2
+ # To support CPU/memory hot-plugging, re-use keys already saved to reserved
+ # memory
+ echo true > /sys/kernel/config/crash_dm_crypt_key/reuse
+
2. Load the dump-capture kernel
3. After the dump-capture kerne get booted, restore the keys to user keyring
diff --git a/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c
index fb25f55f1512..5f4a62389150 100644
--- a/kernel/crash_dump_dm_crypt.c
+++ b/kernel/crash_dump_dm_crypt.c
@@ -28,6 +28,20 @@ static size_t get_keys_header_size(size_t total_keys)
return struct_size(keys_header, keys, total_keys);
}
+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, get_keys_header_size(key_count));
+ kunmap_local(keys_header_loaded);
+ arch_kexec_protect_crashkres();
+}
+
static int read_key_from_user_keying(struct dm_crypt_key *dm_key)
{
const struct user_key_payload *ukp;
@@ -150,8 +164,36 @@ static ssize_t config_keys_count_show(struct config_item *item, char *page)
CONFIGFS_ATTR_RO(config_keys_, count);
+static bool is_dm_key_reused;
+
+static ssize_t config_keys_reuse_show(struct config_item *item, char *page)
+{
+ return sprintf(page, "%d\n", is_dm_key_reused);
+}
+
+static ssize_t config_keys_reuse_store(struct config_item *item,
+ const char *page, size_t count)
+{
+ if (!kexec_crash_image || !kexec_crash_image->dm_crypt_keys_addr) {
+ kexec_dprintk(
+ "dm-crypt keys haven't be saved to crash-reserved memory\n");
+ return -EINVAL;
+ }
+
+ if (kstrtobool(page, &is_dm_key_reused))
+ return -EINVAL;
+
+ if (is_dm_key_reused)
+ get_keys_from_kdump_reserved_memory();
+
+ return count;
+}
+
+CONFIGFS_ATTR(config_keys_, reuse);
+
static struct configfs_attribute *config_keys_attrs[] = {
&config_keys_attr_count,
+ &config_keys_attr_reuse,
NULL,
};
@@ -238,10 +280,12 @@ int crash_load_dm_crypt_keys(struct kimage *image)
return -ENOENT;
}
- image->dm_crypt_keys_addr = 0;
- r = build_keys_header();
- if (r)
- return r;
+ if (!is_dm_key_reused) {
+ image->dm_crypt_keys_addr = 0;
+ r = build_keys_header();
+ if (r)
+ return r;
+ }
kbuf.buffer = keys_header;
kbuf.bufsz = get_keys_header_size(key_count);
--
2.49.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v9 5/8] crash_dump: retrieve dm crypt keys in kdump kernel
2025-05-02 1:12 [PATCH v9 0/8] Support kdump with LUKS encryption by reusing LUKS volume keys Coiby Xu
` (3 preceding siblings ...)
2025-05-02 1:12 ` [PATCH v9 4/8] crash_dump: reuse saved dm crypt keys for CPU/memory hot-plugging Coiby Xu
@ 2025-05-02 1:12 ` Coiby Xu
2025-05-02 1:12 ` [PATCH v9 6/8] Revert "x86/mm: Remove unused __set_memory_prot()" Coiby Xu
` (2 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: Coiby Xu @ 2025-05-02 1:12 UTC (permalink / raw)
To: kexec
Cc: Ondrej Kozina, Milan Broz, Thomas Staudt,
Daniel P . Berrangé, Kairui Song, Pingfan Liu, Baoquan He,
Dave Young, linux-kernel, x86, Dave Hansen, Vitaly Kuznetsov,
Arnaud Lefebvre, Andrew Morton, Vivek Goyal
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/config/crash_dm_crypt_key/restore, 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>
Acked-by: Baoquan He <bhe@redhat.com>
---
include/linux/crash_core.h | 1 +
include/linux/crash_dump.h | 2 +
kernel/crash_dump_dm_crypt.c | 133 +++++++++++++++++++++++++++++++++++
3 files changed, 136 insertions(+)
diff --git a/include/linux/crash_core.h b/include/linux/crash_core.h
index 2e6782239034..d35726d6a415 100644
--- a/include/linux/crash_core.h
+++ b/include/linux/crash_core.h
@@ -36,6 +36,7 @@ static inline void arch_kexec_unprotect_crashkres(void) { }
#ifdef CONFIG_CRASH_DM_CRYPT
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/include/linux/crash_dump.h b/include/linux/crash_dump.h
index 2f2555e6407c..dd6fc3b2133b 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/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c
index 5f4a62389150..401423ba477d 100644
--- a/kernel/crash_dump_dm_crypt.c
+++ b/kernel/crash_dump_dm_crypt.c
@@ -3,6 +3,7 @@
#include <linux/keyctl.h>
#include <keys/user-type.h>
#include <linux/crash_dump.h>
+#include <linux/cc_platform.h>
#include <linux/configfs.h>
#include <linux/module.h>
@@ -28,6 +29,61 @@ static size_t get_keys_header_size(size_t total_keys)
return struct_size(keys_header, keys, total_keys);
}
+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);
+
+/*
+ * 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, cc_platform_has(CC_ATTR_MEM_ENCRYPT));
+}
+
+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;
+}
+
static void get_keys_from_kdump_reserved_memory(void)
{
struct keys_header *keys_header_loaded;
@@ -42,6 +98,47 @@ static void get_keys_from_kdump_reserved_memory(void)
arch_kexec_protect_crashkres();
}
+static int restore_dm_crypt_keys_to_thread_keyring(void)
+{
+ struct dm_crypt_key *key;
+ size_t keys_header_size;
+ 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 the user keyring\n");
+ return PTR_ERR(keyring_ref);
+ }
+
+ addr = dm_crypt_keys_addr;
+ dm_crypt_keys_read((char *)&key_count, sizeof(key_count), &addr);
+ if (key_count < 0 || key_count > KEY_NUM_MAX) {
+ kexec_dprintk("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(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;
+}
+
static int read_key_from_user_keying(struct dm_crypt_key *dm_key)
{
const struct user_key_payload *ukp;
@@ -211,6 +308,37 @@ static const struct config_item_type config_keys_type = {
.ct_owner = THIS_MODULE,
};
+static bool restore;
+
+static ssize_t config_keys_restore_show(struct config_item *item, char *page)
+{
+ return sprintf(page, "%d\n", restore);
+}
+
+static ssize_t config_keys_restore_store(struct config_item *item,
+ const char *page, size_t count)
+{
+ if (!restore)
+ restore_dm_crypt_keys_to_thread_keyring();
+
+ if (kstrtobool(page, &restore))
+ return -EINVAL;
+
+ return count;
+}
+
+CONFIGFS_ATTR(config_keys_, restore);
+
+static struct configfs_attribute *kdump_config_keys_attrs[] = {
+ &config_keys_attr_restore,
+ NULL,
+};
+
+static const struct config_item_type kdump_config_keys_type = {
+ .ct_attrs = kdump_config_keys_attrs,
+ .ct_owner = THIS_MODULE,
+};
+
static struct configfs_subsystem config_keys_subsys = {
.su_group = {
.cg_item = {
@@ -311,6 +439,11 @@ static int __init configfs_dmcrypt_keys_init(void)
{
int ret;
+ if (is_kdump_kernel()) {
+ config_keys_subsys.su_group.cg_item.ci_type =
+ &kdump_config_keys_type;
+ }
+
config_group_init(&config_keys_subsys.su_group);
mutex_init(&config_keys_subsys.su_mutex);
ret = configfs_register_subsystem(&config_keys_subsys);
--
2.49.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v9 6/8] Revert "x86/mm: Remove unused __set_memory_prot()"
2025-05-02 1:12 [PATCH v9 0/8] Support kdump with LUKS encryption by reusing LUKS volume keys Coiby Xu
` (4 preceding siblings ...)
2025-05-02 1:12 ` [PATCH v9 5/8] crash_dump: retrieve dm crypt keys in kdump kernel Coiby Xu
@ 2025-05-02 1:12 ` Coiby Xu
2025-05-04 6:46 ` Andrew Morton
2025-05-02 1:12 ` [PATCH v9 7/8] x86/crash: pass dm crypt keys to kdump kernel Coiby Xu
2025-05-02 1:12 ` [PATCH v9 8/8] x86/crash: make the page that stores the dm crypt keys inaccessible Coiby Xu
7 siblings, 1 reply; 16+ messages in thread
From: Coiby Xu @ 2025-05-02 1:12 UTC (permalink / raw)
To: kexec
Cc: Ondrej Kozina, Milan Broz, Thomas Staudt,
Daniel P . Berrangé, Kairui Song, Pingfan Liu, Baoquan He,
Dave Young, linux-kernel, x86, Dave Hansen, Vitaly Kuznetsov,
Arnaud Lefebvre, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, H. Peter Anvin, Andy Lutomirski, Peter Zijlstra
This reverts commit 693bbf2a50447353c6a47961e6a7240a823ace02 as kdump
LUKS support (CONFIG_CRASH_DM_CRYPT) depends on __set_memory_prot.
Signed-off-by: Coiby Xu <coxu@redhat.com>
---
arch/x86/include/asm/set_memory.h | 1 +
arch/x86/mm/pat/set_memory.c | 13 +++++++++++++
2 files changed, 14 insertions(+)
diff --git a/arch/x86/include/asm/set_memory.h b/arch/x86/include/asm/set_memory.h
index 8d9f1c9aaa4c..023994fe6115 100644
--- a/arch/x86/include/asm/set_memory.h
+++ b/arch/x86/include/asm/set_memory.h
@@ -37,6 +37,7 @@ int set_memory_rox(unsigned long addr, int numpages);
* The caller is required to take care of these.
*/
+int __set_memory_prot(unsigned long addr, int numpages, pgprot_t prot);
int _set_memory_uc(unsigned long addr, int numpages);
int _set_memory_wc(unsigned long addr, int numpages);
int _set_memory_wt(unsigned long addr, int numpages);
diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
index def3d9284254..df7502ad165c 100644
--- a/arch/x86/mm/pat/set_memory.c
+++ b/arch/x86/mm/pat/set_memory.c
@@ -2148,6 +2148,19 @@ static inline int cpa_clear_pages_array(struct page **pages, int numpages,
CPA_PAGES_ARRAY, pages);
}
+/*
+ * __set_memory_prot is an internal helper for callers that have been passed
+ * a pgprot_t value from upper layers and a reservation has already been taken.
+ * If you want to set the pgprot to a specific page protocol, use the
+ * set_memory_xx() functions.
+ */
+int __set_memory_prot(unsigned long addr, int numpages, pgprot_t prot)
+{
+ return change_page_attr_set_clr(&addr, numpages, prot,
+ __pgprot(~pgprot_val(prot)), 0, 0,
+ NULL);
+}
+
int _set_memory_uc(unsigned long addr, int numpages)
{
/*
--
2.49.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v9 7/8] x86/crash: pass dm crypt keys to kdump kernel
2025-05-02 1:12 [PATCH v9 0/8] Support kdump with LUKS encryption by reusing LUKS volume keys Coiby Xu
` (5 preceding siblings ...)
2025-05-02 1:12 ` [PATCH v9 6/8] Revert "x86/mm: Remove unused __set_memory_prot()" Coiby Xu
@ 2025-05-02 1:12 ` Coiby Xu
2025-05-02 1:12 ` [PATCH v9 8/8] x86/crash: make the page that stores the dm crypt keys inaccessible Coiby Xu
7 siblings, 0 replies; 16+ messages in thread
From: Coiby Xu @ 2025-05-02 1:12 UTC (permalink / raw)
To: kexec
Cc: Ondrej Kozina, Milan Broz, Thomas Staudt,
Daniel P . Berrangé, Kairui Song, Pingfan Liu, Baoquan He,
Dave Young, linux-kernel, x86, Dave Hansen, Vitaly Kuznetsov,
Arnaud Lefebvre, Andrew Morton, Vivek Goyal, Jonathan Corbet,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
H. Peter Anvin, open list:DOCUMENTATION
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>
Acked-by: Baoquan He <bhe@redhat.com>
---
Documentation/admin-guide/kdump/kdump.rst | 4 ++--
arch/x86/kernel/crash.c | 26 +++++++++++++++++++++--
arch/x86/kernel/kexec-bzimage64.c | 21 ++++++++++++++++++
3 files changed, 47 insertions(+), 4 deletions(-)
diff --git a/Documentation/admin-guide/kdump/kdump.rst b/Documentation/admin-guide/kdump/kdump.rst
index e25edaa8e533..20fabdf6567e 100644
--- a/Documentation/admin-guide/kdump/kdump.rst
+++ b/Documentation/admin-guide/kdump/kdump.rst
@@ -551,8 +551,8 @@ Write the dump file to encrypted disk volume
============================================
CONFIG_CRASH_DM_CRYPT can be enabled to support saving the dump file to an
-encrypted disk volume. User space can interact with
-/sys/kernel/config/crash_dm_crypt_keys for setup,
+encrypted disk volume (only x86_64 supported for now). User space can interact
+with /sys/kernel/config/crash_dm_crypt_keys for setup,
1. Tell the first kernel what logon keys are needed to unlock the disk volumes,
# Add key #1
diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
index 0be61c45400c..bcb534688dfe 100644
--- a/arch/x86/kernel/crash.c
+++ b/arch/x86/kernel/crash.c
@@ -278,6 +278,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;
@@ -286,22 +287,43 @@ 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 nr_ranges = 0;
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));
+ /*
+ * Using random kexec_buf for passing dm crypt keys may cause a range
+ * split. So use two slots here.
+ */
+ nr_ranges = 2;
+ cmem = vzalloc(struct_size(cmem, ranges, nr_ranges));
if (!cmem)
return -ENOMEM;
+ cmem->max_nr_ranges = nr_ranges;
+ cmem->nr_ranges = 0;
+
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..c71cdd8e425a 100644
--- a/arch/x86/kernel/kexec-bzimage64.c
+++ b/arch/x86/kernel/kexec-bzimage64.c
@@ -27,6 +27,8 @@
#include <asm/kexec-bzimage64.h>
#define MAX_ELFCOREHDR_STR_LEN 30 /* elfcorehdr=0x<64bit-value> */
+#define MAX_DMCRYPTKEYS_STR_LEN 31 /* dmcryptkeys=0x<64bit-value> */
+
/*
* Defines lowest physical address for various segments. Not sure where
@@ -76,6 +78,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 +447,19 @@ 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 == -ENOENT) {
+ kexec_dprintk("No dm crypt key to load\n");
+ } else if (ret) {
+ pr_err("Failed to load dm crypt keys\n");
+ return ERR_PTR(ret);
+ }
+ if (image->dm_crypt_keys_addr &&
+ cmdline_len + MAX_ELFCOREHDR_STR_LEN + MAX_DMCRYPTKEYS_STR_LEN >
+ header->cmdline_size) {
+ pr_err("Appending dmcryptkeys=<addr> to command line exceeds maximum allowed length\n");
+ return ERR_PTR(-EINVAL);
+ }
}
#endif
@@ -468,6 +487,8 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
efi_map_sz = efi_get_runtime_map_size();
params_cmdline_sz = sizeof(struct boot_params) + cmdline_len +
MAX_ELFCOREHDR_STR_LEN;
+ if (image->dm_crypt_keys_addr)
+ params_cmdline_sz += MAX_DMCRYPTKEYS_STR_LEN;
params_cmdline_sz = ALIGN(params_cmdline_sz, 16);
kbuf.bufsz = params_cmdline_sz + ALIGN(efi_map_sz, 16) +
sizeof(struct setup_data) +
--
2.49.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v9 8/8] x86/crash: make the page that stores the dm crypt keys inaccessible
2025-05-02 1:12 [PATCH v9 0/8] Support kdump with LUKS encryption by reusing LUKS volume keys Coiby Xu
` (6 preceding siblings ...)
2025-05-02 1:12 ` [PATCH v9 7/8] x86/crash: pass dm crypt keys to kdump kernel Coiby Xu
@ 2025-05-02 1:12 ` Coiby Xu
7 siblings, 0 replies; 16+ messages in thread
From: Coiby Xu @ 2025-05-02 1:12 UTC (permalink / raw)
To: kexec
Cc: Ondrej Kozina, Milan Broz, Thomas Staudt,
Daniel P . Berrangé, Kairui Song, Pingfan Liu, Baoquan He,
Dave Young, linux-kernel, x86, Dave Hansen, Vitaly Kuznetsov,
Arnaud Lefebvre, 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>
Acked-by: Baoquan He <bhe@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 a68f5a0a9f37..f615fcb6d35d 100644
--- a/arch/x86/kernel/machine_kexec_64.c
+++ b/arch/x86/kernel/machine_kexec_64.c
@@ -598,13 +598,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.49.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v9 6/8] Revert "x86/mm: Remove unused __set_memory_prot()"
2025-05-02 1:12 ` [PATCH v9 6/8] Revert "x86/mm: Remove unused __set_memory_prot()" Coiby Xu
@ 2025-05-04 6:46 ` Andrew Morton
2025-05-07 3:05 ` Coiby Xu
0 siblings, 1 reply; 16+ messages in thread
From: Andrew Morton @ 2025-05-04 6:46 UTC (permalink / raw)
To: Coiby Xu
Cc: kexec, Ondrej Kozina, Milan Broz, Thomas Staudt,
Daniel P . Berrangé, Kairui Song, Pingfan Liu, Baoquan He,
Dave Young, linux-kernel, x86, Dave Hansen, Vitaly Kuznetsov,
Arnaud Lefebvre, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, H. Peter Anvin, Andy Lutomirski, Peter Zijlstra
On Fri, 2 May 2025 09:12:40 +0800 Coiby Xu <coxu@redhat.com> wrote:
> This reverts commit 693bbf2a50447353c6a47961e6a7240a823ace02 as kdump
> LUKS support (CONFIG_CRASH_DM_CRYPT) depends on __set_memory_prot.
>
x86_64 allmodconfig:
In file included from drivers/gpu/drm/i915/gt/intel_ggtt.c:6:
./arch/x86/include/asm/set_memory.h:40:57: error: unknown type name 'pgprot_t'
40 | int __set_memory_prot(unsigned long addr, int numpages, pgprot_t prot);
| ^~~~~~~~
I did this:
From: Andrew Morton <akpm@linux-foundation.org>
Subject: revert-x86-mm-remove-unused-__set_memory_prot-fix
Date: Sat May 3 11:38:32 PM PDT 2025
x86 set_memory.h needs pgtable_types.h for pgprot_t. Obtain it via the
higher-level pgtable.h.
Cc: Baoquan He <bhe@redhat.com>
Cc: Coiby Xu <coxu@redhat.com>
Cc: "Daniel P. Berrange" <berrange@redhat.com>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Dave Young <dyoung@redhat.com>
Cc: Jan Pazdziora <jpazdziora@redhat.com>
Cc: Liu Pingfan <kernelfans@gmail.com>
Cc: Milan Broz <gmazyland@gmail.com>
Cc: Ondrej Kozina <okozina@redhat.com>
Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
arch/x86/include/asm/set_memory.h | 1 +
1 file changed, 1 insertion(+)
--- a/arch/x86/include/asm/set_memory.h~revert-x86-mm-remove-unused-__set_memory_prot-fix
+++ a/arch/x86/include/asm/set_memory.h
@@ -4,6 +4,7 @@
#include <asm/page.h>
#include <asm-generic/set_memory.h>
+#include <asm/pgtable.h>
#define set_memory_rox set_memory_rox
int set_memory_rox(unsigned long addr, int numpages);
_
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v9 6/8] Revert "x86/mm: Remove unused __set_memory_prot()"
2025-05-04 6:46 ` Andrew Morton
@ 2025-05-07 3:05 ` Coiby Xu
0 siblings, 0 replies; 16+ messages in thread
From: Coiby Xu @ 2025-05-07 3:05 UTC (permalink / raw)
To: Andrew Morton
Cc: kexec, Ondrej Kozina, Milan Broz, Thomas Staudt,
Daniel P . Berrangé, Kairui Song, Pingfan Liu, Baoquan He,
Dave Young, linux-kernel, x86, Dave Hansen, Vitaly Kuznetsov,
Arnaud Lefebvre, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, H. Peter Anvin, Andy Lutomirski, Peter Zijlstra
On Sat, May 03, 2025 at 11:46:05PM -0700, Andrew Morton wrote:
>On Fri, 2 May 2025 09:12:40 +0800 Coiby Xu <coxu@redhat.com> wrote:
>
>> This reverts commit 693bbf2a50447353c6a47961e6a7240a823ace02 as kdump
>> LUKS support (CONFIG_CRASH_DM_CRYPT) depends on __set_memory_prot.
>>
>
>x86_64 allmodconfig:
>
>In file included from drivers/gpu/drm/i915/gt/intel_ggtt.c:6:
>./arch/x86/include/asm/set_memory.h:40:57: error: unknown type name 'pgprot_t'
> 40 | int __set_memory_prot(unsigned long addr, int numpages, pgprot_t prot);
> | ^~~~~~~~
>
>I did this:
>
>
>From: Andrew Morton <akpm@linux-foundation.org>
>Subject: revert-x86-mm-remove-unused-__set_memory_prot-fix
>Date: Sat May 3 11:38:32 PM PDT 2025
>
>x86 set_memory.h needs pgtable_types.h for pgprot_t. Obtain it via the
>higher-level pgtable.h.
>
>Cc: Baoquan He <bhe@redhat.com>
>Cc: Coiby Xu <coxu@redhat.com>
>Cc: "Daniel P. Berrange" <berrange@redhat.com>
>Cc: Dave Hansen <dave.hansen@intel.com>
>Cc: Dave Young <dyoung@redhat.com>
>Cc: Jan Pazdziora <jpazdziora@redhat.com>
>Cc: Liu Pingfan <kernelfans@gmail.com>
>Cc: Milan Broz <gmazyland@gmail.com>
>Cc: Ondrej Kozina <okozina@redhat.com>
>Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
>Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
>---
>
> arch/x86/include/asm/set_memory.h | 1 +
> 1 file changed, 1 insertion(+)
>
>--- a/arch/x86/include/asm/set_memory.h~revert-x86-mm-remove-unused-__set_memory_prot-fix
>+++ a/arch/x86/include/asm/set_memory.h
>@@ -4,6 +4,7 @@
>
> #include <asm/page.h>
> #include <asm-generic/set_memory.h>
>+#include <asm/pgtable.h>
>
> #define set_memory_rox set_memory_rox
> int set_memory_rox(unsigned long addr, int numpages);
>_
Thanks for catching this issue and providing a clean fix! I'll make sure
future patches pass allmodconfig. And also thanks for pulling the patch
set into -mm tree!
--
Best regards,
Coiby
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v9 1/8] kexec_file: allow to place kexec_buf randomly
2025-05-02 1:12 ` [PATCH v9 1/8] kexec_file: allow to place kexec_buf randomly Coiby Xu
@ 2025-08-21 11:15 ` Breno Leitao
2025-08-25 1:18 ` Coiby Xu
0 siblings, 1 reply; 16+ messages in thread
From: Breno Leitao @ 2025-08-21 11:15 UTC (permalink / raw)
To: Coiby Xu
Cc: kexec, Ondrej Kozina, Milan Broz, Thomas Staudt,
Daniel P . Berrangé, Kairui Song, Pingfan Liu, Baoquan He,
Dave Young, linux-kernel, x86, Dave Hansen, Vitaly Kuznetsov,
Arnaud Lefebvre, Jan Pazdziora, Andrew Morton, linux-arm-kernel
Hello Coiby,
On Fri, May 02, 2025 at 09:12:35AM +0800, Coiby Xu wrote:
> +static inline void kexec_random_range_start(unsigned long start,
> + unsigned long end,
> + struct kexec_buf *kbuf,
> + unsigned long *temp_start)
> +{
> + unsigned short i;
> +
> + if (kbuf->random) {
> + get_random_bytes(&i, sizeof(unsigned short));
> + *temp_start = start + (end - start) / USHRT_MAX * i;
> + }
> +}
On arm64, I am getting the following UBSAN warning when accessing
kbuf->random:
[ 32.362428] ------------[ cut here ]------------
[ 32.362488] UBSAN: invalid-load in ./include/linux/kexec.h:210:10
[ 32.362649] load of value 252 is not a valid value for type '_Bool'
and line 210 is your `if (kbuf->random)`.
Basically kbuf was not initialized in arm hosts, and probably has
garbage.
I am wondering if we should have something like , while the support for arm64 is
not done:
commit 2608bd8c26b62a9a7cc50106e93d3a1ffb1e1188
Author: Breno Leitao <leitao@debian.org>
Date: Thu Aug 21 04:11:21 2025 -0700
Initialize the random field of kbuf to zero in the ARM64 kexec image loader
Ads an explicit initialization for the random member of the kbuf
structure within the image_load function in
arch/arm64/kernel/kexec_image.c. Setting kbuf.random to zero ensures
a deterministic and clean starting state for the buffer used during
kernel image loading, avoiding this UBSAN issue later, when kbuf.random
is read.
[ 32.362488] UBSAN: invalid-load in ./include/linux/kexec.h:210:10
[ 32.362649] load of value 252 is not a valid value for type '_Bool'
Signed-off-by: Breno Leitao <leitao@debian.org>
diff --git a/arch/arm64/kernel/kexec_image.c b/arch/arm64/kernel/kexec_image.c
index 532d72ea42ee8..287b25e674d76 100644
--- a/arch/arm64/kernel/kexec_image.c
+++ b/arch/arm64/kernel/kexec_image.c
@@ -76,6 +76,7 @@ static void *image_load(struct kimage *image,
kbuf.buf_min = 0;
kbuf.buf_max = ULONG_MAX;
kbuf.top_down = false;
+ kbuf.random = 0;
kbuf.buffer = kernel;
kbuf.bufsz = kernel_len;
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v9 1/8] kexec_file: allow to place kexec_buf randomly
2025-08-21 11:15 ` Breno Leitao
@ 2025-08-25 1:18 ` Coiby Xu
2025-08-26 1:05 ` Andrew Morton
0 siblings, 1 reply; 16+ messages in thread
From: Coiby Xu @ 2025-08-25 1:18 UTC (permalink / raw)
To: Breno Leitao
Cc: kexec, Ondrej Kozina, Milan Broz, Thomas Staudt,
Daniel P . Berrangé, Kairui Song, Pingfan Liu, Baoquan He,
Dave Young, linux-kernel, x86, Dave Hansen, Vitaly Kuznetsov,
Arnaud Lefebvre, Jan Pazdziora, Andrew Morton, linux-arm-kernel
On Thu, Aug 21, 2025 at 04:15:53AM -0700, Breno Leitao wrote:
>Hello Coiby,
Hi Breno,
>
>On Fri, May 02, 2025 at 09:12:35AM +0800, Coiby Xu wrote:
>> +static inline void kexec_random_range_start(unsigned long start,
>> + unsigned long end,
>> + struct kexec_buf *kbuf,
>> + unsigned long *temp_start)
>> +{
>> + unsigned short i;
>> +
>> + if (kbuf->random) {
>> + get_random_bytes(&i, sizeof(unsigned short));
>> + *temp_start = start + (end - start) / USHRT_MAX * i;
>> + }
>> +}
>
>On arm64, I am getting the following UBSAN warning when accessing
>kbuf->random:
>
>[ 32.362428] ------------[ cut here ]------------
>[ 32.362488] UBSAN: invalid-load in ./include/linux/kexec.h:210:10
>[ 32.362649] load of value 252 is not a valid value for type '_Bool'
>
>and line 210 is your `if (kbuf->random)`.
>
>Basically kbuf was not initialized in arm hosts, and probably has
>garbage.
Thank for explaining the problem to me!
>
>I am wondering if we should have something like , while the support for arm64 is
>not done:
>
>commit 2608bd8c26b62a9a7cc50106e93d3a1ffb1e1188
>Author: Breno Leitao <leitao@debian.org>
>Date: Thu Aug 21 04:11:21 2025 -0700
>
> Initialize the random field of kbuf to zero in the ARM64 kexec image loader
>
> Ads an explicit initialization for the random member of the kbuf
> structure within the image_load function in
> arch/arm64/kernel/kexec_image.c. Setting kbuf.random to zero ensures
> a deterministic and clean starting state for the buffer used during
> kernel image loading, avoiding this UBSAN issue later, when kbuf.random
> is read.
>
> [ 32.362488] UBSAN: invalid-load in ./include/linux/kexec.h:210:10
> [ 32.362649] load of value 252 is not a valid value for type '_Bool'
>
> Signed-off-by: Breno Leitao <leitao@debian.org>
>
>diff --git a/arch/arm64/kernel/kexec_image.c b/arch/arm64/kernel/kexec_image.c
>index 532d72ea42ee8..287b25e674d76 100644
>--- a/arch/arm64/kernel/kexec_image.c
>+++ b/arch/arm64/kernel/kexec_image.c
>@@ -76,6 +76,7 @@ static void *image_load(struct kimage *image,
> kbuf.buf_min = 0;
> kbuf.buf_max = ULONG_MAX;
> kbuf.top_down = false;
>+ kbuf.random = 0;
>
> kbuf.buffer = kernel;
> kbuf.bufsz = kernel_len;
>
And also thanks for posing a fix! The patch LGTM. Can you add a Fixes
tag 'Fixes: bf454ec31add ("kexec_file: allow to place kexec_buf
randomly")' and then send it to kexec@lists.infradead.org? Thanks!
--
Best regards,
Coiby
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v9 1/8] kexec_file: allow to place kexec_buf randomly
2025-08-25 1:18 ` Coiby Xu
@ 2025-08-26 1:05 ` Andrew Morton
2025-08-27 11:38 ` Baoquan He
0 siblings, 1 reply; 16+ messages in thread
From: Andrew Morton @ 2025-08-26 1:05 UTC (permalink / raw)
To: Coiby Xu
Cc: Breno Leitao, kexec, Ondrej Kozina, Milan Broz, Thomas Staudt,
Daniel P . Berrangé, Kairui Song, Pingfan Liu, Baoquan He,
Dave Young, linux-kernel, x86, Dave Hansen, Vitaly Kuznetsov,
Arnaud Lefebvre, Jan Pazdziora, linux-arm-kernel
On Mon, 25 Aug 2025 09:18:53 +0800 Coiby Xu <coxu@redhat.com> wrote:
> >diff --git a/arch/arm64/kernel/kexec_image.c b/arch/arm64/kernel/kexec_image.c
> >index 532d72ea42ee8..287b25e674d76 100644
> >--- a/arch/arm64/kernel/kexec_image.c
> >+++ b/arch/arm64/kernel/kexec_image.c
> >@@ -76,6 +76,7 @@ static void *image_load(struct kimage *image,
> > kbuf.buf_min = 0;
> > kbuf.buf_max = ULONG_MAX;
> > kbuf.top_down = false;
> >+ kbuf.random = 0;
> >
> > kbuf.buffer = kernel;
> > kbuf.bufsz = kernel_len;
> >
>
> And also thanks for posing a fix! The patch LGTM. Can you add a Fixes
> tag 'Fixes: bf454ec31add ("kexec_file: allow to place kexec_buf
> randomly")' and then send it to kexec@lists.infradead.org? Thanks!
I turned all this into a regular patch and queued it (see below),
thanks. No additional actions are needed.
I'm really not liking that code. I laboriously verified that all
fields of kexec_buf are now initialized, except for `cma'. Is that a
bug?
This function has a call frequency of about 3x per week. Can we please
just memset the whole thing so people don't have to worry about this
any more?
From: Breno Leitao <leitao@debian.org>
Subject: kexec/arm64: initialize the random field of kbuf to zero in the image loader
Date: Thu Aug 21 04:11:21 2025 -0700
Add an explicit initialization for the random member of the kbuf structure
within the image_load function in arch/arm64/kernel/kexec_image.c.
Setting kbuf.random to zero ensures a deterministic and clean starting
state for the buffer used during kernel image loading, avoiding this UBSAN
issue later, when kbuf.random is read.
[ 32.362488] UBSAN: invalid-load in ./include/linux/kexec.h:210:10
[ 32.362649] load of value 252 is not a valid value for type '_Bool'
Link: https://lkml.kernel.org/r/oninomspajhxp4omtdapxnckxydbk2nzmrix7rggmpukpnzadw@c67o7njgdgm3
Fixes: bf454ec31add ("kexec_file: allow to place kexec_buf randomly
Signed-off-by: Breno Leitao <leitao@debian.org>
Cc: Baoquan He <bhe@redhat.com>
Cc: Coiby Xu <coxu@redhat.com>
Cc: "Daniel P. Berrange" <berrange@redhat.com>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Dave Young <dyoung@redhat.com>
Cc: Kairui Song <ryncsn@gmail.com>
Cc: Liu Pingfan <kernelfans@gmail.com>
Cc: Milan Broz <gmazyland@gmail.com>
Cc: Ondrej Kozina <okozina@redhat.com>
Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
arch/arm64/kernel/kexec_image.c | 1 +
1 file changed, 1 insertion(+)
--- a/arch/arm64/kernel/kexec_image.c~kexec-arm64-initialize-the-random-field-of-kbuf-to-zero-in-the-image-loader
+++ a/arch/arm64/kernel/kexec_image.c
@@ -76,6 +76,7 @@ static void *image_load(struct kimage *i
kbuf.buf_min = 0;
kbuf.buf_max = ULONG_MAX;
kbuf.top_down = false;
+ kbuf.random = 0;
kbuf.buffer = kernel;
kbuf.bufsz = kernel_len;
_
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v9 1/8] kexec_file: allow to place kexec_buf randomly
2025-08-26 1:05 ` Andrew Morton
@ 2025-08-27 11:38 ` Baoquan He
2025-08-27 13:38 ` Breno Leitao
0 siblings, 1 reply; 16+ messages in thread
From: Baoquan He @ 2025-08-27 11:38 UTC (permalink / raw)
To: Andrew Morton
Cc: Coiby Xu, Breno Leitao, kexec, Ondrej Kozina, Milan Broz,
Thomas Staudt, Daniel P . Berrangé, Kairui Song, Pingfan Liu,
Dave Young, linux-kernel, x86, Dave Hansen, Vitaly Kuznetsov,
Arnaud Lefebvre, Jan Pazdziora, linux-arm-kernel
On 08/25/25 at 06:05pm, Andrew Morton wrote:
> On Mon, 25 Aug 2025 09:18:53 +0800 Coiby Xu <coxu@redhat.com> wrote:
>
> > >diff --git a/arch/arm64/kernel/kexec_image.c b/arch/arm64/kernel/kexec_image.c
> > >index 532d72ea42ee8..287b25e674d76 100644
> > >--- a/arch/arm64/kernel/kexec_image.c
> > >+++ b/arch/arm64/kernel/kexec_image.c
> > >@@ -76,6 +76,7 @@ static void *image_load(struct kimage *image,
> > > kbuf.buf_min = 0;
> > > kbuf.buf_max = ULONG_MAX;
> > > kbuf.top_down = false;
> > >+ kbuf.random = 0;
> > >
> > > kbuf.buffer = kernel;
> > > kbuf.bufsz = kernel_len;
> > >
> >
> > And also thanks for posing a fix! The patch LGTM. Can you add a Fixes
> > tag 'Fixes: bf454ec31add ("kexec_file: allow to place kexec_buf
> > randomly")' and then send it to kexec@lists.infradead.org? Thanks!
>
> I turned all this into a regular patch and queued it (see below),
> thanks. No additional actions are needed.
>
> I'm really not liking that code. I laboriously verified that all
> fields of kexec_buf are now initialized, except for `cma'. Is that a
> bug?
>
> This function has a call frequency of about 3x per week. Can we please
> just memset the whole thing so people don't have to worry about this
> any more?
Yeah, adding these trivial patches to mute XXSAN warning is annoying.
Maybe arm64 can initialize the local variable kbuf like we do in x86_64
as below, to explicitly set the necessary fields when defining.
static void *bzImage64_load(struct kimage *image, char *kernel,
unsigned long kernel_len, char *initrd,
unsigned long initrd_len, char *cmdline,
unsigned long cmdline_len)
{
......
struct kexec_buf kbuf = { .image = image, .buf_max = ULONG_MAX,
.top_down = true };
struct kexec_buf pbuf = { .image = image, .buf_min = MIN_PURGATORY_ADDR,
.buf_max = ULONG_MAX, .top_down = true };
.....
}
>
>
> From: Breno Leitao <leitao@debian.org>
> Subject: kexec/arm64: initialize the random field of kbuf to zero in the image loader
> Date: Thu Aug 21 04:11:21 2025 -0700
>
> Add an explicit initialization for the random member of the kbuf structure
> within the image_load function in arch/arm64/kernel/kexec_image.c.
> Setting kbuf.random to zero ensures a deterministic and clean starting
> state for the buffer used during kernel image loading, avoiding this UBSAN
> issue later, when kbuf.random is read.
>
> [ 32.362488] UBSAN: invalid-load in ./include/linux/kexec.h:210:10
> [ 32.362649] load of value 252 is not a valid value for type '_Bool'
>
> Link: https://lkml.kernel.org/r/oninomspajhxp4omtdapxnckxydbk2nzmrix7rggmpukpnzadw@c67o7njgdgm3
> Fixes: bf454ec31add ("kexec_file: allow to place kexec_buf randomly
> Signed-off-by: Breno Leitao <leitao@debian.org>
> Cc: Baoquan He <bhe@redhat.com>
> Cc: Coiby Xu <coxu@redhat.com>
> Cc: "Daniel P. Berrange" <berrange@redhat.com>
> Cc: Dave Hansen <dave.hansen@intel.com>
> Cc: Dave Young <dyoung@redhat.com>
> Cc: Kairui Song <ryncsn@gmail.com>
> Cc: Liu Pingfan <kernelfans@gmail.com>
> Cc: Milan Broz <gmazyland@gmail.com>
> Cc: Ondrej Kozina <okozina@redhat.com>
> Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
>
> arch/arm64/kernel/kexec_image.c | 1 +
> 1 file changed, 1 insertion(+)
>
> --- a/arch/arm64/kernel/kexec_image.c~kexec-arm64-initialize-the-random-field-of-kbuf-to-zero-in-the-image-loader
> +++ a/arch/arm64/kernel/kexec_image.c
> @@ -76,6 +76,7 @@ static void *image_load(struct kimage *i
> kbuf.buf_min = 0;
> kbuf.buf_max = ULONG_MAX;
> kbuf.top_down = false;
> + kbuf.random = 0;
>
> kbuf.buffer = kernel;
> kbuf.bufsz = kernel_len;
> _
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v9 1/8] kexec_file: allow to place kexec_buf randomly
2025-08-27 11:38 ` Baoquan He
@ 2025-08-27 13:38 ` Breno Leitao
0 siblings, 0 replies; 16+ messages in thread
From: Breno Leitao @ 2025-08-27 13:38 UTC (permalink / raw)
To: Baoquan He
Cc: Andrew Morton, Coiby Xu, kexec, Ondrej Kozina, Milan Broz,
Thomas Staudt, Daniel P . Berrangé, Kairui Song, Pingfan Liu,
Dave Young, linux-kernel, x86, Dave Hansen, Vitaly Kuznetsov,
Arnaud Lefebvre, Jan Pazdziora, linux-arm-kernel
On Wed, Aug 27, 2025 at 07:38:29PM +0800, Baoquan He wrote:
> On 08/25/25 at 06:05pm, Andrew Morton wrote:
> > On Mon, 25 Aug 2025 09:18:53 +0800 Coiby Xu <coxu@redhat.com> wrote:
> >
> > > >diff --git a/arch/arm64/kernel/kexec_image.c b/arch/arm64/kernel/kexec_image.c
> > > >index 532d72ea42ee8..287b25e674d76 100644
> > > >--- a/arch/arm64/kernel/kexec_image.c
> > > >+++ b/arch/arm64/kernel/kexec_image.c
> > > >@@ -76,6 +76,7 @@ static void *image_load(struct kimage *image,
> > > > kbuf.buf_min = 0;
> > > > kbuf.buf_max = ULONG_MAX;
> > > > kbuf.top_down = false;
> > > >+ kbuf.random = 0;
> > > >
> > > > kbuf.buffer = kernel;
> > > > kbuf.bufsz = kernel_len;
> > > >
> > >
> > > And also thanks for posing a fix! The patch LGTM. Can you add a Fixes
> > > tag 'Fixes: bf454ec31add ("kexec_file: allow to place kexec_buf
> > > randomly")' and then send it to kexec@lists.infradead.org? Thanks!
> >
> > I turned all this into a regular patch and queued it (see below),
> > thanks. No additional actions are needed.
> >
> > I'm really not liking that code. I laboriously verified that all
> > fields of kexec_buf are now initialized, except for `cma'. Is that a
> > bug?
> >
> > This function has a call frequency of about 3x per week. Can we please
> > just memset the whole thing so people don't have to worry about this
> > any more?
>
> Yeah, adding these trivial patches to mute XXSAN warning is annoying.
The patchset is quite simple, tho:
https://lore.kernel.org/all/20250827-kbuf_all-v1-0-1df9882bb01a@debian.org/
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2025-08-27 13:38 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-02 1:12 [PATCH v9 0/8] Support kdump with LUKS encryption by reusing LUKS volume keys Coiby Xu
2025-05-02 1:12 ` [PATCH v9 1/8] kexec_file: allow to place kexec_buf randomly Coiby Xu
2025-08-21 11:15 ` Breno Leitao
2025-08-25 1:18 ` Coiby Xu
2025-08-26 1:05 ` Andrew Morton
2025-08-27 11:38 ` Baoquan He
2025-08-27 13:38 ` Breno Leitao
2025-05-02 1:12 ` [PATCH v9 2/8] crash_dump: make dm crypt keys persist for the kdump kernel Coiby Xu
2025-05-02 1:12 ` [PATCH v9 3/8] crash_dump: store dm crypt keys in kdump reserved memory Coiby Xu
2025-05-02 1:12 ` [PATCH v9 4/8] crash_dump: reuse saved dm crypt keys for CPU/memory hot-plugging Coiby Xu
2025-05-02 1:12 ` [PATCH v9 5/8] crash_dump: retrieve dm crypt keys in kdump kernel Coiby Xu
2025-05-02 1:12 ` [PATCH v9 6/8] Revert "x86/mm: Remove unused __set_memory_prot()" Coiby Xu
2025-05-04 6:46 ` Andrew Morton
2025-05-07 3:05 ` Coiby Xu
2025-05-02 1:12 ` [PATCH v9 7/8] x86/crash: pass dm crypt keys to kdump kernel Coiby Xu
2025-05-02 1:12 ` [PATCH v9 8/8] x86/crash: make the page that stores the dm crypt keys inaccessible Coiby Xu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).