@@ -2835,8 +2835,6 @@ static noinline int btrfs_ioctl_snap_destroy(struct file *file,
struct dentry *dentry;
struct inode *dir = d_inode(parent);
struct inode *inode;
- struct btrfs_root *root = BTRFS_I(dir)->root;
- struct btrfs_root *dest = NULL;
struct btrfs_ioctl_vol_args *vol_args = NULL;
struct btrfs_ioctl_vol_args_v2 *vol_args2 = NULL;
char *subvol_name, *subvol_name_ptr = NULL;
@@ -2962,39 +2960,11 @@ static noinline int btrfs_ioctl_snap_destroy(struct file *file,
}
inode = d_inode(dentry);
- dest = BTRFS_I(inode)->root;
if (!capable(CAP_SYS_ADMIN)) {
- /*
- * Regular user. Only allow this with a special mount
- * option, when the user has write+exec access to the
- * subvol root, and when rmdir(2) would have been
- * allowed.
- *
- * Note that this is _not_ check that the subvol is
- * empty or doesn't contain data that we wouldn't
- * otherwise be able to delete.
- *
- * Users who want to delete empty subvols should try
- * rmdir(2).
- */
+ /* Regular user. Only allow this with a special mount option */
err = -EPERM;
if (!btrfs_test_opt(fs_info, USER_SUBVOL_RM_ALLOWED))
goto out_dput;
-
- /*
- * Do not allow deletion if the parent dir is the same
- * as the dir to be deleted. That means the ioctl
- * must be called on the dentry referencing the root
- * of the subvol, not a random directory contained
- * within it.
- */
- err = -EINVAL;
- if (root == dest)
- goto out_dput;
-
- err = inode_permission(inode, MAY_WRITE | MAY_EXEC);
- if (err)
- goto out_dput;
}
/* check if subvolume may be deleted by a user */
With the user_subvol_rm_allowed mount flag set, it is possible for non root users to delete subvolumes. Currently, the conditions to do so diverge from the conditions for root. Specifically, there is an additional check on the ability to write to the subvolume to be deleted, not just its containing directory. As a result, we see weird behavior like root being able to delete a read only subvolume it doesn't have write permissions for, but a user not being able to delete a read only subvolume it does otherwise have write permissions for. Furthermore, the existing comments make allusions to rmdir being allowed as a standard, and rmdir only checks permissions on the containing directory, as in the root case. To streamline this, make user subvolume deletes with the mount flag set match the behavior for root. For increased security, it is simple to just mount without user_subvol_rm_allowed. To put it plainly, the risk is that a non-root user who has write access to a parent directory will be able to delete a subvolume in the parent, even if they don't have write access to the subvolume. Currently, only root can do that. Signed-off-by: Boris Burkov <boris@bur.io> --- - remove unused variable dest caught by kernel test robot fs/btrfs/ioctl.c | 32 +------------------------------- 1 file changed, 1 insertion(+), 31 deletions(-)