| From a72fd1e748de50aec8e0a9c1fc78938704b9e353 Mon Sep 17 00:00:00 2001 |
| From: Eric Biggers <ebiggers@google.com> |
| Date: Fri, 10 Apr 2020 14:33:43 -0700 |
| Subject: [PATCH] kmod: make request_module() return an error when autoloading |
| is disabled |
| |
| commit d7d27cfc5cf0766a26a8f56868c5ad5434735126 upstream. |
| |
| Patch series "module autoloading fixes and cleanups", v5. |
| |
| This series fixes a bug where request_module() was reporting success to |
| kernel code when module autoloading had been completely disabled via |
| 'echo > /proc/sys/kernel/modprobe'. |
| |
| It also addresses the issues raised on the original thread |
| (https://lkml.kernel.org/lkml/20200310223731.126894-1-ebiggers@kernel.org/T/#u) |
| bydocumenting the modprobe sysctl, adding a self-test for the empty path |
| case, and downgrading a user-reachable WARN_ONCE(). |
| |
| This patch (of 4): |
| |
| It's long been possible to disable kernel module autoloading completely |
| (while still allowing manual module insertion) 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 |
| [...] |
| |
| This should actually use pr_warn_once() rather than WARN_ONCE(), since |
| it's also user-reachable if userspace immediately unloads the module. |
| Regardless, request_module() should 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. |
| |
| I've also sent patches to document and test this case. |
| |
| Signed-off-by: Eric Biggers <ebiggers@google.com> |
| Signed-off-by: Andrew Morton <akpm@linux-foundation.org> |
| Reviewed-by: Kees Cook <keescook@chromium.org> |
| Reviewed-by: Jessica Yu <jeyu@kernel.org> |
| Acked-by: Luis Chamberlain <mcgrof@kernel.org> |
| Cc: Alexei Starovoitov <ast@kernel.org> |
| Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| Cc: Jeff Vander Stoep <jeffv@google.com> |
| Cc: Ben Hutchings <benh@debian.org> |
| Cc: Josh Triplett <josh@joshtriplett.org> |
| Cc: <stable@vger.kernel.org> |
| Link: http://lkml.kernel.org/r/20200310223731.126894-1-ebiggers@kernel.org |
| Link: http://lkml.kernel.org/r/20200312202552.241885-1-ebiggers@kernel.org |
| Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> |
| Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com> |
| |
| 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.7.4 |
| |