On 7/21/25 6:52 AM, Wang Jinchao wrote:
> When there is no version information, modprobe and insmod only
> report "invalid format".
> Print the actual cause to make it easier to diagnose the issue.
> This helps developers quickly identify version-related module
> loading failures.
> Signed-off-by: Wang Jinchao <wangjinchao600@xxxxxxxxx>
> ---
> kernel/module/version.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/module/version.c b/kernel/module/version.c
> index 2beefeba82d9..bc28c697ff3a 100644
> --- a/kernel/module/version.c
> +++ b/kernel/module/version.c
> @@ -42,8 +42,10 @@ int check_version(const struct load_info *info,
> }
>
> /* No versions at all? modprobe --force does this. */
> - if (versindex == 0)
> + if (versindex == 0) {
> + pr_debug("No version info for module %s\n", info->name);
> return try_to_force_load(mod, symname) == 0;
> + }
>
> versions = (void *)sechdrs[versindex].sh_addr;
> num_versions = sechdrs[versindex].sh_size
I think it would be better to instead improve the behavior of
try_to_force_load(). The function should print the error reason prior to
returning -ENOEXEC. This would also help its two other callers,
check_modinfo() and check_export_symbol_versions().
Additionally, I suggest moving the check to ensure version information
is available for imported symbols earlier in the loading process.
A suitable place might be check_modstruct_version(). This way the check
is performed only once per module.
The following is a prototype patch:
diff --git a/kernel/module/main.c b/kernel/module/main.c
index c2c08007029d..c1ccd343e8c3 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -1053,6 +1053,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: %s\n", mod->name, reason);
return -ENOEXEC;
#endif
}
diff --git a/kernel/module/version.c b/kernel/module/version.c
index 2beefeba82d9..4d9ebf0834de 100644
--- a/kernel/module/version.c
+++ b/kernel/module/version.c
@@ -41,9 +41,9 @@ int check_version(const struct load_info *info,
return 1;
}
- /* No versions at all? modprobe --force does this. */
+ /* No versions? Ok, already tainted in check_modstruct_version(). */
if (versindex == 0)
- return try_to_force_load(mod, symname) == 0;
+ return 1;
versions = (void *)sechdrs[versindex].sh_addr;
num_versions = sechdrs[versindex].sh_size
@@ -90,6 +90,11 @@ int check_modstruct_version(const struct load_info *info,
have_symbol = find_symbol(&fsa);
BUG_ON(!have_symbol);
+ /* No versions at all? modprobe --force does this. */
+ if (!info->index.vers && !info->index.vers_ext_crc)
+ return try_to_force_load(
+ mod, "no versions for imported symbols") == 0;
+
return check_version(info, "module_layout", mod, fsa.crc);
}
As a side note, I'm confused why with CONFIG_MODULE_FORCE_LOAD=y, the
code treats missing modversions for imported symbols as ok, even without
MODULE_INIT_IGNORE_MODVERSIONS. This is at least consistent with the
handling of missing vermagic, but it seems this behavior should be
stricter.
--
Thanks,
Petr
