On Thu, Oct 09, 2025 at 03:47:42PM +0200, Petr Pavlu wrote:
On 10/8/25 8:41 AM, Lucas De Marchi wrote:
On Tue, Aug 19, 2025 at 09:17:50AM -0500, Lucas De Marchi wrote:
On Tue, Aug 19, 2025 at 10:52:16AM +0200, Petr Pavlu wrote:
On 8/18/25 11:34 AM, Phil Sutter wrote:
On Sun, Aug 17, 2025 at 05:54:27PM +0200, Christophe Leroy wrote:
Le 17/08/2025 à 01:33, Phil Sutter a écrit :
[Vous ne recevez pas souvent de courriers de [email protected]. D?couvrez pourquoi
ceci est important ? https://aka.ms/LearnAboutSenderIdentification ]
Hi,
I admittedly didn't fully analyze the cause, but on my system a call to:
# insmod /lib/module/$(uname -r)/kernel/net/netfilter/nf_conntrack_ftp.ko
fails with -EEXIST (due to a previous call to 'nfct add helper ftp inet
tcp'). A call to:
# modprobe nf_conntrack_ftp
though returns 0 even though module loading fails. Is there a bug in
modprobe error status handling?
Read the man page : https://linux.die.net/man/8/modprobe
In the man page I see:
Normally, modprobe will succeed (and do nothing) if told to
insert a module which is already present or to remove a module which
isn't present.
This is not a case of already inserted module, it is not loaded before
the call to modprobe. It is the module_init callback
nf_conntrack_ftp_init() which returns -EEXIST it received from
nf_conntrack_helpers_register().
is this a real failure condition or something benign like "if it's
already registered, there's nothing to do"?
Can't user space distinguish the two causes of -EEXIST? Or in other
words, is use of -EEXIST in module_init callbacks problematic?
Unfortunately, error return codes from (f)init_module cannot be reliably
depended upon. For instance, cpufreq drivers have similar behavior of
returning -EEXIST when another cpufreq driver is already registered.
Returning this code unexpectedly can then confuse kmod, as it interprets
-EEXIST to mean "the module is already loaded" [1].
well, it's not that it can't be relied on. There's 1 exit code that is
treated specially, EEXISTS, because that error is used by the module
loading part, before the module_init call, to signify the module is
already loaded.
I have thought about this problem before. We might fix the main
problematic occurrences, but we can't really audit all the code that
module init functions can invoke. I then wonder if it would make sense
for the module loader to warn about any -EEXIST returned by a module's
init function and translate it to -EBUSY.
If it's a failure condition then yes, -EBUSY looks appropriate.
something like this:
diff --git a/kernel/module/main.c b/kernel/module/main.c
index c66b261849362..e5fb1a4ef3441 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -3038,6 +3038,11 @@ static noinline int do_init_module(struct module *mod)
if (mod->init != NULL)
ret = do_one_initcall(mod->init);
if (ret < 0) {
+ if (ret == -EEXIST) {
+ pr_warn("%s: '%s'->init suspiciously returned %d: Overriding with
%d\n",
+ __func__, mod->name, -EEXIST, -EBUSY);
+ ret = -EBUSY;
+ }
goto fail_free_freeinit;
}
if (ret > 0) {
Yes, that's what I had in mind. Could you please send this as a proper
patch to the list?
I only think we should include a hint to explain why this is a problem
and simplify the message somewhat, something like:
pr_warn("%s: init suspiciously returned -EEXIST (reserved for loaded modules),
overriding with -EBUSY\n", mod->name);
I realize you based the message on the later warning about the init
function returning a >0 value but I think we should rather update that
message as well. It should follow the usual style of
"<mod-name>: <error-description>". I suggest simplifying it to:
pr_warn("%s: init suspiciously returned %d, it should follow 0/-E convention\n",
mod->name, ret);
will do and actually run some tests to make sure it's not only
build-tested.
Thanks,
Lucas De Marchi
--
Thanks,
Petr
