lkml.org 
[lkml]   [2020]   [Mar]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH v2 1/4] kmod: make request_module() return an error when autoloading is disabled
    Date
    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.

    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>
    Cc: NeilBrown <neilb@suse.com>
    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 bc6addd9152b4..a2de58de6ab62 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
    \
     
     \ /
      Last update: 2020-03-12 21:26    [W:4.277 / U:0.500 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site