From: "Darrick J. Wong" <firstname.lastname@example.org> To: Ira Weiny <email@example.com> Cc: "Theodore Y. Ts'o" <firstname.lastname@example.org>, email@example.com, Jan Kara <firstname.lastname@example.org>, Dan Williams <email@example.com>, Dave Chinner <firstname.lastname@example.org>, Christoph Hellwig <email@example.com>, Jeff Moyer <firstname.lastname@example.org>, email@example.com, firstname.lastname@example.org, email@example.com 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, firstname.lastname@example.org 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 > >
next prev parent 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 \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --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).