Skip to contentSkip to conversation

module: Tweak return and warning

PATCH Applied Lucas De Marchi 2 participants 4 messages Mar 30 – Apr 4 Module support

lore
Share & tools
lore
#0 lore raw

v2 cover module: Tweak return and warning

Do not let userspace tools and end users confused on the return code and log messages.

To: Luis Chamberlain <[email protected]>
To: Daniel Gomez <[email protected]>
To: Sami Tolvanen <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Aaron Tomlin <[email protected]>
Cc: Petr Pavlu <[email protected]>
Cc: Daniel Gomez <[email protected]>
Cc: Phil Sutter <[email protected]>
Cc: Christophe Leroy <[email protected]>
Cc: [email protected]
Cc: [email protected]

XXX "from-thread": "[email protected]"

XXX "change-id": "20260324-module-warn-ret-7bde7d4851be",

---
Lucas De Marchi (2):
module: Override -EEXISTS module return
module: Simplify warning on positive returns from module_init()

 kernel/module/main.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

---
base-commit: c369299895a591d96745d6492d4888259b004a9e change-id:

Best regards,

Signature · 1 line
Lucas De Marchi <[email protected]>
#1 lore raw

PATCH v2 1/2 module: Override -EEXISTS module return

The -EEXIST errno is reserved by the module loading functionality. When userspace calls [f]init_module(), it expects a -EEXIST to mean that the module is already loaded in the kernel. If module_init() returns it, that is not true anymore.

Override the error when returning to userspace: it doesn't make sense to change potentially long error propagation call chains just because it's will end up as the return of module_init().

Cc Greg Kroah-Hartman <[email protected]>
Cc Aaron Tomlin <[email protected]>
Cc Petr Pavlu <[email protected]>
Cc Daniel Gomez <[email protected]>
Cc Phil Sutter <[email protected]>
Cc Christophe Leroy <[email protected]>
---
 kernel/module/main.c | 8 ++++++++
 1 file changed, 8 insertions(+)
kernel/module/main.c +8 -0
index c3ce106c70af..3b60b7cda329 100644
@@ -3045,6 +3045,14 @@ static noinline int do_init_module(struct module *mod)
 	if (mod->init != NULL)
 		ret = do_one_initcall(mod->init);
 	if (ret < 0) {
+		/*
+		 * -EEXIST is reserved by [f]init_module() to signal to usersapace that
+		 * a module with this name is already loaded. Use something else if the
+		 * module itself returning that.
+		 */
+		if (ret == -EEXIST)
+			ret = -EBUSY;
+
 		goto fail_free_freeinit;
 	}
 	if (ret > 0) {
Signature · 1 line
2.53.0
#2 lore raw

PATCH v2 2/2 module: Simplify warning on positive returns from module_init()

In reply to Lucas De Marchi

It should now be rare to trigger this warning - it doesn't need to be so verbose. Make it follow the usual style in the module loading code.

For the same reason, drop the dump_stack().

---
 kernel/module/main.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)
kernel/module/main.c +3 -7
index 3b60b7cda329..5aeb5af584b4 100644
@@ -3055,13 +3055,9 @@ static noinline int do_init_module(struct module *mod)
 
 		goto fail_free_freeinit;
 	}
-	if (ret > 0) {
-		pr_warn("%s: '%s'->init suspiciously returned %d, it should "
-			"follow 0/-E convention\n"
-			"%s: loading module anyway...\n",
-			__func__, mod->name, ret, __func__);
-		dump_stack();
-	}
+	if (ret > 0)
+		pr_warn("%s: init suspiciously returned %d, it should follow 0/-E convention\n",
+			mod->name, ret);
 
 	/* Now it's a first class citizen! */
 	mod->state = MODULE_STATE_LIVE;
Signature · 1 line
2.53.0
maintainer Applied
#3 lore raw
In reply to Lucas De Marchi
Quoted 15 lines · Lucas De Marchi

Do not let userspace tools and end users confused on the return code and log messages.

To: Luis Chamberlain <[email protected]>
To: Daniel Gomez <[email protected]>
To: Sami Tolvanen <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Aaron Tomlin <[email protected]>
Cc: Petr Pavlu <[email protected]>
Cc: Daniel Gomez <[email protected]>
Cc: Phil Sutter <[email protected]>
Cc: Christophe Leroy <[email protected]>
Cc: [email protected]
Cc: [email protected]

Quoted 1 line

[...]

Applied to modules-next, thanks!

[1/2] module: Override -EEXIST module return

      commit: 743f8cae549affe8eafb021b8c0e78a9f3bc23fa
[2/2] module: Simplify warning on positive returns from module_init()
      commit: 663385f9155f27892a97a5824006f806a32eb8dc

Best regards,

Sami