Re: [PATCH] crash_dump: Fix potential double free and UAF of keys_header
From: Sourabh Jain
Date: Tue Apr 07 2026 - 05:59:57 EST
On 07/04/26 06:14, Coiby Xu wrote:
On Fri, Apr 03, 2026 at 07:48:29PM +0530, Sourabh Jain wrote:
Hello Coiby,
Hi Sourabh,
On 03/04/26 15:31, Coiby Xu wrote:
If kexec_add_buffer fails, keys_header will be freed. And 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
Address these problems by setting keys_header to NULL after freeing
kbuf.buffer and re-building keys_header when necessary respectively.
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")
Cc: stable@xxxxxxxxxxxxxxx
Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
Reported-by: Sourabh Jain <sourabhjain@xxxxxxxxxxxxx>
Signed-off-by: Coiby Xu <coxu@xxxxxxxxxx>
---
kernel/crash_dump_dm_crypt.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c
index a20d4097744a..92eebef27156 100644
--- a/kernel/crash_dump_dm_crypt.c
+++ b/kernel/crash_dump_dm_crypt.c
@@ -417,7 +417,7 @@ int crash_load_dm_crypt_keys(struct kimage *image)
return -ENOENT;
}
- if (!is_dm_key_reused) {
+ if (!is_dm_key_reused || !keys_header) {
image->dm_crypt_keys_addr = 0;
r = build_keys_header();
if (r)
@@ -433,6 +433,7 @@ int crash_load_dm_crypt_keys(struct kimage *image)
r = kexec_add_buffer(&kbuf);
if (r) {
kvfree((void *)kbuf.buffer);
+ keys_header = NULL;
return r;
}
image->dm_crypt_keys_addr = kbuf.mem;
base-commit: d8a9a4b11a137909e306e50346148fc5c3b63f9d
Sashiko raised seven concerns on this patch. Most of them are
not directly related to the changes introduced here, but I
think they can be addressed along with this fix.
https://sashiko.dev/#/patchset/20260403100126.1468200-1-coxu%40redhat.com
Thanks for pointing me to the Sashiko's code review and also sharing
your meticulous analysis!
1. build_keys_header() does not release key_header memory on
error. This can cause incorrect keys to be loaded for the
kdump kernel in subsequent system calls.
Can be addressed by releasing keys_header on error path.
I'll address this issue! Thanks for the suggestion!
2–3. get_keys_header_size() uses key_count to find the size of
key_header buffer, which can lead to out-of-bounds access
at two places.
a. Around kexec_add_buffer()
b. In build_keys_header()
I think there is one more place where this applies is:
c. In get_keys_from_kdump_reserved_memory() at memcpy
I agree with solution provided by Sashiko of using keys_header->total_keys
instead.
Thanks for showing me where out-of-bounds accesses can happen! I'll do
some testing to see if using keys_header->total_keys is sufficient.
4. get_keys_from_kdump_reserved_memory() may run into issues
if kexec_crash_image->dm_crypt_keys_addr is larger than a
page size during memcpy. Because kmap_local_page only maps
one page.
How about moving this in a loop and do map and copy page by page?
Yeah, looping over the pages should be a robust solution.
5. Related to releasing the keyring_ref reference count, but
I did not fully understand this concern.
My latest test already covers the case where there are two keys to
iterate over. I'll dig more into keyring_ref to see if Sashiko's
concerns is valid.
6. restore_dm_crypt_keys_to_thread_keyring() does not release
previously allocated keys_header, leading to a memory leak.
Thanks for raising the concern! Although we can assume the system will
reboot soon after vmcore dumping is finished, it's better to free
keys_header.
As per kdump.rst, restore was introduced to handle CPU and
memory hotplug cases. Is it needed when there is no in-kernel
update to the kdump image on CPU or memory hotplug events?
But in that case, we rely on a udev rule to reload the kdump image
again.
I am confused about when exactly we need to restore.
To clarify, reuse other than restore is needed for non in-kernel update
when handing CPU/memory hotplugging. Yes, a udev rule is also needed in
this case.
Below commit explains how the reuse is utilized:
commit 9ebfa8dcaea77a8ef02d0f9478717a138b0ad828
Author: Coiby Xu <coxu@xxxxxxxxxx>
Date: Fri May 2 09:12:38 2025 +0800
crash_dump: reuse saved dm crypt keys for CPU/memory hot-plugging
It got it now. This is helpful when kdump needs to be reloaded due to
CPU/memory hotplug events using the kexec_file_load system call,
but only when CONFIG_CRASH_HOTPLUG is not enabled.
IIUC this feature is not support on crash image loaded using kexec_load syscall, right?
- Sourabh Jain