Date:	Mon, 19 Oct 2015 09:12:34 -0400
From:	Austin S Hemmelgarn <ahferroin7@...il.com>
To:	Dave Chinner <david@...morbit.com>
Cc:	Andreas Gruenbacher <agruenba@...hat.com>,
	Alexander Viro <viro@...iv.linux.org.uk>,
	Theodore Ts'o <tytso@....edu>,
	Andreas Dilger <adilger.kernel@...ger.ca>,
	"J. Bruce Fields" <bfields@...ldses.org>,
	Jeff Layton <jlayton@...chiereds.net>,
	Trond Myklebust <trond.myklebust@...marydata.com>,
	Anna Schumaker <anna.schumaker@...app.com>,
	linux-ext4 <linux-ext4@...r.kernel.org>, xfs@....sgi.com,
	LKML <linux-kernel@...r.kernel.org>,
	linux-fsdevel <linux-fsdevel@...r.kernel.org>,
	Linux NFS Mailing List <linux-nfs@...r.kernel.org>,
	linux-cifs@...r.kernel.org, Linux API <linux-api@...r.kernel.org>,
	"Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>
Subject: Re: [PATCH v11 21/48] ext4: Add richacl feature flag

On 2015-10-17 19:17, Dave Chinner wrote:
> On Fri, Oct 16, 2015 at 02:27:57PM -0400, Austin S Hemmelgarn wrote:
>> On 2015-10-16 13:41, Andreas Gruenbacher wrote:
>>> On Fri, Oct 16, 2015 at 7:31 PM, Austin S Hemmelgarn
>>> <ahferroin7@...il.com> wrote:
>>>> I would like to re-iterate, on both XFS and ext4, I _really_ think this
>>>> should be a ro_compat flag, and not an incompat one.  If a person has the
>>>> ability to mount the FS (even if it's a read-only mount), then they by
>>>> definition have read access to the file or partition that the filesystem is
>>>> contained in, which means that any ACL's stored on the filesystem are
>>>> functionally irrelevant,
>>>
>>> It is unfortunately not safe to make such a file system accessible to
>>> other users, so the feature is not strictly read-only compatible.
>> If it's not safe WRT data integrity, then the design needs to be
>> reworked, as that directly implies that isn't safe for even every
>> day usage on a writable filesystem.
>
> This is exactly what we have *incompat feature flags for*: to
> protect old code that doesn't know about potentially dangerous new
> on-disk formats from trying to parse those formats and causing
> unpredictable bad things from happening.
However, unless things have changed (I haven't had time to re-read the 
patch-set yet), then the only change will be a new set of xattrs, and 
that type of change _shouldn't_ break existing code that doesn't know 
about them. Andreas really needs to explain _exactly_ why it isn't safe 
to mount this on a kernel that doesn't support it and let other users 
access it, and if the answer is 'because the ACL's won't be honored' 
then that really isn't acceptable reason IMHO, because (as I outlined in 
the previous e-mail) being able to mount the filesystem implies that 
they have at least read access to the underlying storage, which means 
that the ACL's in the filesystem are irrelevant as far as any competent 
individual who is actively trying to illegitimately access the data is 
concerned.
> Austin, your arguments hold no weight because they are no different
> to the considerations for any new on-disk feature: the user needs to
> have both kernel and userspace support to recover filesystems that
> go bad. If you are using a brand new fs/kernel feature, then it is
> expected that you know that your DR processes take this into
> account.
Except that the given argument from Andreas as to why it's an incompat 
feature does not clarify whether it's to prevent breaking the existing 
filesystem code (which I understand and agree is a proper usage for such 
a flag), or to try and provide some thin facade of security when there 
really is none (which is what the bit about 'and expose it to other 
users' really sounds like to me).  Yes the argument that I made which 
you have replied to was admittedly shortsighted and didn't need to be 
made to get the point that I was actually trying to make across, and I 
sincerely apologize for that.




Download attachment "smime.p7s" of type "application/pkcs7-signature" (3019 bytes)

Powered by blists - more mailing lists