[PATCH v2 0/2] module: Tweak return and warning

[PATCH v2 0/2] module: Tweak return and warning

[Lucas De Marchi]

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

To: Luis Chamberlain <mcgrof@kernel.org>
To: Daniel Gomez <da.gomez@kernel.org>
To: Sami Tolvanen <samitolvanen@google.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Aaron Tomlin <atomlin@atomlin.com>
Cc: Petr Pavlu <petr.pavlu@suse.com>
Cc: Daniel Gomez <da.gomez@samsung.com>
Cc: Phil Sutter <phil@nwl.cc>
Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
Cc: linux-modules@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

XXX    "from-thread": "20251013-module-warn-ret-v1-0-ab65b41af01f@intel.com"

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,
--  
Lucas De Marchi <demarchi@kernel.org>

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

[Lucas De Marchi]

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().

Closes: https://lore.kernel.org/all/aKLzsAX14ybEjHfJ@orbyte.nwl.cc/
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Aaron Tomlin <atomlin@atomlin.com>
Cc: Petr Pavlu <petr.pavlu@suse.com>
Cc: Daniel Gomez <da.gomez@samsung.com>
Cc: Phil Sutter <phil@nwl.cc>
Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
Signed-off-by: Lucas De Marchi <demarchi@kernel.org>
---
 kernel/module/main.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/kernel/module/main.c b/kernel/module/main.c
index c3ce106c70af..3b60b7cda329 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -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) {

-- 
2.53.0

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

[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().

Suggested-by: Petr Pavlu <petr.pavlu@suse.com>
Signed-off-by: Lucas De Marchi <demarchi@kernel.org>
Reviewed-by: Aaron Tomlin <atomlin@atomlin.com>
Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>
Reviewed-by: Daniel Gomez <da.gomez@samsung.com>
---
 kernel/module/main.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/kernel/module/main.c b/kernel/module/main.c
index 3b60b7cda329..5aeb5af584b4 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -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;

-- 
2.53.0