Re: [PATCH] crash_dump: Fix potential double free and UAF of keys_header

[Coiby Xu <coiby.xu@gmail.com>]

On Tue, Apr 07, 2026 at 08:44:39AM +0800, 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@vger.kernel.org
>>>Cc: Andrew Morton <akpm@linux-foundation.org>
>>>Reported-by: Sourabh Jain <sourabhjain@linux.ibm.com>
>>>Signed-off-by: Coiby Xu <coxu@redhat.com>
>>>---
>>> 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!
>
>>
>>
[...]
>>
>>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.

After failing to reproduce the predicted issue, I realized there is no
need for looping page by page because kexec_add_buffer will try to find
a continuous physical memory region. So I dropped this idea in v2. But
thanks for helping me learning something new:)

-- 
Best regards,
Coiby