linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Darrick J. Wong" <darrick.wong@oracle.com>
To: Ira Weiny <ira.weiny@intel.com>
Cc: "Theodore Y. Ts'o" <tytso@mit.edu>,
	linux-kernel@vger.kernel.org, Jan Kara <jack@suse.cz>,
	Dan Williams <dan.j.williams@intel.com>,
	Dave Chinner <david@fromorbit.com>,
	Christoph Hellwig <hch@lst.de>, Jeff Moyer <jmoyer@redhat.com>,
	linux-ext4@vger.kernel.org, linux-xfs@vger.kernel.org,
	linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH RFC 3/8] fs/ext4: Disallow encryption if inode is DAX
Date: Tue, 21 Apr 2020 11:56:57 -0700	[thread overview]
Message-ID: <20200421185657.GK6749@magnolia> (raw)
In-Reply-To: <20200421184143.GA3004764@iweiny-DESK2.sc.intel.com>

On Tue, Apr 21, 2020 at 11:41:43AM -0700, Ira Weiny wrote:
> On Wed, Apr 15, 2020 at 12:54:34PM -0700, 'Ira Weiny' wrote:
> > On Wed, Apr 15, 2020 at 12:03:07PM -0400, Theodore Y. Ts'o wrote:
> > > On Mon, Apr 13, 2020 at 09:00:25PM -0700, ira.weiny@intel.com wrote:
>  
> [snip]
> 
> > > 
> > > Also note that encrypted files are read/write so we must never allow
> > > the combination of ENCRPYT_FL and DAX_FL.  So that may be something
> > > where we should teach __ext4_iget() to check for this, and declare the
> > > file system as corrupted if it sees this combination.
> > 
> > ok...
> 
> After thinking about this...
> 
> Do we really want to declare the FS corrupted?

Seeing as we're defining the dax inode flag to be advisory (since its
value is ignored if the fs isn't on pmem, or the administrator overrode
with dax=never mount option), I don't see why that's filesystem
corruption.

I can see a case for returning errors if you're trying to change ENCRYPT
or VERITY on a file that's has S_DAX set.  We can't encrypt or set
verity data for a file that could be changed behind our backs, so the
kernel cannot satisfy /that/ request.

As for changing FS_DAX_FL on an encrypted/verity'd file, the API says
that it might not have an immedate effect on S_DAX and that programs
have to check S_DAX after changing FS_DAX_FL.  It'll never result in
S_DAX being set, but the current spec never guarantees that. ;)

(If FS_DAX_FL were *mandatory* then yes that would be corruption.)

--D

> If so, I think we need to return errors when such a configuration is attempted.
> If in the future we have an encrypted mode which can co-exist with DAX (such as
> Dan mentioned) we can change this.
> 
> FWIW I think we should return errors when such a configuration is attempted but
> _not_ declare the FS corrupted.  That allows users to enable this configuration
> later if we can figure out how to support it.
> 
> > 
> > > (For VERITY_FL
> > > && DAX_FL that is a combo that we might want to support in the future,
> > > so that's probably a case where arguably, we should just ignore the
> > > DAX_FL for now.)
> > 
> > ok...
> 
> I think this should work the same.
> 
> It looks like VERITY_FL and ENCRYPT_FL are _not_ user modifiable?  Is that
> correct?
> 
> You said that ENCRPYT_FL is set from the parent directory?  But I'm not seeing
> where that occurs?
> 
> Similarly I don't see where VERITY_FL is being set either?  :-/
> 
> I think to make this work correctly we should restrict setting those flags if
> DAX_FL is set and vice versa.  But I'm not finding where to do that.  :-/
> 
> Ira
> 
> > 
> > Ira
> > 

  reply	other threads:[~2020-04-21 18:57 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-14  4:00 [PATCH RFC 0/8] Enable ext4 support for per-file/directory DAX operations ira.weiny
2020-04-14  4:00 ` [PATCH RFC 1/8] fs/ext4: Narrow scope of DAX check in setflags ira.weiny
2020-04-15 11:45   ` Jan Kara
2020-04-14  4:00 ` [PATCH RFC 2/8] fs/ext4: Disallow verity if inode is DAX ira.weiny
2020-04-15 11:58   ` Jan Kara
2020-04-15 12:00   ` Jan Kara
2020-04-15 15:55     ` Theodore Y. Ts'o
2020-04-15 19:21       ` Ira Weiny
2020-04-15 19:14     ` Ira Weiny
2020-04-16  1:29       ` Eric Biggers
2020-04-16  3:48         ` Ira Weiny
2020-04-15 20:34     ` Ira Weiny
2020-04-14  4:00 ` [PATCH RFC 3/8] fs/ext4: Disallow encryption " ira.weiny
2020-04-15 12:02   ` Jan Kara
2020-04-15 20:35     ` Ira Weiny
2020-04-15 16:03   ` Theodore Y. Ts'o
2020-04-15 17:27     ` Dan Williams
2020-04-15 19:54     ` Ira Weiny
2020-04-21 18:41       ` Ira Weiny
2020-04-21 18:56         ` Darrick J. Wong [this message]
2020-04-14  4:00 ` [PATCH RFC 4/8] fs/ext4: Introduce DAX inode flag ira.weiny
2020-04-15 12:08   ` Jan Kara
2020-04-15 20:39     ` Ira Weiny
2020-04-16 10:32       ` Jan Kara
2020-04-16 18:01       ` Theodore Y. Ts'o
2020-04-16 16:25   ` Darrick J. Wong
2020-04-16 22:33     ` Ira Weiny
2020-04-16 22:49       ` Darrick J. Wong
2020-04-17  0:37         ` Ira Weiny
2020-04-17  1:57           ` Darrick J. Wong
2020-04-17  2:20             ` Ira Weiny
2020-04-17  6:43               ` Andreas Dilger
2020-04-17 17:19                 ` Ira Weiny
2020-04-14  4:00 ` [PATCH RFC 5/8] fs/ext4: Make DAX mount option a tri-state ira.weiny
2020-04-15 12:51   ` Jan Kara
2020-04-14  4:00 ` [PATCH RFC 6/8] fs/ext4: Update ext4_should_use_dax() ira.weiny
2020-04-15 13:58   ` Jan Kara
2020-04-17 17:16     ` Ira Weiny
2020-04-14  4:00 ` [PATCH RFC 7/8] fs/ext4: Only change S_DAX on inode load ira.weiny
2020-04-15 14:03   ` Jan Kara
2020-04-17 17:18     ` Ira Weiny
2020-04-14  4:00 ` [PATCH RFC 8/8] Documentation/dax: Update DAX enablement for ext4 ira.weiny

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200421185657.GK6749@magnolia \
    --to=darrick.wong@oracle.com \
    --cc=dan.j.williams@intel.com \
    --cc=david@fromorbit.com \
    --cc=hch@lst.de \
    --cc=ira.weiny@intel.com \
    --cc=jack@suse.cz \
    --cc=jmoyer@redhat.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=tytso@mit.edu \
    --subject='Re: [PATCH RFC 3/8] fs/ext4: Disallow encryption if inode is DAX' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).