Btrfs: fix race between enabling quotas and subvolume creation
PATCH 1/1 Btrfs: fix race between enabling quotas and subvolume creation
From: Filipe Manana <[email protected]>
We have a race between enabling quotas end subvolume creation that cause subvolume creation to fail with -EINVAL, and the following diagram shows how it happens:
CPU 0 CPU 1
btrfs_ioctl()
btrfs_ioctl_quota_ctl()
btrfs_quota_enable()
mutex_lock(fs_info->qgroup_ioctl_lock)
btrfs_ioctl()
create_subvol()
btrfs_qgroup_inherit()
-> save fs_info->quota_root
into quota_root
-> stores a NULL value
-> tries to lock the mutex
qgroup_ioctl_lock
-> blocks waiting for
the task at CPU0 -> sets BTRFS_FS_QUOTA_ENABLED in fs_info
-> sets quota_root in fs_info->quota_root
(non-NULL value)
mutex_unlock(fs_info->qgroup_ioctl_lock)
-> checks quota enabled
flag is set
-> returns -EINVAL because
fs_info->quota_root was
NULL before it acquired
the mutex
qgroup_ioctl_lock
-> ioctl returns -EINVALReturning -EINVAL to user space will be confusing if all the arguments passed to the subvolume creation ioctl were valid.
Fix it by grabbing the value from fs_info->quota_root after acquiring the mutex.
--- fs/btrfs/qgroup.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
fs/btrfs/qgroup.c +2 -1
@@ -2250,7 +2250,7 @@ int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans, u64 srcid,int i;u64 *i_qgroups;struct btrfs_fs_info *fs_info = trans->fs_info; -struct btrfs_root *quota_root = fs_info->quota_root; +struct btrfs_root *quota_root;struct btrfs_qgroup *srcgroup;struct btrfs_qgroup *dstgroup;u32 level_size =0 ;
@@ -2260,6 +2260,7 @@ int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans, u64 srcid,if (!test_bit (BTRFS_FS_QUOTA_ENABLED , &fs_info->flags)) goto out; + quota_root = fs_info->quota_root;if (!quota_root) { ret = -EINVAL ; goto out;
Signature · 1 line
2.11.0
Quoted 39 lines · On 2018/11/20 上午12:20, [email protected] wrote:
From: Filipe Manana <[email protected]>
We have a race between enabling quotas end subvolume creation that cause subvolume creation to fail with
-EINVAL, and the following diagram shows how it happens:CPU 0 CPU 1
btrfs_ioctl()
btrfs_ioctl_quota_ctl()
btrfs_quota_enable()mutex_lock(fs_info->qgroup_ioctl_lock) btrfs_ioctl() create_subvol() btrfs_qgroup_inherit() -> save fs_info->quota_root into quota_root -> stores a NULL value -> tries to lock the mutex qgroup_ioctl_lock -> blocks waiting for the task at CPU0-> sets BTRFS_FS_QUOTA_ENABLED in fs_info -> sets quota_root in fs_info->quota_root (non-NULL value) mutex_unlock(fs_info->qgroup_ioctl_lock) -> checks quota enabled flag is set -> returns -EINVAL because fs_info->quota_root was NULL before it acquired the mutex qgroup_ioctl_lock -> ioctl returns -EINVAL
Quoted 7 lines
Returning
-EINVALto user space will be confusing if all the arguments passed to the subvolume creation ioctl were valid.Fix it by grabbing the value from
fs_info->quota_rootafter acquiring the mutex.Signed-off-by Filipe Manana <[email protected]>
Thanks,
Qu
Quoted 26 lines
--- fs/btrfs/qgroup.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)fs/btrfs/qgroup.c +2
-1@@ -2250,7 +2250,7 @@ int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans, u64 srcid,int i;u64 *i_qgroups;struct btrfs_fs_info *fs_info = trans->fs_info; -struct btrfs_root *quota_root = fs_info->quota_root; +struct btrfs_root *quota_root;struct btrfs_qgroup *srcgroup;struct btrfs_qgroup *dstgroup;u32 level_size =0 ;@@ -2260,6 +2260,7 @@ int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans, u64 srcid,if (!test_bit (BTRFS_FS_QUOTA_ENABLED , &fs_info->flags)) goto out; + quota_root = fs_info->quota_root;if (!quota_root) { ret = -EINVAL ; goto out;
Quoted 39 lines · On Mon, Nov 19, 2018 at 04:20:34PM +0000, [email protected] wrote:
From: Filipe Manana <[email protected]>
We have a race between enabling quotas end subvolume creation that cause subvolume creation to fail with
-EINVAL, and the following diagram shows how it happens:CPU 0 CPU 1
btrfs_ioctl()
btrfs_ioctl_quota_ctl()
btrfs_quota_enable()mutex_lock(fs_info->qgroup_ioctl_lock) btrfs_ioctl() create_subvol() btrfs_qgroup_inherit() -> save fs_info->quota_root into quota_root -> stores a NULL value -> tries to lock the mutex qgroup_ioctl_lock -> blocks waiting for the task at CPU0-> sets BTRFS_FS_QUOTA_ENABLED in fs_info -> sets quota_root in fs_info->quota_root (non-NULL value) mutex_unlock(fs_info->qgroup_ioctl_lock) -> checks quota enabled flag is set -> returns -EINVAL because fs_info->quota_root was NULL before it acquired the mutex qgroup_ioctl_lock -> ioctl returns -EINVAL
Quoted 7 lines
Returning
-EINVALto user space will be confusing if all the arguments passed to the subvolume creation ioctl were valid.Fix it by grabbing the value from
fs_info->quota_rootafter acquiring the mutex.Signed-off-by Filipe Manana <[email protected]>
Added to misc-next, thanks.