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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Thu, 1 Jun 2017 20:03:47 +0200
From:   "Luis R. Rodriguez" <mcgrof@...nel.org>
To:     Jessica Yu <jeyu@...hat.com>
Cc:     "Luis R. Rodriguez" <mcgrof@...nel.org>, viro@...iv.linux.org.uk,
        linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
        rusty@...tcorp.com.au
Subject: Re: [PATCH] fs: warn in case userspace lied about modprobe return

On Sat, May 27, 2017 at 07:01:45PM -0700, Jessica Yu wrote:
> +++ 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@...tcorp.com.au>
> > Cc: Jessica Yu <jeyu@...hat.com>
> > Signed-off-by: Luis R. Rodriguez <mcgrof@...nel.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.

Doh, yes, sorry will send v2.

  Luis