LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] kmod: make request_module() return an error when autoloading is disabled
@ 2020-03-10 22:37 Eric Biggers
  2020-03-11  4:32 ` Luis Chamberlain
  2020-03-11 17:28 ` Kees Cook
  0 siblings, 2 replies; 15+ messages in thread
From: Eric Biggers @ 2020-03-10 22:37 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-fsdevel, stable, Alexei Starovoitov, Andrew Morton,
	Greg Kroah-Hartman, Jeff Vander Stoep, Jessica Yu, Kees Cook,
	Luis Chamberlain

From: Eric Biggers <ebiggers@google.com>

It's long been possible to disable kernel module autoloading completely
by setting /proc/sys/kernel/modprobe to the empty string.  This can be
preferable to setting it to a nonexistent file since it avoids the
overhead of an attempted execve(), avoids potential deadlocks, and
avoids the call to security_kernel_module_request() and thus on
SELinux-based systems eliminates the need to write SELinux rules to
dontaudit module_request.

However, when module autoloading is disabled in this way,
request_module() returns 0.  This is broken because callers expect 0 to
mean that the module was successfully loaded.

Apparently this was never noticed because this method of disabling
module autoloading isn't used much, and also most callers don't use the
return value of request_module() since it's always necessary to check
whether the module registered its functionality or not anyway.  But
improperly returning 0 can indeed confuse a few callers, for example
get_fs_type() in fs/filesystems.c where it causes a WARNING to be hit:

	if (!fs && (request_module("fs-%.*s", len, name) == 0)) {
		fs = __get_fs_type(name, len);
		WARN_ONCE(!fs, "request_module fs-%.*s succeeded, but still no fs?\n", len, name);
	}

This is easily reproduced with:

	echo > /proc/sys/kernel/modprobe
	mount -t NONEXISTENT none /

It causes:

	request_module fs-NONEXISTENT succeeded, but still no fs?
	WARNING: CPU: 1 PID: 1106 at fs/filesystems.c:275 get_fs_type+0xd6/0xf0
	[...]

Arguably this warning is broken and should be removed, since the module
could have been unloaded already.  However, request_module() should also
correctly return an error when it fails.  So let's make it return
-ENOENT, which matches the error when the modprobe binary doesn't exist.

Cc: stable@vger.kernel.org
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jeff Vander Stoep <jeffv@google.com>
Cc: Jessica Yu <jeyu@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Luis Chamberlain <mcgrof@kernel.org>
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 kernel/kmod.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/kmod.c b/kernel/kmod.c
index bc6addd9152b..a2de58de6ab6 100644
--- a/kernel/kmod.c
+++ b/kernel/kmod.c
@@ -120,7 +120,7 @@ static int call_modprobe(char *module_name, int wait)
  * invoke it.
  *
  * If module auto-loading support is disabled then this function
- * becomes a no-operation.
+ * simply returns -ENOENT.
  */
 int __request_module(bool wait, const char *fmt, ...)
 {
@@ -137,7 +137,7 @@ int __request_module(bool wait, const char *fmt, ...)
 	WARN_ON_ONCE(wait && current_is_async());
 
 	if (!modprobe_path[0])
-		return 0;
+		return -ENOENT;
 
 	va_start(args, fmt);
 	ret = vsnprintf(module_name, MODULE_NAME_LEN, fmt, args);
-- 
2.25.1.481.gfbce0eb801-goog


^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2020-03-11 18:40 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-10 22:37 [PATCH] kmod: make request_module() return an error when autoloading is disabled Eric Biggers
2020-03-11  4:32 ` Luis Chamberlain
2020-03-11  5:26   ` Eric Biggers
2020-03-11  6:31     ` Luis Chamberlain
2020-03-11 17:35       ` Eric Biggers
2020-03-11 18:00         ` Luis Chamberlain
2020-03-11 18:21           ` Eric Biggers
2020-03-11 18:40             ` Luis Chamberlain
2020-03-11  5:55   ` Josh Triplett
2020-03-11  6:32     ` Luis Chamberlain
2020-03-11 17:28 ` Kees Cook
2020-03-11 17:41   ` Eric Biggers
2020-03-11 17:50     ` Kees Cook
2020-03-11 18:01     ` Luis Chamberlain
2020-03-11 18:08       ` Eric Biggers

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).