btrfs-util,rm-rf: clean up subvolumes without user_subvol_rm_allowed

Without CAP_SYS_ADMIN and without the 'user_subvol_rm_allowed' mount
option, BTRFS_IOC_SNAP_DESTROY is rejected with EPERM (or EROFS for a
read-only subvolume), so rm_rf_subvolume() left subvolumes behind.
test-btrfs thus accumulated leftover subvolumes in /var/tmp on every
unprivileged run on a btrfs filesystem.

An unprivileged owner can however clear the RDONLY flag, empty a
subvolume and rmdir() it. So clear the RDONLY flag on EPERM/EACCES too
(not just EROFS) to leave the subvolume writable, and let rm_rf() fall
through on EPERM/EACCES to empty the subvolume recursively and rmdir()
it, matching what rm_rf_at() already did.

Fixes https://github.com/systemd/systemd/issues/42674

diff --git a/src/shared/btrfs-util.c b/src/shared/btrfs-util.c
index f721e68753ab2a2912e5914d8da03d089caa384c..4833149005f15bfc1833f232c2a25c6736d117dc 100644 (file)
--- a/src/shared/btrfs-util.c
+++ b/src/shared/btrfs-util.c
@@ -929,23 +929,35 @@ static int subvol_remove_children(int fd, const char *subvolume, uint64_t subvol
          * regular files inside don't matter; it only returns ENOTEMPTY when there are nested
          * subvolumes (BTRFS_ROOT_REF_KEY entries), which we then handle below. */
         strncpy(vol_args.name, subvolume, sizeof(vol_args.name)-1);
+        int destroy_errno = 0;
         for (;;) {
                 if (ioctl(fd, BTRFS_IOC_SNAP_DESTROY, &vol_args) >= 0)
                         goto finish;
-
-                /* Without CAP_SYS_ADMIN the kernel runs an inode_permission(MAY_WRITE) check against
-                 * the subvolume root, which btrfs_permission() rejects with EROFS for a read-only
-                 * subvolume. Clear the RDONLY flag and retry. */
-                if (errno != EROFS || made_writable)
+                destroy_errno = errno;
+
+                /* Without CAP_SYS_ADMIN the destroy ioctl is rejected for a read-only subvolume: with
+                 * EROFS when 'user_subvol_rm_allowed' is set (btrfs_permission() fails the inode
+                 * MAY_WRITE check), or with EPERM/EACCES when it is not (the privilege check fires
+                 * first). In either case, if the subvolume is read-only, clear the RDONLY flag (only
+                 * inode ownership is required) and retry. This lets the retry succeed when removal is
+                 * permitted, and otherwise leaves the subvolume writable so the caller (e.g. rm_rf())
+                 * can empty and rmdir() it without privileges. */
+                if (made_writable || !ERRNO_IS_FS_WRITE_REFUSED(destroy_errno))
                         break;
 
+                r = btrfs_subvol_get_read_only_fd(subvol_fd);
+                if (r < 0)
+                        return r;
+                if (r == 0)
+                        break; /* Not read-only; clearing the flag would not help. */
+
                 r = btrfs_subvol_set_read_only_fd(subvol_fd, false);
                 if (r < 0)
                         return r;
                 made_writable = true;
         }
-        if (!(flags & BTRFS_REMOVE_RECURSIVE) || errno != ENOTEMPTY)
-                return -errno;
+        if (!(flags & BTRFS_REMOVE_RECURSIVE) || destroy_errno != ENOTEMPTY)
+                return -destroy_errno;
 
         /* OK, there are nested subvolumes — enumerate and recurse into them, then retry.
          * BTRFS_IOC_GET_SUBVOL_ROOTREF and BTRFS_IOC_INO_LOOKUP_USER (kernel 4.18+) enumerate child

diff --git a/src/shared/rm-rf.c b/src/shared/rm-rf.c
index ede18318abfe6509b466096a2e05a612b6633a99..573d6aeb045574d120d55ac5b791b3813822b498 100644 (file)
--- a/src/shared/rm-rf.c
+++ b/src/shared/rm-rf.c
@@ -228,10 +228,14 @@ static int rm_rf_inner_child(
 
                         r = btrfs_subvol_remove_at(fd, fname, BTRFS_REMOVE_RECURSIVE|BTRFS_REMOVE_QUOTA);
                         if (r < 0) {
-                                if (!IN_SET(r, -ENOTTY, -EINVAL))
+                                if (!IN_SET(r, -ENOTTY, -EINVAL, -EPERM, -EACCES))
                                         return r;
 
-                                /* ENOTTY, then it wasn't a btrfs subvolume, continue below. */
+                                /* ENOTTY, then it wasn't a btrfs subvolume. EPERM/EACCES means we lack
+                                 * the privileges for the destroy ioctl (no CAP_SYS_ADMIN and no
+                                 * 'user_subvol_rm_allowed'); btrfs_subvol_remove_at() will have cleared
+                                 * the read-only flag where it could, so fall through and try to empty the
+                                 * subvolume recursively and rmdir() it, which an unprivileged owner may do. */
                         } else
                                 /* It was a subvolume, done. */
                                 return 1;