From: Marcos Paulo de Souza <marcos@mpdesouza.com>
To: linux-btrfs@vger.kernel.org, dsterba@suse.com, wqu@suse.com
Cc: Marcos Paulo de Souza <mpdesouza@suse.com>
Subject: [PATCH RFC] btrfs: send: Emit all xattr of an inode if the uid/gid changed
Date: Tue, 24 Mar 2020 22:52:51 -0300 [thread overview]
Message-ID: <20200325015251.28838-1-marcos@mpdesouza.com> (raw)
From: Marcos Paulo de Souza <mpdesouza@suse.com>
[PROBLEM]
When doing incremental send with a file with capabilities, there is a
situation where the capability can be lost in the receiving side. The
sequence of actions bellow show the problem:
$ mount /dev/sda fs1
$ mount /dev/sdb fs2
$ touch fs1/foo.bar
$ setcap cap_sys_nice+ep fs1/foo.bar
$ btrfs subvol snap -r fs1 fs1/snap_init
$ btrfs send fs1/snap_init | btrfs receive fs2
$ chgrp adm fs1/foo.bar
$ setcap cap_sys_nice+ep fs1/foo.bar
$ btrfs subvol snap -r fs1 fs1/snap_complete
$ btrfs subvol snap -r fs1 fs1/snap_incremental
$ btrfs send fs1/snap_complete | btrfs receive fs2
$ btrfs send -p fs1/snap_init fs1/snap_incremental | btrfs receive fs2
At this point fs/snap_increment/foo.bar lost the capability, since a
chgrp was emitted by "btrfs send". The current code only checks for the
items that changed, and as the XATTR kept the value only the chgrp change
is emitted.
[FIX]
In order to fix this issue, check if the uid/gid of the inode change,
and if yes, emit all XATTR again, including the capability.
Fixes: https://github.com/kdave/btrfs-progs/issues/202
Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---
I'm posting this patch as a RFC because I had some questions
* Is this the correct place to fix?
* Also, emitting all XATTR of the inode seems overkill...
* Should it be fixed in userspace?
fs/btrfs/send.c | 29 +++++++++++++++++++++++++----
1 file changed, 25 insertions(+), 4 deletions(-)
diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index c5f41bd86765..5cffe5da91cf 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -6187,6 +6187,14 @@ static int changed_inode(struct send_ctx *sctx,
sctx->cur_inode_mode = btrfs_inode_mode(
sctx->right_path->nodes[0], right_ii);
} else if (result == BTRFS_COMPARE_TREE_CHANGED) {
+ u64 left_uid = btrfs_inode_uid(sctx->left_path->nodes[0],
+ left_ii);
+ u64 left_gid = btrfs_inode_gid(sctx->left_path->nodes[0],
+ left_ii);
+ u64 right_uid = btrfs_inode_uid(sctx->right_path->nodes[0],
+ right_ii);
+ u64 right_gid = btrfs_inode_gid(sctx->right_path->nodes[0],
+ right_ii);
/*
* We need to do some special handling in case the inode was
* reported as changed with a changed generation number. This
@@ -6236,15 +6244,12 @@ static int changed_inode(struct send_ctx *sctx,
sctx->send_progress = sctx->cur_ino + 1;
/*
- * Now process all extents and xattrs of the inode as if
+ * Now process all extents of the inode as if
* they were all new.
*/
ret = process_all_extents(sctx);
if (ret < 0)
goto out;
- ret = process_all_new_xattrs(sctx);
- if (ret < 0)
- goto out;
} else {
sctx->cur_inode_gen = left_gen;
sctx->cur_inode_new = 0;
@@ -6255,6 +6260,22 @@ static int changed_inode(struct send_ctx *sctx,
sctx->cur_inode_mode = btrfs_inode_mode(
sctx->left_path->nodes[0], left_ii);
}
+
+ /*
+ * Process all XATTR of the inode if the generation or owner
+ * changed.
+ *
+ * If the inode changed it's uid/gid, but kept a
+ * security.capability xattr, only the uid/gid will be emitted,
+ * causing the related xattr to deleted. For this reason always
+ * emit the XATTR when an inode has changed.
+ */
+ if (sctx->cur_inode_new_gen || left_uid != right_uid ||
+ left_gid != right_gid) {
+ ret = process_all_new_xattrs(sctx);
+ if (ret < 0)
+ goto out;
+ }
}
out:
--
2.25.1
next reply other threads:[~2020-03-25 1:50 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-25 1:52 Marcos Paulo de Souza [this message]
2020-03-25 1:56 ` [PATCH RFC] btrfs: send: Emit all xattr of an inode if the uid/gid changed Qu Wenruo
2020-03-26 13:51 ` Filipe Manana
2020-03-26 13:52 ` Filipe Manana
2020-03-26 17:14 ` David Sterba
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=20200325015251.28838-1-marcos@mpdesouza.com \
--to=marcos@mpdesouza.com \
--cc=dsterba@suse.com \
--cc=linux-btrfs@vger.kernel.org \
--cc=mpdesouza@suse.com \
--cc=wqu@suse.com \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).