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

[Petr Pavlu <petr.pavlu@suse.com>]

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