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

[Coiby Xu]

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@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!
>
> >
[...]
> > 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