On Wed, Aug 26, 2026 at 2:24 AM Petr Pavlu <[email protected]> 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.
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
