Re: [PATCH] module: pr_debug when there is no version info

[Jinchao Wang]

On 8/26/25 15:20, Petr Pavlu wrote:

On 7/22/25 10:25 AM, Petr Pavlu wrote:

On 7/22/25 5:08 AM, Wang Jinchao wrote:

On 7/21/25 22:40, Petr Pavlu wrote:

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 <wangjinchao...@gmail.com>
---
   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.

When debugging syzkaller, I noticed that the insmod command always reports 
errors. However, to get the exact information, I need to trace the kernel, so I 
casually submitted this patch.

Based on your response, I also feel that the meaning of force_load here is 
somewhat unclear. It would be better to create a mask or a clear list to 
indicate which fields can be forced and which cannot. Once this is clear, we 
can create a function named may_force_check().

I cannot find an explicit reason in the Git history why a missing
vermagic is treated as if the module was loaded with
MODULE_INIT_IGNORE_VERMAGIC, and similarly why missing modversions data
is treated as if the module was loaded with
MODULE_INIT_IGNORE_MODVERSIONS.

I would argue that a more sensible behavior would be to consider
a missing vermagic as an error and allow loading the module only if
MODULE_INIT_IGNORE_VERMAGIC is explicitly specified. And analogously for
missing modversions and MODULE_INIT_IGNORE_MODVERSIONS.

Nonetheless, if I understand correctly, this should be mostly separate
from your issue.

To answer my own confusion, the thing that I missed is that the
MODULE_INIT_IGNORE_* flags are available only for the finit_module
syscall, not for init_module. In the case of init_module, the force
logic is handled by kmod in userspace by stripping the relevant
modversions and vermagic data. This means that when using init_module,
the module loader cannot distinguish between a module that lacks this
data and one that has it deliberately removed. When finit_module was
introduced in 2012, commit 2f3238aebedb ("module: add flags arg to
sys_finit_module()") added the MODULE_INIT_IGNORE_* flags, and they were
simply implemented to mirror the kmod behavior.

-- Petr

The composition of 'force' and 'ignore' is confusing.
I learn much from your feedback, thank you very much.

--
Best regards,
Jinchao