Hi Petr
On 2026/8/26 17:24, Petr Pavlu wrote:
> On 8/24/26 3:47 AM, Hao Ge wrote:
>> If codetag_load_module() fails, err is never set and load_module()
>> returns 0 after the module has been torn down.
>>
>> Fixes: 044d2aee6c57 ("alloc_tag: handle module codetag load errors as module
>> load failures")
>> Reported-by: Sashiko <[email protected]>
>> Cc: [email protected]
>> Signed-off-by: Hao Ge <[email protected]>
>> ---
>> kernel/module/main.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/kernel/module/main.c b/kernel/module/main.c
>> index 46dd8d25a605..afb810f0154c 100644
>> --- a/kernel/module/main.c
>> +++ b/kernel/module/main.c
>> @@ -3568,7 +3568,8 @@ static int load_module(struct load_info *info, const
>> char __user *uargs,
>> goto sysfs_cleanup;
>> }
>>
>> - if (codetag_load_module(mod))
>> + err = codetag_load_module(mod);
>> + if (err)
>> goto sysfs_cleanup;
>>
>> /* Get rid of temporary copy. */
>
> This looks ok to me but there appears another bug related to this code
> that would be good to fix at the same time. If the module is
> a livepatch, the preceding call to copy_module_elf() allocates
> mod->klp_info. However, if codetag_load_module() fails, the code doesn't
> free it.
>
Yes, I missed that. Thanks for catching it!
I've folded your suggested fix into v2 (already sent) and tested it.
Thanks
Best Regards
Hao
> I think we want something like this (not tested):
>
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index d0e1e0bd2ad0..c1b34dc1e89a 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -3581,8 +3581,9 @@ static int load_module(struct load_info *info, const
> char __user *uargs,
> goto sysfs_cleanup;
> }
>
> - if (codetag_load_module(mod))
> - goto sysfs_cleanup;
> + err = codetag_load_module(mod);
> + if (err)
> + goto livepatch_cleanup;
>
> /* Get rid of temporary copy. */
> free_copy(info, flags);
> @@ -3592,6 +3593,9 @@ static int load_module(struct load_info *info, const
> char __user *uargs,
>
> return do_init_module(mod);
>
> + livepatch_cleanup:
> + if (is_livepatch_module(mod))
> + free_module_elf(mod);
> sysfs_cleanup:
> mod_sysfs_teardown(mod);
> coming_cleanup:
>
