On Tue, Mar 10, 2020 at 03:37:31PM -0700, Eric Biggers wrote:
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
Hunh. I've never seen that before. :) I've always used;
echo 1 > /proc/sys/kernel/modules_disabled
Regardless,
Reviewed-by: Kees Cook keescook@chromium.org
-Kees
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