linux-kernel - Re: [PATCH 3/9] VFS: Introduce a mount context

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <15762.1494322915@warthog.procyon.org.uk>
Date:   Tue, 09 May 2017 10:41:55 +0100
From:   David Howells <dhowells@...hat.com>
To:     Miklos Szeredi <mszeredi@...hat.com>
Cc:     dhowells@...hat.com, viro <viro@...iv.linux.org.uk>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        linux-nfs@...r.kernel.org, lkml <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 3/9] VFS: Introduce a mount context

Miklos Szeredi <mszeredi@...hat.com> wrote:

> I think that's crazy.  We don't return detailed errors for any other
> syscall for path lookup, so why would path lookup for mount be
> special.

Firstly, we don't return detailed errors for mount() at the moment either.

Secondly, path lookup might entail automounts, so perhaps we should do it for
path lookup too.  Particularly in light of the fact that NFS4 mount uses
pathwalk to get from server:/ to server:/the/dir/I/actually/wanted/ so I'm
currently losing that error:-/

Thirdly, the security operation I'm talking about is separate to path lookup -
though perhaps we should pass LOOKUP_MOUNT as an intent flag into pathwalk so
that the security check can be done there; perhaps combined with another one.

Fourthly, why shouldn't we consider extending the facility to other system
calls in future?  It would involve copying the string to task_struct and
providing a way to retrieve it, but that's not that hard to achieve.

> And why would
> 
>     fd = open("/foo/bar", O_PATH);
>     fsmount(fsfd, fd, NULL);
> 
> behave differently from
> 
>     fsmount(fsfd, -1, "/foo/bar");
> 
> ?

There's argument that the former should return EFAULT.  And that you should
set the path to "" and pass AT_EMPTY_PATH.  I should probably make sure it
does that - and add a flags field.  statx() was fixed to work this way.

Question for you: Should the MNT_* flags be passed to fsmount(), perhaps in
MS_* form?

David