From: Filipe Manana <fdman...@suse.com>
We had a few bugs on the kernel side of send/receive where capabilities
ended up being lost after receiving a send stream. They all stem from the
fact that the kernel used to send all xattrs before issuing the chown
command, and the later clears any existing capabilities in a file or
directory.
Initially a workaround was added to btrfs-progs' receive command, in commit
123a2a085027e ("btrfs-progs: receive: restore capabilities after chown"),
and that fixed some instances of the problem. More recently, other instances
of the problem were found, a proper fix for the kernel was made, which fixes
the root problem by making send always emit the sexattr command for setting
capabilities after issuing a chown command. This was done in kernel commit
89efda52e6b693 ("btrfs: send: emit file capabilities after chown"), which
landed in kernel 5.8.
However, the workaround on the receive command now causes us to incorrectly
set a capability on a file that should not have it, because it assumes all
setxattr commands for a file always comes before a chown.
Example reproducer:
$ cat send-caps.sh
#!/bin/bash
DEV1=/dev/sdh
DEV2=/dev/sdi
MNT1=/mnt/sdh
MNT2=/mnt/sdi
mkfs.btrfs -f $DEV1 > /dev/null
mkfs.btrfs -f $DEV2 > /dev/null
mount $DEV1 $MNT1
mount $DEV2 $MNT2
touch $MNT1/foo
touch $MNT1/bar
setcap cap_net_raw=p $MNT1/foo
btrfs subvolume snapshot -r $MNT1 $MNT1/snap1
btrfs send $MNT1/snap1 | btrfs receive $MNT2
echo
echo "capabilities on destination filesystem:"
echo
getcap $MNT2/snap1/foo
getcap $MNT2/snap1/bar
umount $MNT1
umount $MNT2
When running the test script, we can see that both files foo and bar get
the capability set, when only file foo should have it:
$ ./send-caps.sh
Create a readonly snapshot of '/mnt/sdh' in '/mnt/sdh/snap1'
At subvol /mnt/sdh/snap1
At subvol snap1
capabilities on destination filesystem:
/mnt/sdi/snap1/foo cap_net_raw=p
/mnt/sdi/snap1/bar cap_net_raw=p
Since the kernel fix was backported to all currently supported stable
releases (5.10.x, 5.4.x, 4.19.x, 4.14.x, 4.9.x and 4.4.x), remove the
workaround from receive. Having such a workaround relying on the order
of commands in a send stream is always troublesome and doomed to break
one day.
A test case for fstests will come soon.
Reported-by: Richard Brown <rbr...@suse.de>
Signed-off-by: Filipe Manana <fdman...@suse.com>
---
cmds/receive.c | 49 -------------------------------------------------
1 file changed, 49 deletions(-)
diff --git a/cmds/receive.c b/cmds/receive.c
index 2aaba3ff..b4099bc4 100644
--- a/cmds/receive.c
+++ b/cmds/receive.c
@@ -77,14 +77,6 @@ struct btrfs_receive
struct subvol_uuid_search sus;
int honor_end_cmd;
-
- /*
- * Buffer to store capabilities from security.capabilities xattr,
- * usually 20 bytes, but make same room for potentially larger
- * encodings. Must be set only once per file, denoted by length > 0.
- */
- char cached_capabilities[64];
- int cached_capabilities_len;
};
static int finish_subvol(struct btrfs_receive *rctx)
@@ -825,22 +817,6 @@ static int process_set_xattr(const char *path, const char
*name,
goto out;
}
- if (strcmp("security.capability", name) == 0) {
- if (bconf.verbose >= 4)
- fprintf(stderr, "set_xattr: cache capabilities\n");
- if (rctx->cached_capabilities_len)
- warning("capabilities set multiple times per file: %s",
- full_path);
- if (len > sizeof(rctx->cached_capabilities)) {
- error("capabilities encoded to %d bytes, buffer too
small",
- len);
- ret = -E2BIG;
- goto out;
- }
- rctx->cached_capabilities_len = len;
- memcpy(rctx->cached_capabilities, data, len);
- }
-
if (bconf.verbose >= 3) {
fprintf(stderr, "set_xattr %s - name=%s data_len=%d "
"data=%.*s\n", path, name, len,
@@ -961,23 +937,6 @@ static int process_chown(const char *path, u64 uid, u64
gid, void *user)
error("chown %s failed: %m", path);
goto out;
}
-
- if (rctx->cached_capabilities_len) {
- if (bconf.verbose >= 3)
- fprintf(stderr, "chown: restore capabilities\n");
- ret = lsetxattr(full_path, "security.capability",
- rctx->cached_capabilities,
- rctx->cached_capabilities_len, 0);
- memset(rctx->cached_capabilities, 0,
- sizeof(rctx->cached_capabilities));
- rctx->cached_capabilities_len = 0;
- if (ret < 0) {
- ret = -errno;
- error("restoring capabilities %s: %m", path);
- goto out;
- }
- }
-
out:
return ret;
}
@@ -1155,14 +1114,6 @@ static int do_receive(struct btrfs_receive *rctx, const
char *tomnt,
goto out;
while (!end) {
- if (rctx->cached_capabilities_len) {
- if (bconf.verbose >= 4)
- fprintf(stderr, "clear cached capabilities\n");
- memset(rctx->cached_capabilities, 0,
- sizeof(rctx->cached_capabilities));
- rctx->cached_capabilities_len = 0;
- }
-
ret = btrfs_read_and_process_send_stream(r_fd, &send_ops,
rctx,
rctx->honor_end_cmd,
--
2.28.0
