On Feb 17, 2017, at 4:33 PM, Eric Biggers <ebiggers3@xxxxxxxxx> wrote:
>
> From: Eric Biggers <ebiggers@xxxxxxxxxx>
>
> 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 <ebiggers@xxxxxxxxxx>
Reviewed-by: Andreas Dilger <adilger@xxxxxxxxx>
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
> ---
> 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
Attachment:
signature.asc
Description: Message signed with OpenPGP
