1. Patchew
  2. linux
  3. View series

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

Hao Ge posted 1 patch 1 month ago
There is a newer version of this series
kernel/module/main.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
[PATCH] module: fix lost error code from codetag_load_module()
Posted by Hao Ge 1 month ago 
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
Re: [PATCH] module: fix lost error code from codetag_load_module()
Posted by Petr Pavlu 1 month ago 
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
Re: [PATCH] module: fix lost error code from codetag_load_module()
Posted by Hao Ge 1 month ago 
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:
>
Re: [PATCH] module: fix lost error code from codetag_load_module()
Posted by Bradley Morgan 1 month ago 
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/
Re: [PATCH] module: fix lost error code from codetag_load_module()
Posted by Suren Baghdasaryan 1 month ago 
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
Re: [PATCH] module: fix lost error code from codetag_load_module()
Posted by Bradley Morgan 1 month ago 
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/
Re: [PATCH] module: fix lost error code from codetag_load_module()
Posted by Aaron Tomlin 1 month ago 
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
Re: [PATCH] module: fix lost error code from codetag_load_module()
Posted by Suren Baghdasaryan 1 month ago 
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
>

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.