Messages in this thread | | | Date | Wed, 18 Mar 2020 17:03:16 +0100 | From | Jessica Yu <> | Subject | Re: [PATCH v3 1/5] kmod: make request_module() return an error when autoloading is disabled |
| |
+++ Eric Biggers [14/03/20 14:34 -0700]: >From: Eric Biggers <ebiggers@google.com> > >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. > >Acked-by: Luis Chamberlain <mcgrof@kernel.org> >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: NeilBrown <neilb@suse.com> >Signed-off-by: Eric Biggers <ebiggers@google.com>
Reviewed-by: Jessica Yu <jeyu@kernel.org>
| |