[PATCH v2 0/4] module: logging and code improvements

[PATCH v2 0/4] module: logging and code improvements

[Jinchao Wang]

This series of patches cleans up and refactors the kernel's module loading
code. The goal is to make the subsystem's logging clearer and its internal
logic more straightforward for developers to understand.

The patches in this series: 

- module: signing: Use pr_err for signature rejection
  Makes module signature rejection messages more visible.
- module: show why force load fails 
  Adds a reason to the error message when force loading is disabled.
- module: centralize no-versions force load check
  Refactors the code to centralize the "no versions" force load check.
- module: separate vermagic and livepatch checks
  Improves code organization by separating vermagic and livepatch checks.

Changes from v1:
A patch was dropped because it was based on a misunderstanding
of the ignore versioning flag's original intent.

v1 link:
https://lore.kernel.org/all/20250822125454.1287066-1-wangjinchao600@gmail.com/T/#mf748b6e97934f7a463dfdafbb426965f3e0ad646


Jinchao Wang (4):
  module: signing: Use pr_err for signature rejection
  module: show why force load fails
  module: centralize no-versions force load check
  module: separate vermagic and livepatch checks

 kernel/module/main.c    | 13 +++++++------
 kernel/module/signing.c |  2 +-
 kernel/module/version.c |  9 +++++----
 3 files changed, 13 insertions(+), 11 deletions(-)

-- 
2.43.0

[PATCH v2 1/4] module: signing: Use pr_err for signature rejection

[Jinchao Wang]

Make module signature rejection messages more visible by using pr_err
instead of pr_notice.

Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
 kernel/module/signing.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/module/signing.c b/kernel/module/signing.c
index a2ff4242e623..557cb795fa31 100644
--- a/kernel/module/signing.c
+++ b/kernel/module/signing.c
@@ -117,7 +117,7 @@ int module_sig_check(struct load_info *info, int flags)
 	}
 
 	if (is_module_sig_enforced()) {
-		pr_notice("Loading of %s is rejected\n", reason);
+		pr_err("Loading of %s is rejected\n", reason);
 		return -EKEYREJECTED;
 	}
 
-- 
2.43.0

[PATCH v2 2/4] module: show why force load fails

[Jinchao Wang]

Include reason in error message when force loading is disabled.

Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
 kernel/module/main.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/module/main.c b/kernel/module/main.c
index c66b26184936..a426bd8a18b5 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -1083,6 +1083,7 @@ int try_to_force_load(struct module *mod, const char *reason)
 	add_taint_module(mod, TAINT_FORCED_MODULE, LOCKDEP_NOW_UNRELIABLE);
 	return 0;
 #else
+	pr_err("%s force load is not supported\n", reason);
 	return -ENOEXEC;
 #endif
 }
-- 
2.43.0

[PATCH v2 3/4] module: centralize no-versions force load check

[Jinchao Wang]

Move try_to_force_load() call from check_version() to
check_modstruct_version() to handle "no versions" case only once before
other version checks.

This prevents duplicate force load attempts and makes the error message
show the proper reason.

Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
 kernel/module/version.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/kernel/module/version.c b/kernel/module/version.c
index 2beefeba82d9..3f07fd03cb30 100644
--- a/kernel/module/version.c
+++ b/kernel/module/version.c
@@ -41,10 +41,6 @@ int check_version(const struct load_info *info,
 		return 1;
 	}
 
-	/* No versions at all?  modprobe --force does this. */
-	if (versindex == 0)
-		return try_to_force_load(mod, symname) == 0;
-
 	versions = (void *)sechdrs[versindex].sh_addr;
 	num_versions = sechdrs[versindex].sh_size
 		/ sizeof(struct modversion_info);
@@ -81,6 +77,11 @@ int check_modstruct_version(const struct load_info *info,
 	};
 	bool have_symbol;
 
+	/* No versions at all?  modprobe --force does this. */
+	if (info->index.vers == 0 &&
+	    try_to_force_load(mod, "no versions module"))
+		return 1;
+
 	/*
 	 * Since this should be found in kernel (which can't be removed), no
 	 * locking is necessary. Regardless use a RCU read section to keep
-- 
2.43.0

[PATCH v2 4/4] module: separate vermagic and livepatch checks

[Jinchao Wang]

Rename check_modinfo() to check_modinfo_vermagic() to clarify it only
checks vermagic compatibility.

Move livepatch check to happen after vermagic check in early_mod_check(),
creating proper separation of concerns.

This makes the module loading sequence more logical:
- First verify module vermagic
- Then check livepatch-specific requirements

No functional changes, just clearer code organization.

Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
 kernel/module/main.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/kernel/module/main.c b/kernel/module/main.c
index a426bd8a18b5..d30bffeef63e 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -2571,7 +2571,7 @@ static void module_augment_kernel_taints(struct module *mod, struct load_info *i
 
 }
 
-static int check_modinfo(struct module *mod, struct load_info *info, int flags)
+static int check_modinfo_vermagic(struct module *mod, struct load_info *info, int flags)
 {
 	const char *modmagic = get_modinfo(info, "vermagic");
 	int err;
@@ -2590,10 +2590,6 @@ static int check_modinfo(struct module *mod, struct load_info *info, int flags)
 		return -ENOEXEC;
 	}
 
-	err = check_modinfo_livepatch(mod, info);
-	if (err)
-		return err;
-
 	return 0;
 }
 
@@ -3334,7 +3330,11 @@ static int early_mod_check(struct load_info *info, int flags)
 	if (!check_modstruct_version(info, info->mod))
 		return -ENOEXEC;
 
-	err = check_modinfo(info->mod, info, flags);
+	err = check_modinfo_vermagic(info->mod, info, flags);
+	if (err)
+		return err;
+
+	err = check_modinfo_livepatch(info->mod, info);
 	if (err)
 		return err;
 
-- 
2.43.0

Re: [PATCH v2 2/4] module: show why force load fails

[Petr Pavlu]

On 8/25/25 11:15 AM, Jinchao Wang wrote:
> Include reason in error message when force loading is disabled.
> 
> Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
> ---
>  kernel/module/main.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index c66b26184936..a426bd8a18b5 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -1083,6 +1083,7 @@ int try_to_force_load(struct module *mod, const char *reason)
>  	add_taint_module(mod, TAINT_FORCED_MODULE, LOCKDEP_NOW_UNRELIABLE);
>  	return 0;
>  #else
> +	pr_err("%s force load is not supported\n", reason);
>  	return -ENOEXEC;
>  #endif
>  }

The module name is already available at all points where
try_to_force_load() is called, so the new error message should include
it.

Additionally, we should be careful about the message. In the case of the
init_module syscall, the missing modversions and vermagic could mean
that the data was deliberately stripped by kmod because the module was
inserted with --force, or it could mean that the module lacks this data
in the first place. In other words, it is not always the case that that
we're reaching this logic because of a force load.

My suggestion would be to use the following:

pr_err("%s: %s, force load is not supported\n", mod->name, reason);

-- 
Thanks,
Petr

Re: [PATCH v2 3/4] module: centralize no-versions force load check

[Petr Pavlu]

On 8/25/25 11:15 AM, Jinchao Wang wrote:
> Move try_to_force_load() call from check_version() to
> check_modstruct_version() to handle "no versions" case only once before
> other version checks.
> 
> This prevents duplicate force load attempts and makes the error message
> show the proper reason.
> 
> Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
> ---
>  kernel/module/version.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/module/version.c b/kernel/module/version.c
> index 2beefeba82d9..3f07fd03cb30 100644
> --- a/kernel/module/version.c
> +++ b/kernel/module/version.c
> @@ -41,10 +41,6 @@ int check_version(const struct load_info *info,
>  		return 1;
>  	}
>  
> -	/* No versions at all?  modprobe --force does this. */
> -	if (versindex == 0)
> -		return try_to_force_load(mod, symname) == 0;
> -
>  	versions = (void *)sechdrs[versindex].sh_addr;
>  	num_versions = sechdrs[versindex].sh_size
>  		/ sizeof(struct modversion_info);

Removing this return completely means that when versindex == 0, the
function incorrectly looks at the dummy section #0 and eventually
calls pr_warn_once("%s: no symbol version for %s\n", ...).

As I outlined in my prototype patch previously [1], I think this should
be changed to:

	/* No versions? Ok, already tainted in check_modstruct_version(). */
	if (versindex == 0)
		return 1;

> @@ -81,6 +77,11 @@ int check_modstruct_version(const struct load_info *info,
>  	};
>  	bool have_symbol;
>  
> +	/* No versions at all?  modprobe --force does this. */
> +	if (info->index.vers == 0 &&
> +	    try_to_force_load(mod, "no versions module"))
> +		return 1;
> +
>  	/*
>  	 * Since this should be found in kernel (which can't be removed), no
>  	 * locking is necessary. Regardless use a RCU read section to keep

I suggest that the reason message should say "no versions for imported
symbols" to indicate which version data is missing and to be consistent
with check_export_symbol_versions(), which uses "no versions for
exported symbols".

[1] https://lore.kernel.org/linux-modules/3992b57d-3d8b-4d60-bc4a-f227f712dcca@suse.com/

-- 
Thanks,
Petr

Re: [PATCH v2 2/4] module: show why force load fails

[Jinchao Wang]

On 8/26/25 17:33, Petr Pavlu wrote:
> On 8/25/25 11:15 AM, Jinchao Wang wrote:
>> Include reason in error message when force loading is disabled.
>>
>> Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
>> ---
>>   kernel/module/main.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/kernel/module/main.c b/kernel/module/main.c
>> index c66b26184936..a426bd8a18b5 100644
>> --- a/kernel/module/main.c
>> +++ b/kernel/module/main.c
>> @@ -1083,6 +1083,7 @@ int try_to_force_load(struct module *mod, const char *reason)
>>   	add_taint_module(mod, TAINT_FORCED_MODULE, LOCKDEP_NOW_UNRELIABLE);
>>   	return 0;
>>   #else
>> +	pr_err("%s force load is not supported\n", reason);
>>   	return -ENOEXEC;
>>   #endif
>>   }
> 
> The module name is already available at all points where
> try_to_force_load() is called, so the new error message should include
> it.
> 
> Additionally, we should be careful about the message. In the case of the
> init_module syscall, the missing modversions and vermagic could mean
> that the data was deliberately stripped by kmod because the module was
> inserted with --force, or it could mean that the module lacks this data
> in the first place. In other words, it is not always the case that that
> we're reaching this logic because of a force load.
> 
> My suggestion would be to use the following:
> 
> pr_err("%s: %s, force load is not supported\n", mod->name, reason);
> 
Good suggestion. Thanks.

-- 
Best regards,
Jinchao