btrfs-util: clear RDONLY flag on subvolume before destroy ioctl

Without CAP_SYS_ADMIN, btrfs_ioctl_snap_destroy() runs an
inode_permission(MAY_WRITE) check against the target subvolume root, which
btrfs_permission() rejects with EROFS for a read-only subvolume. As a
result, unprivileged removal of a read-only subvolume fails — both via
btrfs_subvol_remove_at() directly and via the recursive cleanup path used
by rm_rf_subvolume_and_freep(), which propagates the EROFS up.

Detect EROFS after the destroy ioctl, clear the RDONLY flag (only inode
ownership is required for BTRFS_IOC_SUBVOL_SETFLAGS), and retry once.

While at it, fix the surrounding comments: BTRFS_IOC_SNAP_DESTROY drops the
entire subvolume tree, so regular files inside are irrelevant; ENOTEMPTY
from the ioctl indicates nested subvolumes (BTRFS_ROOT_REF_KEY entries) via
may_destroy_subvol(), not non-empty contents.

diff --git a/src/shared/btrfs-util.c b/src/shared/btrfs-util.c
index 9c86e49f0f86c9a81537a6ca1b659dc4b4e8ea8e..33d6af3942ec6712ce01d248c8fdc6ae4e2a5285 100644 (file)
--- a/src/shared/btrfs-util.c
+++ b/src/shared/btrfs-util.c
@@ -891,8 +891,9 @@ static int subvol_remove_children(int fd, const char *subvolume, uint64_t subvol
         if (r == 0) /* Not a btrfs subvolume */
                 return -ENOTTY;
 
-        /* Before we try anything, let's see if 'user_subvol_rm_allowed' is enabled and we can just remove
-         * the dir directly */
+        /* Before we try anything, attempt VFS rmdir(). For an empty subvolume btrfs_rmdir dispatches
+         * to btrfs_delete_subvolume; for the non-empty case it returns ENOTEMPTY and we fall through
+         * to the destroy ioctl below. */
         if (unlinkat(fd, subvolume, AT_REMOVEDIR) >= 0)
                 goto finish;
 
@@ -902,15 +903,29 @@ static int subvol_remove_children(int fd, const char *subvolume, uint64_t subvol
                         return r;
         }
 
-        /* First, try to remove the subvolume. If it happens to be
-         * already empty, this will just work. */
+        /* Try the destroy ioctl. Unlike unlinkat() above, this drops the entire subvolume tree, so
+         * 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);
-        if (ioctl(fd, BTRFS_IOC_SNAP_DESTROY, &vol_args) >= 0)
-                goto finish;
+        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)
+                        break;
+
+                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;
 
-        /* OK, the subvolume is not empty, let's look for child subvolumes, and remove them, first.
+        /* 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
          * subvolumes without requiring CAP_SYS_ADMIN in the initial user namespace, unlike the older
          * BTRFS_IOC_TREE_SEARCH ioctl. */