Liu Bo
2012-Jun-21 11:48 UTC
[PATCH 1/4] Btrfs: check write access to mount earlier while creating snapshots
Move check of write access to mount into upper functions so that we can use mnt_want_write_file instead, which is faster than mnt_want_write. Signed-off-by: Liu Bo <liubo2009@cn.fujitsu.com> --- fs/btrfs/ioctl.c | 22 +++++++++++----------- 1 files changed, 11 insertions(+), 11 deletions(-) diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 0e92e57..b5e946e 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -652,13 +652,9 @@ static noinline int btrfs_mksubvol(struct path *parent, if (dentry->d_inode) goto out_dput; - error = mnt_want_write(parent->mnt); - if (error) - goto out_dput; - error = btrfs_may_create(dir, dentry); if (error) - goto out_drop_write; + goto out_dput; down_read(&BTRFS_I(dir)->root->fs_info->subvol_sem); @@ -676,8 +672,6 @@ static noinline int btrfs_mksubvol(struct path *parent, fsnotify_mkdir(dir, dentry); out_up_read: up_read(&BTRFS_I(dir)->root->fs_info->subvol_sem); -out_drop_write: - mnt_drop_write(parent->mnt); out_dput: dput(dentry); out_unlock: @@ -1393,16 +1387,20 @@ static noinline int btrfs_ioctl_snap_create_transid(struct file *file, if (root->fs_info->sb->s_flags & MS_RDONLY) return -EROFS; + ret = mnt_want_write_file(file); + if (ret) + goto out; + namelen = strlen(name); if (strchr(name, ''/'')) { ret = -EINVAL; - goto out; + goto out_drop_write; } if (name[0] == ''.'' && (namelen == 1 || (name[1] == ''.'' && namelen == 2))) { ret = -EEXIST; - goto out; + goto out_drop_write; } if (subvol) { @@ -1413,7 +1411,7 @@ static noinline int btrfs_ioctl_snap_create_transid(struct file *file, src_file = fget(fd); if (!src_file) { ret = -EINVAL; - goto out; + goto out_drop_write; } src_inode = src_file->f_path.dentry->d_inode; @@ -1422,13 +1420,15 @@ static noinline int btrfs_ioctl_snap_create_transid(struct file *file, "another FS\n"); ret = -EINVAL; fput(src_file); - goto out; + goto out_drop_write; } ret = btrfs_mksubvol(&file->f_path, name, namelen, BTRFS_I(src_inode)->root, transid, readonly); fput(src_file); } +out_drop_write: + mnt_drop_write_file(file); out: return ret; } -- 1.6.5.2 -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
mnt_want_write() and mnt_want_write_file() will check sb->s_flags with MS_RDONLY, and we don''t need to do it ourselves. Signed-off-by: Liu Bo <liubo2009@cn.fujitsu.com> --- fs/btrfs/ioctl.c | 7 ------- 1 files changed, 0 insertions(+), 7 deletions(-) diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index b5e946e..069cd85 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -1379,14 +1379,10 @@ static noinline int btrfs_ioctl_snap_create_transid(struct file *file, u64 *transid, bool readonly) { - struct btrfs_root *root = BTRFS_I(fdentry(file)->d_inode)->root; struct file *src_file; int namelen; int ret = 0; - if (root->fs_info->sb->s_flags & MS_RDONLY) - return -EROFS; - ret = mnt_want_write_file(file); if (ret) goto out; @@ -3265,9 +3261,6 @@ static long btrfs_ioctl_balance(struct file *file, void __user *arg) if (!capable(CAP_SYS_ADMIN)) return -EPERM; - if (fs_info->sb->s_flags & MS_RDONLY) - return -EROFS; - ret = mnt_want_write(file->f_path.mnt); if (ret) return ret; -- 1.6.5.2 -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Liu Bo
2012-Jun-21 11:48 UTC
[PATCH 3/4] Btrfs: use mnt_want_write_file instead of mnt_want_write
mnt_want_write_file is faster when file has been opened for write. Signed-off-by: Liu Bo <liubo2009@cn.fujitsu.com> --- fs/btrfs/ioctl.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 069cd85..df4c04d 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -3261,7 +3261,7 @@ static long btrfs_ioctl_balance(struct file *file, void __user *arg) if (!capable(CAP_SYS_ADMIN)) return -EPERM; - ret = mnt_want_write(file->f_path.mnt); + ret = mnt_want_write_file(file); if (ret) return ret; @@ -3331,7 +3331,7 @@ out_bargs: out: mutex_unlock(&fs_info->balance_mutex); mutex_unlock(&fs_info->volume_mutex); - mnt_drop_write(file->f_path.mnt); + mnt_drop_write_file(file); return ret; } -- 1.6.5.2 -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Liu Bo
2012-Jun-21 11:48 UTC
[PATCH 4/4] Btrfs: do not set subvolume flags in readonly mode
$ mkfs.btrfs /dev/sdb7 $ btrfstune -S1 /dev/sdb7 $ mount /dev/sdb7 /mnt/btrfs mount: block device /dev/sdb7 is write-protected, mounting read-only $ btrfs dev add /dev/sdb8 /mnt/btrfs/ Now we get a btrfs in which mnt flags has readonly but sb flags does not. So for those ioctls that only check sb flags with MS_RDONLY, it is going to be a problem. Setting subvolume flags is such an ioctl, we should use mnt_want_write_file() to check RO flags. Signed-off-by: Liu Bo <liubo2009@cn.fujitsu.com> --- fs/btrfs/ioctl.c | 27 ++++++++++++++++++--------- 1 files changed, 18 insertions(+), 9 deletions(-) diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index df4c04d..ae29737 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -1519,29 +1519,35 @@ static noinline int btrfs_ioctl_subvol_setflags(struct file *file, u64 flags; int ret = 0; - if (root->fs_info->sb->s_flags & MS_RDONLY) - return -EROFS; + ret = mnt_want_write_file(file); + if (ret) + goto out; + ret = -EINVAL; if (btrfs_ino(inode) != BTRFS_FIRST_FREE_OBJECTID) - return -EINVAL; + goto out_drop_write; + ret = -EFAULT; if (copy_from_user(&flags, arg, sizeof(flags))) - return -EFAULT; + goto out_drop_write; + ret = -EINVAL; if (flags & BTRFS_SUBVOL_CREATE_ASYNC) - return -EINVAL; + goto out_drop_write; + ret = -EOPNOTSUPP; if (flags & ~BTRFS_SUBVOL_RDONLY) - return -EOPNOTSUPP; + goto out_drop_write; + ret = -EACCES; if (!inode_owner_or_capable(inode)) - return -EACCES; + goto out_drop_write; down_write(&root->fs_info->subvol_sem); /* nothing to do */ if (!!(flags & BTRFS_SUBVOL_RDONLY) == btrfs_root_readonly(root)) - goto out; + goto out_drop_sem; root_flags = btrfs_root_flags(&root->root_item); if (flags & BTRFS_SUBVOL_RDONLY) @@ -1564,8 +1570,11 @@ static noinline int btrfs_ioctl_subvol_setflags(struct file *file, out_reset: if (ret) btrfs_set_root_flags(&root->root_item, root_flags); -out: +out_drop_sem: up_write(&root->fs_info->subvol_sem); +out_drop_write: + mnt_drop_write_file(file); +out: return ret; } -- 1.6.5.2 -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Tsutomu Itoh
2012-Jun-21 23:57 UTC
Re: [PATCH 4/4] Btrfs: do not set subvolume flags in readonly mode
(2012/06/21 20:48), Liu Bo wrote:> $ mkfs.btrfs /dev/sdb7 > $ btrfstune -S1 /dev/sdb7 > $ mount /dev/sdb7 /mnt/btrfs > mount: block device /dev/sdb7 is write-protected, mounting read-only > $ btrfs dev add /dev/sdb8 /mnt/btrfs/ > > Now we get a btrfs in which mnt flags has readonly but sb flags does > not. So for those ioctls that only check sb flags with MS_RDONLY, it > is going to be a problem. > Setting subvolume flags is such an ioctl, we should use mnt_want_write_file() > to check RO flags. > > Signed-off-by: Liu Bo <liubo2009@cn.fujitsu.com> > --- > fs/btrfs/ioctl.c | 27 ++++++++++++++++++--------- > 1 files changed, 18 insertions(+), 9 deletions(-) > > diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c > index df4c04d..ae29737 100644 > --- a/fs/btrfs/ioctl.c > +++ b/fs/btrfs/ioctl.c > @@ -1519,29 +1519,35 @@ static noinline int btrfs_ioctl_subvol_setflags(struct file *file, > u64 flags; > int ret = 0; > > - if (root->fs_info->sb->s_flags & MS_RDONLY) > - return -EROFS; > + ret = mnt_want_write_file(file); > + if (ret) > + goto out; > > + ret = -EINVAL; > if (btrfs_ino(inode) != BTRFS_FIRST_FREE_OBJECTID) > - return -EINVAL; > + goto out_drop_write; > > + ret = -EFAULT; > if (copy_from_user(&flags, arg, sizeof(flags))) > - return -EFAULT; > + goto out_drop_write; > > + ret = -EINVAL; > if (flags & BTRFS_SUBVOL_CREATE_ASYNC) > - return -EINVAL; > + goto out_drop_write; > > + ret = -EOPNOTSUPP; > if (flags & ~BTRFS_SUBVOL_RDONLY) > - return -EOPNOTSUPP; > + goto out_drop_write; > > + ret = -EACCES; > if (!inode_owner_or_capable(inode)) > - return -EACCES; > + goto out_drop_write;I think that if (!inode_owner_or_capable(inode)) { ret = -EACCES; goto out_drop_write; } is better than ret = -EACCES; if (!inode_owner_or_capable(inode)) goto out_drop_write; Thanks, Tsutomu> > down_write(&root->fs_info->subvol_sem); > > /* nothing to do */ > if (!!(flags & BTRFS_SUBVOL_RDONLY) == btrfs_root_readonly(root)) > - goto out; > + goto out_drop_sem; > > root_flags = btrfs_root_flags(&root->root_item); > if (flags & BTRFS_SUBVOL_RDONLY) > @@ -1564,8 +1570,11 @@ static noinline int btrfs_ioctl_subvol_setflags(struct file *file, > out_reset: > if (ret) > btrfs_set_root_flags(&root->root_item, root_flags); > -out: > +out_drop_sem: > up_write(&root->fs_info->subvol_sem); > +out_drop_write: > + mnt_drop_write_file(file); > +out: > return ret; > } > >-- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html