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;
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.