Re: [PATCH v3 07/17] crash_dump: Fix potential double-free of keys_header
Sourabh Jain <[email protected]>
| Newsgroups | gmane.linux.ports.ppc.embedded |
|---|---|
| Message-ID | <87186b8a-f68c-400b-97cf-8ea6129eba69__7801.51857049845$1788069694$gmane$org@linux.ibm.com> |
Hello Jinjie, Coiby is handling this issue in the below patch series: https://lore.kernel.org/all/[email protected]/ Since the above patch series is all about crash_load_dm_crypt_keys, could you please consider dropping this patch from your series and reviewing his patch instead? Thanks, Sourabh Jain On 26/08/26 14:55, Jinjie Ruan wrote: > `keys_header` was freed in `build_keys_header()` without being reset > to NULL, and the error path in `crash_load_dm_crypt_keys()` freed it > unconditionally even when reused, leading to double-free or > use-after-free. > > Add `free_keys_header()` to centralize freeing and NULL-setting. > Use it in `build_keys_header()` and only free in the error path when > the header was newly built (`!is_dm_key_reused`). > > Cc: Andrew Morton <[email protected]> > Cc: Baoquan He <[email protected]> > Cc: Mike Rapoport <[email protected]> > Cc: Pasha Tatashin <[email protected]> > Cc: Pratyush Yadav <[email protected]> > Cc: Dave Young <[email protected]> > Cc: [email protected] > Fixes: e3a84be1ec2f ("arm64,ppc64le/kdump: pass dm-crypt keys to kdump kernel") > Signed-off-by: Jinjie Ruan <[email protected]> > --- > kernel/crash_dump_dm_crypt.c | 15 +++++++++++---- > 1 file changed, 11 insertions(+), 4 deletions(-) > > diff --git a/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c > index c685497cd470..ed0960ff0987 100644 > --- a/kernel/crash_dump_dm_crypt.c > +++ b/kernel/crash_dump_dm_crypt.c > @@ -363,15 +363,21 @@ static struct configfs_subsystem config_keys_subsys = { > }, > }; > > +static void free_keys_header(void) > +{ > + if (keys_header) { > + kvfree(keys_header); > + keys_header = NULL; > + } > +} > + > 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); > - > + free_keys_header(); > keys_header = kzalloc(get_keys_header_size(key_count), GFP_KERNEL); > if (!keys_header) > return -ENOMEM; > @@ -441,7 +447,8 @@ 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); > + if (!is_dm_key_reused) > + free_keys_header(); > return r; > } > image->dm_crypt_keys_addr = kbuf.mem;