Re: [PATCH] fs: warn in case userspace lied about modprobe return

[Jessica Yu <jeyu@redhat.com>]

+++ Luis R. Rodriguez [25/05/17 17:44 -0700]:
>kmod <= v19 was broken -- it could return 0 to modprobe calls,
>incorrectly assuming that a kernel module was built-in, whereas in
>reality the module was just forming in the kernel. The reason for this
>is an incorrect userspace heuristics. A userspace kmod fix is available
>for it [0], however should userspace break again we could go on with
>an failed get_fs_type() which is hard to debug as the request_module()
>is detected as returning 0. The first suspect would be that there is
>something worth with the kernel's module loader and obviously in this
>case that is not the issue.
>
>Since these issues are painful to debug complain when we know userspace
>has  outright lied to us.
>
>[0] http://git.kernel.org/cgit/utils/kernel/kmod/kmod.git/commit/libkmod/libkmod-module.c?id=fd44a98ae2eb5eb32161088954ab21e58e19dfc4
>
>Suggested-by: Rusty Russell <rusty@rustcorp.com.au>
>Cc: Jessica Yu <jeyu@redhat.com>
>Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
>---
> fs/filesystems.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
>diff --git a/fs/filesystems.c b/fs/filesystems.c
>index cac75547d35c..0f477a5de6ea 100644
>--- a/fs/filesystems.c
>+++ b/fs/filesystems.c
>@@ -275,8 +275,10 @@ struct file_system_type *get_fs_type(const char *name)
> 	int len = dot ? dot - name : strlen(name);
>
> 	fs = __get_fs_type(name, len);
>-	if (!fs && (request_module("fs-%.*s", len, name) == 0))
>+	if (!fs && (request_module("fs-%.*s", len, name) == 0)) {
>+		WARN_ONCE(!fs, "request_module fs-%.*s succeeded, but still no fs?\n", len, name);

The WARN needs to go below the second __get_fs_type() attempt, no?
Because we want to try __get_fs_type() again right after
request_module(), to see if the fs loaded, and _then_ WARN if it
doesn't appear to be loaded.

Jessica

> 		fs = __get_fs_type(name, len);
>+	}
>
> 	if (dot && fs && !(fs->fs_flags & FS_HAS_SUBTYPE)) {
> 		put_filesystem(fs);
>-- 
>2.11.0