btrfs: extend BTRFS_IOC_SNAP_CREATE_V2 to snapshot by subvolid
PATCH 1/1 btrfs: extend BTRFS_IOC_SNAP_CREATE_V2 to snapshot by subvolid
The BTRFS_IOC_SNAP_CREATE_V2 ioctl is limited by requiring that a file descriptor be passed in order to create the snapshot. This means that snapshots may only be created of trees that are available in the mounted namespace. We have a need to create snapshots from subvolumes outside of the namespace. This is already possible by mounting the numbered subvolume by ID on a separate mount point, creating the snapshot, and unmounting it. That's a tedious and unnecessary process since the ioctl can be extended so easily.
This patch adds a new BTRFS_SUBVOL_CREATE_SUBVOLID flag that instructs the ioctl to use the fd argument (which is now a union) as a subvolume id instead. The subvolume ID is used to look up the root and instantiate the inode so proper permission checking takes place.
--- fs/btrfs/ioctl.c | 77 +++++++++++++++++++++++++++++++++++---------- include/uapi/linux/btrfs.h | 6 ++- 2 files changed, 65 insertions(+), 18 deletions(-)
fs/btrfs/ioctl.c +60 -17
@@ -1535,8 +1535,8 @@ out: } static noinline int btrfs_ioctl_snap_create_transid(struct file *file, -char *name,unsigned long fd,int subvol, -u64 *transid,bool readonly, +char *name,struct inode *src_inode, +int subvol,u64 *transid,bool readonly,struct btrfs_qgroup_inherit *inherit) {int namelen;
@@ -1562,14 +1562,6 @@ static noinline int btrfs_ioctl_snap_cre ret =btrfs_mksubvol (&file->f_path, name, namelen,NULL , transid, readonly, inherit); }else { -struct fd src =fdget (fd); -struct inode *src_inode; -if (!src.file) { - ret = -EINVAL ; - goto out_drop_write; - } - - src_inode =file_inode (src.file);if (src_inode->i_sb !=file_inode (file)->i_sb) {btrfs_info (BTRFS_I (src_inode)->root->fs_info,"Snapshot src from another FS" );
@@ -1585,7 +1577,6 @@ static noinline int btrfs_ioctl_snap_creBTRFS_I (src_inode)->root, transid, readonly, inherit); } -fdput (src); } out_drop_write:mnt_drop_write_file (file);
@@ -1597,6 +1588,7 @@ static noinline int btrfs_ioctl_snap_cre void __user *arg, int subvol) {struct btrfs_ioctl_vol_args *vol_args; +struct fd src;int ret; vol_args =memdup_user (arg,sizeof (*vol_args));
@@ -1604,10 +1596,19 @@ static noinline int btrfs_ioctl_snap_crereturn PTR_ERR (vol_args); vol_args->name[BTRFS_PATH_NAME_MAX ] ='\0' ; +if (!subvol) { + src =fdget (vol_args->fd); +if (!src.file) { + ret = -EINVAL ; + goto out_free; + } + } + ret =btrfs_ioctl_snap_create_transid (file, vol_args->name, - vol_args->fd, subvol, +file_inode (src.file), subvol,NULL , false,NULL ); - +fdput (src); +out_free:kfree (vol_args);return ret; }
@@ -1621,6 +1622,8 @@ static noinline int btrfs_ioctl_snap_creu64 *ptr =NULL ;bool readonly = false;struct btrfs_qgroup_inherit *inherit =NULL ; +struct fd src; +struct inode *src_inode =NULL ; vol_args =memdup_user (arg,sizeof (*vol_args));if (IS_ERR (vol_args))
@@ -1629,7 +1632,7 @@ static noinline int btrfs_ioctl_snap_creif (vol_args->flags & ~(BTRFS_SUBVOL_CREATE_ASYNC |BTRFS_SUBVOL_RDONLY | -BTRFS_SUBVOL_QGROUP_INHERIT )) { +BTRFS_SUBVOL_QGROUP_INHERIT |BTRFS_SUBVOL_CREATE_SUBVOLID )) { ret = -EOPNOTSUPP ; goto out; }
@@ -1650,9 +1653,49 @@ static noinline int btrfs_ioctl_snap_cre } } - ret =btrfs_ioctl_snap_create_transid (file, vol_args->name, - vol_args->fd, subvol, ptr, - readonly, inherit); +if (!subvol) { +if (vol_args->flags &BTRFS_SUBVOL_CREATE_SUBVOLID ) { +struct super_block *sb =file_inode (file)->i_sb; +struct btrfs_root *root; +struct btrfs_key location = { + .objectid = vol_args->subvolid, + .offset =-1 , + .type =BTRFS_ROOT_ITEM_KEY , + }; + + root =btrfs_get_fs_root (btrfs_sb (sb), &location, true); +if (IS_ERR (root)) { + ret =PTR_ERR (root); + goto out; + } + + location.objectid =btrfs_root_dirid (&root->root_item); + location.type =BTRFS_INODE_ITEM_KEY ; + location.offset =0 ; + src_inode =btrfs_iget (sb, &location, root,NULL ); +if (IS_ERR (src_inode)) { + ret =PTR_ERR (src_inode); + goto out; + } + }else { + src =fdget (vol_args->fd); +if (!src.file) { + ret = -EINVAL ; + goto out; + } + src_inode =file_inode (src.file); + } + } + + ret =btrfs_ioctl_snap_create_transid (file, vol_args->name, src_inode, + subvol, ptr, readonly, inherit); + +if (!subvol) { +if (vol_args->flags &BTRFS_SUBVOL_CREATE_SUBVOLID ) +iput (src_inode); +else +fdput (src); + }if (ret ==0 && ptr && copy_to_user(arg +
include/uapi/linux/btrfs.h +5 -1
@@ -36,6 +36,7 @@ struct btrfs_ioctl_vol_args {#define BTRFS_SUBVOL_CREATE_ASYNC (1ULL << 0)#define BTRFS_SUBVOL_RDONLY (1ULL << 1)#define BTRFS_SUBVOL_QGROUP_INHERIT (1ULL << 2) +#define BTRFS_SUBVOL_CREATE_SUBVOLID (1ULL << 3)#define BTRFS_FSID_SIZE 16#define BTRFS_UUID_SIZE 16
@@ -65,7 +66,10 @@ struct btrfs_ioctl_qgroup_limit_args {#define BTRFS_SUBVOL_NAME_MAX 4039struct btrfs_ioctl_vol_args_v2 { -__s64 fd; +union { +__s64 fd; +__u64 subvolid; + };__u64 transid;__u64 flags;union {
Signature · 2 lines
Jeff Mahoney SUSE Labs
Quoted 9 lines · Jeff Mahoney
The
BTRFS_IOC_SNAP_CREATE_V2ioctl is limited by requiring that a file descriptor be passed in order to create the snapshot. This means that snapshots may only be created of trees that are available in the mounted namespace. We have a need to create snapshots from subvolumes outside of the namespace. This is already possible by mounting the numbered subvolume by ID on a separate mount point, creating the snapshot, and unmounting it. That's a tedious and unnecessary process since the ioctl can be extended so easily.
Quoted 4 lines
This patch adds a new
BTRFS_SUBVOL_CREATE_SUBVOLIDflag that instructs the ioctl to use the fd argument (which is now a union) as a subvolume id instead. The subvolume ID is used to look up the root and instantiate the inode so proper permission checking takes place.
Can you please make this root only? The file handle makes sure the admin hasn't intentionally hidden the subvol from the user, and this bypasses those permission checks.
-chris
Quoted 10 lines · Chris Mason
Quoted 9 lines · Jeff Mahoney
The
BTRFS_IOC_SNAP_CREATE_V2ioctl is limited by requiring that a file descriptor be passed in order to create the snapshot. This means that snapshots may only be created of trees that are available in the mounted namespace. We have a need to create snapshots from subvolumes outside of the namespace. This is already possible by mounting the numbered subvolume by ID on a separate mount point, creating the snapshot, and unmounting it. That's a tedious and unnecessary process since the ioctl can be extended so easily.
Quoted 8 lines
Quoted 4 lines
This patch adds a new
BTRFS_SUBVOL_CREATE_SUBVOLIDflag that instructs the ioctl to use the fd argument (which is now a union) as a subvolume id instead. The subvolume ID is used to look up the root and instantiate the inode so proper permission checking takes place.Can you please make this root only? The file handle makes sure the admin hasn't intentionally hidden the subvol from the user, and this bypasses those permission checks.
I'll take care of this and send an updated patch.