[PATCH v2] module: fix lost error code from codetag_load_module()

[PATCH v2] module: fix lost error code from codetag_load_module()

[Hao Ge]

If codetag_load_module() fails, err is not set to reflect the failure
and load_module() returns 0 after the module has been torn down.

Also, if the module is a livepatch, mod->klp_info allocated by
copy_module_elf() leaks on this error path. Free it via a new
livepatch_cleanup label.

Fixes: 044d2aee6c57 ("alloc_tag: handle module codetag load errors as module load failures")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Suggested-by: Petr Pavlu <petr.pavlu@suse.com>
Reviewed-by: Bradley Morgan <brads@mainlining.org>
Cc: stable@vger.kernel.org
Signed-off-by: Hao Ge <hao.ge@linux.dev>
---
v2: also free mod->klp_info of livepatch modules on this
    error path (Petr Pavlu).

v1 link: https://lore.kernel.org/all/20260824014736.35026-1-hao.ge@linux.dev/
---
 kernel/module/main.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/kernel/module/main.c b/kernel/module/main.c
index 46dd8d25a605..c32f1d370b73 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -3568,8 +3568,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);
@@ -3579,6 +3580,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:
-- 
2.25.1

Re: [PATCH v2] module: fix lost error code from codetag_load_module()

[Daniel Gomez]

On 2026-08-27T11:05:03+08:00, Hao Ge <hao.ge@linux.dev> wrote:
> If codetag_load_module() fails, err is not set to reflect the failure
> and load_module() returns 0 after the module has been torn down.
>
> Also, if the module is a livepatch, mod->klp_info allocated by
> copy_module_elf() leaks on this error path. Free it via a new
> livepatch_cleanup label.
>
> Fixes: 044d2aee6c57 ("alloc_tag: handle module codetag load errors as module load failures")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Suggested-by: Petr Pavlu <petr.pavlu@suse.com>
> Reviewed-by: Bradley Morgan <brads@mainlining.org>
> Cc: stable@vger.kernel.org
> Signed-off-by: Hao Ge <hao.ge@linux.dev>

We need the unwind from codetag_unload_module() as reported by sashiko.

With that,

Reviewed-by: Daniel Gomez <da.gomez@samsung.com>

Re: [PATCH v2] module: fix lost error code from codetag_load_module()

[Hao Ge]

Hi Daniel


On 2026/9/8 03:42, Daniel Gomez wrote:
> 
> 
> On 2026-08-27T11:05:03+08:00, Hao Ge <hao.ge@linux.dev> wrote:
>> If codetag_load_module() fails, err is not set to reflect the failure
>> and load_module() returns 0 after the module has been torn down.
>>
>> Also, if the module is a livepatch, mod->klp_info allocated by
>> copy_module_elf() leaks on this error path. Free it via a new
>> livepatch_cleanup label.
>>
>> Fixes: 044d2aee6c57 ("alloc_tag: handle module codetag load errors as module load failures")
>> Reported-by: Sashiko <sashiko-bot@kernel.org>
>> Suggested-by: Petr Pavlu <petr.pavlu@suse.com>
>> Reviewed-by: Bradley Morgan <brads@mainlining.org>
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Hao Ge <hao.ge@linux.dev>
> 
> We need the unwind from codetag_unload_module() as reported by sashiko.
> 

Thanks for you review.

I skipped it because alloc_tag is the only codetag type and it
already cleans up after itself.

With a second codetag type the problem Sashiko reported could indeed
be triggered.

My thinking at the time was to do it in codetag itself, as a separate patch,
since it feels odd to unload something whose load just failed.

Happy to hear your thoughts on this.

Thanks
Best Regards
Hao
     
> With that,
> 
> Reviewed-by: Daniel Gomez <da.gomez@samsung.com>
> 
> 

Re: [PATCH v2] module: fix lost error code from codetag_load_module()

[Daniel Gomez]

On 2026-09-08T17:50:33+08:00, Hao Ge <hao.ge@linux.dev> wrote:
> On 2026/9/8 03:42, Daniel Gomez wrote:

> I skipped it because alloc_tag is the only codetag type and it
> already cleans up after itself.
>
> With a second codetag type the problem Sashiko reported could indeed
> be triggered.
>
> My thinking at the time was to do it in codetag itself, as a separate patch,
> since it feels odd to unload something whose load just failed.

Then, why returning an error at all? I don't see why not doing it inside
codetag itself wouldn't work but it's a "bigger" change. IMO, the error
is supposed to be handled at load_module() then, the unwinding as the
rest.

Re: [PATCH v2] module: fix lost error code from codetag_load_module()

[Hao Ge]

Hi Daniel


On 2026/9/10 06:00, Daniel Gomez wrote:
> On 2026-09-08T17:50:33+08:00, Hao Ge <hao.ge@linux.dev> wrote:
>> On 2026/9/8 03:42, Daniel Gomez wrote:
> 
>> I skipped it because alloc_tag is the only codetag type and it
>> already cleans up after itself.
>>
>> With a second codetag type the problem Sashiko reported could indeed
>> be triggered.
>>
>> My thinking at the time was to do it in codetag itself, as a separate patch,
>> since it feels odd to unload something whose load just failed.
> 
> Then, why returning an error at all? I don't see why not doing it inside

Codetag is a generic framework with a dedicated section in modules. We keep
this error path to account for possible future data stored in this section,
allowing us to perform security and validity checks for codetag within
codetag_load_module.

> codetag itself wouldn't work but it's a "bigger" change. IMO, the error
> is supposed to be handled at load_module() then, the unwinding as the
> rest.
> 
> 
I'm worried about this as well. Invoking codetag_unload_module () here brings
kvfree_rcu_barrier () into the error path and triggers two runs of ->free_section_mem (),
(used=true from the unload, used=false later from module_deallocate).

Perhaps we could put this patch on hold for now, until I come up with a proper solution?
The intention is to let codetag take care of its own cleanup, preserving the logic in our
existing patch.

How does this sound?

Thanks
Best Regards
Hao

Re: [PATCH v2] module: fix lost error code from codetag_load_module()

[Hao Ge]

Hi Daniel

Thanks for you comment.

On 2026/9/10 06:00, Daniel Gomez wrote:
> On 2026-09-08T17:50:33+08:00, Hao Ge <hao.ge@linux.dev> wrote:
>> On 2026/9/8 03:42, Daniel Gomez wrote:
> 
>> I skipped it because alloc_tag is the only codetag type and it
>> already cleans up after itself.
>>
>> With a second codetag type the problem Sashiko reported could indeed
>> be triggered.
>>
>> My thinking at the time was to do it in codetag itself, as a separate patch,
>> since it feels odd to unload something whose load just failed.
> 
> Then, why returning an error at all? I don't see why not doing it inside

codetag is a generic framework with a dedicated section in modules.
We keep this error path to account for possible future data stored
in this section, allowing us to perform security and validity
checks for codetag within codetag_load_module.

> codetag itself wouldn't work but it's a "bigger" change. IMO, the error
> is supposed to be handled at load_module() then, the unwinding as the
> rest.
> 
> 

I'm worried about this as well. Invoking codetag_unload_module() here
brings kvfree_rcu_barrier() into the error path and triggers two runs of
->free_section_mem() (used=true from the unload, used=false later from
module_deallocate).

My idea is to let codetag handle its own cleanup while keeping the logic of
our current patch. Maybe we can put this patch on hold temporarily until I
work out a proper solution. How does this sound?

Thanks
Best Regards
Hao