Re: [PATCH] ext4: inherit encryption xattr before other xattrs
From: Andreas Dilger <hidden>
Date: 2017-02-27 20:28:24
Also in:
linux-ext4
On Feb 17, 2017, at 4:33 PM, Eric Biggers [off-list ref] wrote:
From: Eric Biggers <redacted> When using both encryption and SELinux (or another feature that requires an xattr per file) on a filesystem with 256-byte inodes, each file's xattrs usually spill into an external xattr block. Currently, the xattrs are inherited in the order ACL, security, then encryption. Therefore, if spillage occurs, the encryption xattr will always end up in the external block. This is not ideal because the encryption xattrs contain a nonce, so they will always be unique and will prevent the external xattr blocks from being deduplicated. To improve the situation, change the inheritance order to encryption, ACL, then security. This gives the encryption xattr a better chance to be stored in-inode, allowing the other xattr(s) to be deduplicated. Note that it may be better for userspace to format the filesystem with 512-byte inodes in this case. However, it's not the default. Signed-off-by: Eric Biggers <redacted>
Reviewed-by: Andreas Dilger <redacted> Note that we've been using 512-byte xattrs for Lustre metadata servers for ages. We may want to consider enabling this by default when the filesystem features are set (e.g. crypto, inline data, etc). Having a general mechanism to bias xattrs to in-inode or external xattr storage would be nice also, but I don't know any way to do this beyond just having a list of xattr names and then prioritizing the ones that go into the in-inode space. Cheers, Andreas
quoted hunk ↗ jump to hunk
--- fs/ext4/ialloc.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-)diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c index b14bae2598bc..0304e28c2014 100644 --- a/fs/ext4/ialloc.c +++ b/fs/ext4/ialloc.c@@ -1096,6 +1096,17 @@ struct inode *__ext4_new_inode(handle_t *handle, struct inode *dir,if (err) goto fail_drop; + /* + * Since the encryption xattr will always be unique, create it first so + * that it's less likely to end up in an external xattr block and + * prevent its deduplication. + */ + if (encrypt) { + err = fscrypt_inherit_context(dir, inode, handle, true); + if (err) + goto fail_free_drop; + } + err = ext4_init_acl(handle, inode, dir); if (err) goto fail_free_drop;@@ -1117,12 +1128,6 @@ struct inode *__ext4_new_inode(handle_t *handle, struct inode *dir,ei->i_datasync_tid = handle->h_transaction->t_tid; } - if (encrypt) { - err = fscrypt_inherit_context(dir, inode, handle, true); - if (err) - goto fail_free_drop; - } - err = ext4_mark_inode_dirty(handle, inode); if (err) { ext4_std_error(sb, err); -- 2.11.0.483.g087da7b7c-goog
Cheers, Andreas
Attachments
- signature.asc [application/pgp-signature] 195 bytes