Skip to content

Commit

Permalink
btrfs-progs: receive: remove workaround for setting capabilities
Browse files Browse the repository at this point in the history
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
123a2a0 ("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 setxattr 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.

Issue: #85
Issue: #202
Issue: #292
Reported-by: Richard Brown <rbrown@suse.de>
Reviewed-by: Su Yue <l@damenly.su>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
  • Loading branch information
fdmanana authored and kdave committed Feb 11, 2021
1 parent 591c69a commit bc4d83d
Showing 1 changed file with 0 additions and 49 deletions.
49 changes: 0 additions & 49 deletions cmds/receive.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit bc4d83d

Please sign in to comment.