[PATCH RFC] btrfs: send: Emit all xattr of an inode if the uid/gid changed

[Marcos Paulo de Souza <marcos@mpdesouza.com>]

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