* [PATCH] module: fix lost error code from codetag_load_module()
@ 2026-08-24 1:47 Hao Ge
2026-08-24 16:26 ` Suren Baghdasaryan
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Hao Ge @ 2026-08-24 1:47 UTC (permalink / raw)
To: Suren Baghdasaryan, Andrew Morton, Luis Chamberlain, Petr Pavlu,
Daniel Gomez, Sami Tolvanen, Aaron Tomlin
Cc: linux-modules, linux-kernel, Hao Ge, Sashiko, stable
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
^ permalink raw reply [flat|nested] 8+ 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 16:26 ` Suren Baghdasaryan 2026-08-25 8:54 ` Aaron Tomlin ` (2 subsequent siblings) 3 siblings, 0 replies; 8+ 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] 8+ 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 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 3 siblings, 0 replies; 8+ 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] 8+ 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 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 3 siblings, 0 replies; 8+ 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] 8+ 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 19:36 ` Bradley Morgan @ 2026-08-26 9:24 ` Petr Pavlu 2026-08-27 0:11 ` Suren Baghdasaryan ` (2 more replies) 3 siblings, 3 replies; 8+ 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 [flat|nested] 8+ 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; 8+ 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] 8+ 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; 8+ 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] 8+ 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; 8+ 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] 8+ messages in thread
end of thread, other threads:[~2026-08-27 3:14 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-08-24 1:47 [PATCH] module: fix lost error code from codetag_load_module() Hao Ge 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 2026-08-27 0:11 ` Suren Baghdasaryan 2026-08-27 0:23 ` Bradley Morgan 2026-08-27 3:15 ` Hao Ge
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®