Re: [PATCH v3 07/17] crash_dump: Fix potential double-free of keys_header

Jinjie Ruan <[email protected]>
Newsgroups gmane.linux.ports.ppc.embedded
Message-ID <[email protected]>

在 2026/8/30 13:59, Sourabh Jain 写道:
> 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?

Absolutely, happy to do so — I'll drop this patch and review Coiby's
series 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.