* Re: [PATCH] module: fix lost error code from codetag_load_module()
2026-08-24 1:47 [PATCH] module: fix lost error code from codetag_load_module() Hao Ge
@ 2026-08-24 2:03 ` sashiko-bot
2026-08-24 16:26 ` Suren Baghdasaryan
` (3 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-08-24 2:03 UTC (permalink / raw)
To: Hao Ge; +Cc: linux-modules
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Missing call to codetag_unload_module() in the error path leaves dangling module pointers in codetag_types IDR.
--
--- Patch [1]: [PATCH] module: fix lost error code from codetag_load_module() ---
commit 5006baafc1e00091b0abb9c8d393876e887ccb31
Author: Hao Ge <hao.ge@linux.dev>
module: fix lost error code from codetag_load_module()
This commit fixes a bug where the error code from codetag_load_module()
was previously lost. It ensures load_module() correctly captures the error
and aborts module loading upon failure.
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -3581,7 +3581,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;
[Severity: High]
This is a pre-existing issue, but by fixing the error path here, we might
expose a use-after-free bug during module load failures.
If codetag_load_module() fails partway through the list of codetag_types:
lib/codetag.c:codetag_load_module() {
...
list_for_each_entry(cttype, &codetag_types, link) {
ret = codetag_module_init(cttype, mod);
if (ret)
break;
}
...
}
It returns an error without rolling back the types that were already
successfully initialized. We then jump to sysfs_cleanup, which eventually
calls module_deallocate() in the free_module block to free the module memory.
However, there is no call to codetag_unload_module() anywhere in this cleanup
path to clean up the dangling references from the codetag_types IDR.
Will this leave dangling pointers that result in a use-after-free dereference
of the freed module memory when codetags are later iterated (for example,
when reading from /proc/allocinfo)?
lib/codetag.c:codetag_next_ct() {
...
ct = get_first_module_ct(cmod);
...
}
>
> /* Get rid of temporary copy. */
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260824014736.35026-1-hao.ge@linux.dev?part=1
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] module: fix lost error code from codetag_load_module()
2026-08-24 1:47 [PATCH] module: fix lost error code from codetag_load_module() Hao Ge
2026-08-24 2:03 ` sashiko-bot
@ 2026-08-24 16:26 ` Suren Baghdasaryan
2026-08-25 8:54 ` Aaron Tomlin
` (2 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Suren Baghdasaryan @ 2026-08-24 16:26 UTC (permalink / raw)
To: Hao Ge
Cc: Andrew Morton, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
Sami Tolvanen, Aaron Tomlin, linux-modules, linux-kernel, Sashiko,
stable
On Sun, Aug 23, 2026 at 6:47 PM Hao Ge <hao.ge@linux.dev> 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 <sashiko-bot@kernel.org>
> Cc: stable@vger.kernel.org
> Signed-off-by: Hao Ge <hao.ge@linux.dev>
Acked-by: Suren Baghdasaryan <surenb@google.com>
> ---
> 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. */
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] module: fix lost error code from codetag_load_module()
2026-08-24 1:47 [PATCH] module: fix lost error code from codetag_load_module() Hao Ge
2026-08-24 2:03 ` sashiko-bot
2026-08-24 16:26 ` Suren Baghdasaryan
@ 2026-08-25 8:54 ` Aaron Tomlin
2026-08-25 19:36 ` Bradley Morgan
2026-08-26 9:24 ` Petr Pavlu
4 siblings, 0 replies; 9+ messages in thread
From: Aaron Tomlin @ 2026-08-25 8:54 UTC (permalink / raw)
To: Hao Ge
Cc: Suren Baghdasaryan, Andrew Morton, Luis Chamberlain, Petr Pavlu,
Daniel Gomez, Sami Tolvanen, linux-modules, linux-kernel, Sashiko,
stable
On Mon, Aug 24, 2026 at 09:47:36AM +0800, 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 <sashiko-bot@kernel.org>
> Cc: stable@vger.kernel.org
> Signed-off-by: Hao Ge <hao.ge@linux.dev>
> ---
> 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. */
> --
> 2.25.1
>
Reviewed-by: Aaron Tomlin <atomlin@atomlin.com>
--
Aaron Tomlin
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] module: fix lost error code from codetag_load_module()
2026-08-24 1:47 [PATCH] module: fix lost error code from codetag_load_module() Hao Ge
` (2 preceding siblings ...)
2026-08-25 8:54 ` Aaron Tomlin
@ 2026-08-25 19:36 ` Bradley Morgan
2026-08-26 9:24 ` Petr Pavlu
4 siblings, 0 replies; 9+ messages in thread
From: Bradley Morgan @ 2026-08-25 19:36 UTC (permalink / raw)
To: hao.ge
Cc: akpm, atomlin, da.gomez, linux-kernel, linux-modules, mcgrof,
petr.pavlu, samitolvanen, sashiko-bot, stable, surenb
On 24 August 2026 02:47:36 BST, Hao Ge <hao.ge@linux.dev> wrote:
>If codetag_load_module() fails, err is never set and load_module()
>returns 0 after the module has been torn down.
>
I see!
>Fixes: 044d2aee6c57 ("alloc_tag: handle module codetag load errors as module load failures")
>Reported-by: Sashiko <sashiko-bot@kernel.org>
>Cc: stable@vger.kernel.org
Reviewed-by: Bradley Morgan <brads@mainlining.org>
>Signed-off-by: Hao Ge <hao.ge@linux.dev>
>---
> 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)
Nice!
> goto sysfs_cleanup;
>
> /* Get rid of temporary copy. */
>
--- Thanks!
https://lore.kernel.org/all/EE579805-42F2-4C58-B752-F28779EEB717@grrlz.net/
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] module: fix lost error code from codetag_load_module()
2026-08-24 1:47 [PATCH] module: fix lost error code from codetag_load_module() Hao Ge
` (3 preceding siblings ...)
2026-08-25 19:36 ` Bradley Morgan
@ 2026-08-26 9:24 ` Petr Pavlu
2026-08-27 0:11 ` Suren Baghdasaryan
` (2 more replies)
4 siblings, 3 replies; 9+ messages in thread
From: Petr Pavlu @ 2026-08-26 9:24 UTC (permalink / raw)
To: Hao Ge
Cc: Suren Baghdasaryan, Andrew Morton, Luis Chamberlain, Daniel Gomez,
Sami Tolvanen, Aaron Tomlin, linux-modules, linux-kernel, Sashiko,
stable
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 <sashiko-bot@kernel.org>
> Cc: stable@vger.kernel.org
> Signed-off-by: Hao Ge <hao.ge@linux.dev>
> ---
> 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.
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:
--
Thanks,
Petr
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH] module: fix lost error code from codetag_load_module()
2026-08-26 9:24 ` Petr Pavlu
@ 2026-08-27 0:11 ` Suren Baghdasaryan
2026-08-27 0:23 ` Bradley Morgan
2026-08-27 3:15 ` Hao Ge
2 siblings, 0 replies; 9+ messages in thread
From: Suren Baghdasaryan @ 2026-08-27 0:11 UTC (permalink / raw)
To: Petr Pavlu
Cc: Hao Ge, Andrew Morton, Luis Chamberlain, Daniel Gomez,
Sami Tolvanen, Aaron Tomlin, linux-modules, linux-kernel, Sashiko,
stable
On Wed, Aug 26, 2026 at 2:24 AM Petr Pavlu <petr.pavlu@suse.com> 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 <sashiko-bot@kernel.org>
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Hao Ge <hao.ge@linux.dev>
> > ---
> > 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.
Ah, I see. Nice catch!
>
> I think we want something like this (not tested):
Yep, that looks like the right fix to me. Thanks!
>
> 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:
>
> --
> Thanks,
> Petr
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] module: fix lost error code from codetag_load_module()
2026-08-26 9:24 ` Petr Pavlu
2026-08-27 0:11 ` Suren Baghdasaryan
@ 2026-08-27 0:23 ` Bradley Morgan
2026-08-27 3:15 ` Hao Ge
2 siblings, 0 replies; 9+ messages in thread
From: Bradley Morgan @ 2026-08-27 0:23 UTC (permalink / raw)
To: petr.pavlu
Cc: akpm, atomlin, da.gomez, hao.ge, linux-kernel, linux-modules,
mcgrof, samitolvanen, sashiko-bot, stable, surenb
On 26 August 2026 10:24:03 BST, Petr Pavlu <petr.pavlu@suse.com> 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 <sashiko-bot@kernel.org>
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Hao Ge <hao.ge@linux.dev>
>> ---
>> 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.
>
>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:
>
>
If patch is submitted!
Reviewed-by: Bradley Morgan <brads@mainlining.org>
--- Thanks!
https://lore.kernel.org/all/EE579805-42F2-4C58-B752-F28779EEB717@grrlz.net/
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] module: fix lost error code from codetag_load_module()
2026-08-26 9:24 ` Petr Pavlu
2026-08-27 0:11 ` Suren Baghdasaryan
2026-08-27 0:23 ` Bradley Morgan
@ 2026-08-27 3:15 ` Hao Ge
2 siblings, 0 replies; 9+ messages in thread
From: Hao Ge @ 2026-08-27 3:15 UTC (permalink / raw)
To: Petr Pavlu
Cc: Suren Baghdasaryan, Andrew Morton, Luis Chamberlain, Daniel Gomez,
Sami Tolvanen, Aaron Tomlin, linux-modules, linux-kernel, Sashiko,
stable
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 <sashiko-bot@kernel.org>
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Hao Ge <hao.ge@linux.dev>
>> ---
>> 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:
>
^ permalink raw reply [flat|nested] 9+ messages in thread