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]

Date:   Tue, 9 May 2017 14:02:57 +0200
From:   Miklos Szeredi <mszeredi@...hat.com>
To:     David Howells <dhowells@...hat.com>
Cc:     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

On Tue, May 9, 2017 at 11:41 AM, David Howells <dhowells@...hat.com> wrote:
> 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.

Maybe we should.   In fact that sounds like a splendid idea.  IMO even
better, than having errors go via the fsfd descriptor.  Pretty cheap
on the kernel side, and completely optional on the userspace side.

>
>> 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?

MS_* flags are a mess.  I don't think they should be used for any new
functionality.  MNT_* flags are much better, but there are some
internal flags there as well.

I think the struct file model is better, where we have the external
O_* flags and the internal FMODE_* flags.

Thanks,
Miklos