If kexec_add_buffer somehow fails, keys_header will be freed. Depending
on /sys/kernel/config/crash_dm_crypt_key/reuse, it will lead to the
following two problems if the kexec_file_load syscall is called again,
1. Double free of keys_header if reuse=false
2. UAF of keys_header if reuse=true
To address these problems and also make it easier to reason about the
code, keep two invariants,
1. keys_header will always be freed at the end of kexec_file_load
syscall except during kdump image unloading for CPU/memory
hot-plugging support
2. There will always be valid keys_header if reuse=true
Fixes: 479e58549b0f ("crash_dump: store dm crypt keys in kdump reserved memory")
Fixes: 9ebfa8dcaea7 ("crash_dump: reuse saved dm crypt keys for CPU/memory
hot-plugging")
Reported-by: Sourabh Jain <[email protected]>
Signed-off-by: Coiby Xu <[email protected]>
---
include/linux/kexec.h | 6 ++++
kernel/crash_dump_dm_crypt.c | 66 ++++++++++++++++++++++++++----------
kernel/kexec_file.c | 2 ++
3 files changed, 56 insertions(+), 18 deletions(-)
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index 8a22bc9b8c6c..91256d7ff434 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -552,6 +552,12 @@ void set_kexec_sig_enforced(void);
static inline void set_kexec_sig_enforced(void) {}
#endif
+#ifdef CONFIG_CRASH_DM_CRYPT
+void kexec_file_post_load_cleanup_dm_crypt(struct kimage *image);
+#else
+static inline void kexec_file_post_load_cleanup_dm_crypt(struct kimage *image)
{}
+#endif
+
#endif /* !defined(__ASSEBMLY__) */
#endif /* LINUX_KEXEC_H */
diff --git a/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c
index c685497cd470..4335b6cb1fc4 100644
--- a/kernel/crash_dump_dm_crypt.c
+++ b/kernel/crash_dump_dm_crypt.c
@@ -84,18 +84,25 @@ static int add_key_to_keyring(struct dm_crypt_key *dm_key,
return r;
}
-static void get_keys_from_kdump_reserved_memory(void)
+static int get_keys_from_kdump_reserved_memory(void)
{
struct keys_header *keys_header_loaded;
+ size_t keys_header_size;
- arch_kexec_unprotect_crashkres();
+ keys_header_size = get_keys_header_size(key_count);
+ keys_header = kzalloc(keys_header_size, GFP_KERNEL);
+ if (!keys_header)
+ return -ENOMEM;
+ arch_kexec_unprotect_crashkres();
keys_header_loaded = kmap_local_page(pfn_to_page(
kexec_crash_image->dm_crypt_keys_addr >> PAGE_SHIFT));
- memcpy(keys_header, keys_header_loaded,
get_keys_header_size(key_count));
+ memcpy(keys_header, keys_header_loaded, keys_header_size);
kunmap_local(keys_header_loaded);
arch_kexec_protect_crashkres();
+
+ return 0;
}
static int restore_dm_crypt_keys_to_thread_keyring(void)
@@ -286,17 +293,28 @@ static ssize_t config_keys_reuse_show(struct config_item
*item, char *page)
static ssize_t config_keys_reuse_store(struct config_item *item,
const char *page, size_t count)
{
+ bool val;
+ int r;
+
if (!kexec_crash_image || !kexec_crash_image->dm_crypt_keys_addr) {
kexec_dprintk(
"dm-crypt keys haven't be saved to crash-reserved
memory\n");
return -EINVAL;
}
- if (kstrtobool(page, &is_dm_key_reused))
+ if (kstrtobool(page, &val) || !val)
return -EINVAL;
- if (is_dm_key_reused)
- get_keys_from_kdump_reserved_memory();
+ if (is_dm_key_reused) {
+ pr_info("Already got dm-crypt keys, please continue with
kexec_file_load syscall\n");
+ } else {
+ r = get_keys_from_kdump_reserved_memory();
+ if (r) {
+ pr_warn("Failed to get dm-crypt keys from reserved
memory\n");
+ return r;
+ }
+ is_dm_key_reused = true;
+ }
return count;
}
@@ -369,9 +387,6 @@ static int build_keys_header(void)
struct config_key *key;
int i, r;
- if (keys_header != NULL)
- kvfree(keys_header);
-
keys_header = kzalloc(get_keys_header_size(key_count), GFP_KERNEL);
if (!keys_header)
return -ENOMEM;
@@ -415,8 +430,7 @@ int crash_load_dm_crypt_keys(struct kimage *image)
.top_down = false,
.random = true,
};
- int r;
-
+ int r = 0;
if (key_count <= 0) {
kexec_dprintk("No dm-crypt keys\n");
@@ -424,14 +438,15 @@ int crash_load_dm_crypt_keys(struct kimage *image)
}
if (!is_dm_key_reused) {
- image->dm_crypt_keys_addr = 0;
r = build_keys_header();
- if (r) {
- pr_err("Failed to build dm-crypt keys header,
ret=%d\n", r);
- return r;
- }
+ if (r)
+ goto out;
}
+ /*
+ * keys_header will be copied to reserver memory later and then be
+ * cleaned up at the end of kexec_file_load syscall
+ */
kbuf.buffer = keys_header;
kbuf.bufsz = get_keys_header_size(key_count);
@@ -441,18 +456,33 @@ int crash_load_dm_crypt_keys(struct kimage *image)
r = kexec_add_buffer(&kbuf);
if (r) {
pr_err("Failed to call kexec_add_buffer, ret=%d\n", r);
- kvfree((void *)kbuf.buffer);
- return r;
+ goto out;
}
+
image->dm_crypt_keys_addr = kbuf.mem;
image->dm_crypt_keys_sz = kbuf.bufsz;
kexec_dprintk(
"Loaded dm crypt keys to kexec_buffer bufsz=0x%lx
memsz=0x%lx\n",
kbuf.bufsz, kbuf.memsz);
+out:
+ is_dm_key_reused = false;
return r;
}
+void kexec_file_post_load_cleanup_dm_crypt(struct kimage *image)
+{
+ /*
+ * For CPU/memory hot-plugging, the kdump image will be reloaded.
Prevent
+ * keys_header from being cleaned up during unloading when
+ * is_dm_key_reused=true
+ */
+ if (!is_dm_key_reused) {
+ kfree_sensitive(keys_header);
+ keys_header = NULL;
+ }
+}
+
static int __init configfs_dmcrypt_keys_init(void)
{
int ret;
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 2bfbb2d144e6..0421f1e89791 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -139,6 +139,8 @@ void kimage_file_post_load_cleanup(struct kimage *image)
kfree(image->image_loader_data);
image->image_loader_data = NULL;
+ kexec_file_post_load_cleanup_dm_crypt(image);
+
kexec_file_dbg_print = false;
}
--
2.55.0
