DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 55CC9158A81
Organization: Red Hat UK Ltd. Registered Address: Red Hat UK Ltd, Amberley
        Place, 107-111 Peascod Street, Windsor, Berkshire, SI4 1TE, United
        Kingdom.
        Registered in England and Wales under Company Registration No. 3798903
From: David Howells <dhowells@redhat.com>
In-Reply-To: <87tw2qetbf.fsf@xmission.com>
References: <87tw2qetbf.fsf@xmission.com> <149926824154.20611.6104595541055328700.stgit@warthog.procyon.org.uk>
To: ebiederm@xmission.com (Eric W. Biederman)
Cc: dhowells@redhat.com, viro@zeniv.linux.org.uk,
        linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [RFC][PATCH 00/14] VFS: Make all filesystems implement ->show_options()
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <24032.1499443136.1@warthog.procyon.org.uk>
Date: Fri, 07 Jul 2017 16:58:56 +0100
Message-ID: <24033.1499443136@warthog.procyon.org.uk>
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2051
Lines: 48

Eric W. Biederman <ebiederm@xmission.com> wrote:

> > This makes it easier to implement a context-based mount where the options
> > are passed individually over a file descriptor.  It also allows duplicate
> > options, options that override each other and ignored options to be
> > resolved rather than storing irrelevant data.
> 
> This makes me a little nervous but is probably fine.  But we do need
> to be careful with remount.  Today the rule is all options that need
> to be preserved need to be passed to remount.  Passing options in one by
> one looks like it may make it easy to get that confused while you are
> developing your patches.

Yeah.  We actually currently get this *wrong* in both ext4 and btrfs - and
probably other disk filesystems too.  During ext4 remount and btrfs remount,
the options are parsed directly into the live xxx_fs_info struct, but if
there's a parse error mid-way, we only partially apply the options and have no
idea where it went wrong and what the current state is.

What I'm looking it is breaking it down into a number of steps:

 (1) Parse the options one at a time into a context struct.

 (2) Once we've got all the options, validate the options we've been given
     with respect to themselves.

 (3) Under lock:

     (a) Check the coherency of the options we've been given with respect to
     	 the superblock (if new mount) or the current live state (if remount).

     (b) Apply the options.  This is not permitted to fail.

This gets more complicated under ext4 as you've got an extra option string
stored on disk that you also have to apply and combine with the options
presented to the mount interface.

> Options should be relative to the mount call.  Which is almost always
> &init_user_ns and in the general case would be sb->s_user_ns.

I generally agree with this - my only doubt is that it might leak external
user/group IDs into a container.  Not sure if that's really worth worrying
about, though.

'user' permitted mounts are handled in userspace by a SUID mount program,
right?

David