• Eric Biggers's avatar
    kmod: make request_module() return an error when autoloading is disabled · 048e29cb
    Eric Biggers authored
    BugLink: https://bugs.launchpad.net/bugs/1875905
    
    commit d7d27cfc 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: default avatarEric Biggers <ebiggers@google.com>
    Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
    Reviewed-by: default avatarKees Cook <keescook@chromium.org>
    Reviewed-by: default avatarJessica Yu <jeyu@kernel.org>
    Acked-by: default avatarLuis 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.orgSigned-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
    Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
    Signed-off-by: default avatarIan May <ian.may@canonical.com>
    Signed-off-by: default avatarKleber Sacilotto de Souza <kleber.souza@canonical.com>
    048e29cb
kmod.c 19.2 KB