From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Sat, 27 May 2017 19:01:45 -0700 From: Jessica Yu To: "Luis R. Rodriguez" Cc: viro@zeniv.linux.org.uk, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, rusty@rustcorp.com.au Subject: Re: [PATCH] fs: warn in case userspace lied about modprobe return Message-ID: <20170528020144.3dgchl3wegukth2i@jeyu> References: <20170526004418.19550-1-mcgrof@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline In-Reply-To: <20170526004418.19550-1-mcgrof@kernel.org> Sender: linux-kernel-owner@vger.kernel.org List-ID: +++ 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 >Cc: Jessica Yu >Signed-off-by: Luis R. Rodriguez >--- > 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