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:38:48 +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:56 AM, David Howells <dhowells@...hat.com> wrote:
> Miklos Szeredi <mszeredi@...hat.com> wrote:
>
>> So say we have commands like
>>
>> "o+ foo"
>> "o- bar"
>
> The convention seems to be to prepend "no" to things you want to disable, so
> let's stick with that, e.g.:
>
>         "o foo"
>         "o nobar"
>
> otherwise we will have to have separate parsers for old mount() and the new sb
> config code - and not just for NFS, but at least for ext2/3/4 also.
>
> Further, we can only publish one format in /proc/mounts - and we cannot change
> that from the foo/nofoo standard we already use as it's part of the UAPI.

You're right, that this is a complicated issue and worth more
discussion.  And also you are right that we cannot change existing
UAPI, which is going to cause some headaches.

But that doesn't mean the new UAPI must follow the conventions of the
badly defined existing UAPI.

And the "no*" convention is anything but well defined, so we cannot
just stick it into generic code, because you'll find exceptions
everywhere.

And one more reason to have a new, unambiguous UAPI for retrieving
superblock options.

>
>> The generic option parser would just add or remove the option in the
>> current set of options,
>
> It sounds like you want to build up a string of "opt1,opt2,opt3" then have the
> VFS add and remove things from it and then parse it into the filesystem's
> internal structures on "commit".

That would be the default operation, if the filesystem doesn't define
its own parser.

>> and commit would just call ->remount_fs() with the new set of options.
>
> You're defining "commit" to do different things depending on the situation.
> You need a separation between "commit create" and "commit update".

It would be different, yes, at least until the superblock creation api
is completely transformed, at which point it may actually become the
same thing.  But lets not jump ahead.

>> It would probably not work for the NFS case, but that's okay, NFS can
>> implement its own option parsing.
>
> If NFS has to implement its own option parsing, we've done it wrong.

My above sentence was not clear.  What I meant to say that NFS needs
to implement the non-generic option parsing function in order to be
able to handle the case of "you can't change the server IP address".
Which it would want to do anyway, since it will result in cleaner
code.

Thanks,
Miklos