FAILED: patch "[PATCH] btrfs: send: emit file capabilities after chown" failed to apply to 4.4-stable tree

PATCH [email protected] 2 participants 3 messages Jun 18, 2020 – Jun 19, 2020

Subsystems (2)

Stable branch Btrfs file system

lore

Share & tools

lore

[email protected]

author maintainer

2020-06-18 14:45 UTC #0 lore raw

The patch below does not apply to the 4.4-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From 89efda52e6b6930f80f5adda9c3c9edfb1397191 Mon Sep 17 00:00:00 2001
From: Marcos Paulo de Souza <[email protected]>
Date: Sun, 10 May 2020 23:15:07 -0300
Subject: [PATCH] btrfs: send: emit file capabilities after chown

Whenever a chown is executed, all capabilities of the file being touched are lost. When doing incremental send with a file with capabilities, there is a situation where the capability can be lost on the receiving side. The sequence of actions bellow shows the problem:

  $ mount /dev/sda fs1
  $ mount /dev/sdb fs2

  $ touch fs1/foo.bar
  $ setcap cap_sys_nice+ep fs1/foo.bar
  $ btrfs subvolume snapshot -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 subvolume snapshot -r fs1 fs1/snap_complete
  $ btrfs subvolume snapshot -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, only a chown was emitted by "btrfs send" since only the group was changed. This makes the cap_sys_nice capability to be dropped from fs2/snap_incremental/foo.bar

To fix that, only emit capabilities after chown is emitted. The current code first checks for xattrs that are new/changed, emits them, and later emit the chown. Now, __process_new_xattr skips capabilities, letting only finish_inode_if_needed to emit them, if they exist, for the inode being processed.

This behavior was being worked around in "btrfs receive" side by caching the capability and only applying it after chown. Now, xattrs are only emmited _after_ chown, making that workaround not needed anymore.

Link https://github.com/kdave/btrfs-progs/issues/202

CC [email protected] # 4.4+

Reviewed-by Filipe Manana <[email protected]>

Signed-off-by Marcos Paulo de Souza <[email protected]>

Signed-off-by David Sterba <[email protected]>

fs/btrfs/send.c +67 -0

index c5f41bd86765..4f3b8d2bb56b 100644
@@ -23,6 +23,7 @@
 #include "btrfs_inode.h"
 #include "transaction.h"
 #include "compression.h"
+#include "xattr.h"
 
 /*
  * Maximum number of references an extent can have in order for us to attempt to

@@ -4545,6 +4546,10 @@ static int __process_new_xattr(int num, struct btrfs_key *di_key,
 	struct fs_path *p;
 	struct posix_acl_xattr_header dummy_acl;
 
+	/* Capabilities are emitted by finish_inode_if_needed */
+	if (!strncmp(name, XATTR_NAME_CAPS, name_len))
+		return 0;
+
 	p = fs_path_alloc();
 	if (!p)
 		return -ENOMEM;

@@ -5107,6 +5112,64 @@ static int send_extent_data(struct send_ctx *sctx,
 	return 0;
 }
 
+/*
+ * Search for a capability xattr related to sctx->cur_ino. If the capability is
+ * found, call send_set_xattr function to emit it.
+ *
+ * Return 0 if there isn't a capability, or when the capability was emitted
+ * successfully, or < 0 if an error occurred.
+ */
+static int send_capabilities(struct send_ctx *sctx)
+{
+	struct fs_path *fspath = NULL;
+	struct btrfs_path *path;
+	struct btrfs_dir_item *di;
+	struct extent_buffer *leaf;
+	unsigned long data_ptr;
+	char *buf = NULL;
+	int buf_len;
+	int ret = 0;
+
+	path = alloc_path_for_send();
+	if (!path)
+		return -ENOMEM;
+
+	di = btrfs_lookup_xattr(NULL, sctx->send_root, path, sctx->cur_ino,
+				XATTR_NAME_CAPS, strlen(XATTR_NAME_CAPS), 0);
+	if (!di) {
+		/* There is no xattr for this inode */
+		goto out;
+	} else if (IS_ERR(di)) {
+		ret = PTR_ERR(di);
+		goto out;
+	}
+
+	leaf = path->nodes[0];
+	buf_len = btrfs_dir_data_len(leaf, di);
+
+	fspath = fs_path_alloc();
+	buf = kmalloc(buf_len, GFP_KERNEL);
+	if (!fspath || !buf) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	ret = get_cur_path(sctx, sctx->cur_ino, sctx->cur_inode_gen, fspath);
+	if (ret < 0)
+		goto out;
+
+	data_ptr = (unsigned long)(di + 1) + btrfs_dir_name_len(leaf, di);
+	read_extent_buffer(leaf, buf, data_ptr, buf_len);
+
+	ret = send_set_xattr(sctx, fspath, XATTR_NAME_CAPS,
+			strlen(XATTR_NAME_CAPS), buf, buf_len);
+out:
+	kfree(buf);
+	fs_path_free(fspath);
+	btrfs_free_path(path);
+	return ret;
+}
+
 static int clone_range(struct send_ctx *sctx,
 		       struct clone_root *clone_root,
 		       const u64 disk_byte,

@@ -5972,6 +6035,10 @@ static int finish_inode_if_needed(struct send_ctx *sctx, int at_end)
 			goto out;
 	}
 
+	ret = send_capabilities(sctx);
+	if (ret < 0)
+		goto out;
+
 	/*
 	 * If other directory inodes depended on our current directory
 	 * inode's move/rename, now do their move/rename operations.

Sasha Levin

maintainer

2020-06-19 01:19 UTC #1 lore raw

Quoted 53 lines · On Thu, Jun 18, 2020 at 04:45:23PM +0200, [email protected] wrote:

The patch below does not apply to the 4.4-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From 89efda52e6b6930f80f5adda9c3c9edfb1397191 Mon Sep 17 00:00:00 2001
From: Marcos Paulo de Souza <[email protected]>
Date: Sun, 10 May 2020 23:15:07 -0300
Subject: [PATCH] btrfs: send: emit file capabilities after chown
Whenever a chown is executed, all capabilities of the file being touched are lost. When doing incremental send with a file with capabilities, there is a situation where the capability can be lost on the receiving side. The sequence of actions bellow shows the problem:
 $ mount /dev/sda fs1
 $ mount /dev/sdb fs2
 $ touch fs1/foo.bar
 $ setcap cap_sys_nice+ep fs1/foo.bar
 $ btrfs subvolume snapshot -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 subvolume snapshot -r fs1 fs1/snap_complete
 $ btrfs subvolume snapshot -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, only a chown was emitted by "btrfs send" since only the group was changed. This makes the cap_sys_nice capability to be dropped from fs2/snap_incremental/foo.bar
To fix that, only emit capabilities after chown is emitted. The current code first checks for xattrs that are new/changed, emits them, and later emit the chown. Now, __process_new_xattr skips capabilities, letting only finish_inode_if_needed to emit them, if they exist, for the inode being processed.
This behavior was being worked around in "btrfs receive" side by caching the capability and only applying it after chown. Now, xattrs are only emmited _after_ chown, making that workaround not needed anymore.

Quoted 6 lines

Link https://github.com/kdave/btrfs-progs/issues/202
CC [email protected] # 4.4+
Suggested-by Filipe Manana <[email protected]>
Reviewed-by Filipe Manana <[email protected]>
Signed-off-by Marcos Paulo de Souza <[email protected]>
Signed-off-by David Sterba <[email protected]>

I took 1ec9a1ae1e30 ("Btrfs: fix unreplayable log after snapshot delete
+ parent dir fsync") to work around the conflict.

Signature · 2 lines

Thanks,
Sasha

Greg KH

author maintainer

2020-06-19 08:31 UTC #2 lore raw

Quoted 61 lines · Sasha Levin

Quoted 53 lines · On Thu, Jun 18, 2020 at 04:45:23PM +0200, [email protected] wrote:
The patch below does not apply to the 4.4-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
Quoted 1 line
From 89efda52e6b6930f80f5adda9c3c9edfb1397191 Mon Sep 17 00:00:00 2001
From: Marcos Paulo de Souza <[email protected]>
Date: Sun, 10 May 2020 23:15:07 -0300
Subject: [PATCH] btrfs: send: emit file capabilities after chown
Whenever a chown is executed, all capabilities of the file being touched are lost. When doing incremental send with a file with capabilities, there is a situation where the capability can be lost on the receiving side. The sequence of actions bellow shows the problem:
 $ mount /dev/sda fs1
 $ mount /dev/sdb fs2
 $ touch fs1/foo.bar
 $ setcap cap_sys_nice+ep fs1/foo.bar
 $ btrfs subvolume snapshot -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 subvolume snapshot -r fs1 fs1/snap_complete
 $ btrfs subvolume snapshot -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, only a chown was emitted by "btrfs send" since only the group was changed. This makes the cap_sys_nice capability to be dropped from fs2/snap_incremental/foo.bar
To fix that, only emit capabilities after chown is emitted. The current code first checks for xattrs that are new/changed, emits them, and later emit the chown. Now, __process_new_xattr skips capabilities, letting only finish_inode_if_needed to emit them, if they exist, for the inode being processed.
This behavior was being worked around in "btrfs receive" side by caching the capability and only applying it after chown. Now, xattrs are only emmited _after_ chown, making that workaround not needed anymore.
Quoted 6 lines
Link https://github.com/kdave/btrfs-progs/issues/202
CC [email protected] # 4.4+
Suggested-by Filipe Manana <[email protected]>
Reviewed-by Filipe Manana <[email protected]>
Signed-off-by Marcos Paulo de Souza <[email protected]>
Signed-off-by David Sterba <[email protected]>

Quoted 2 lines

I took 1ec9a1ae1e30 ("Btrfs: fix unreplayable log after snapshot delete
+ parent dir fsync") to work around the conflict.

Great, thanks for this fixup and the others you merged.

greg k-h