Re: [PATCH 3/3] btrfs: fix race between mkfs and mount

[Anand Jain <anand.jain@oracle.com>]

On 06/20/2018 10:06 PM, David Sterba wrote:
> On Mon, Jun 04, 2018 at 11:00:30PM +0800, Anand Jain wrote:
>> In an instrumented testing it is possible that the mount and
>> a newer mkfs.btrfs thread on the same device can race and if the new
>> mkfs.btrfs wins it will free the older fs_devices, then the mount thread
>> will lead to oops.
>>
>> Thread1						Thread2
>> -------						-------
>> mkfs.btrfs -fq /dev/sdb
>> mount /dev/sdb /btrfs
>> |_btrfs_mount_root()
>>    |_btrfs_scan_one_device(... &fs_devices)
>>
>> 						mkfs.btrfs -fq /dev/sdb
>> 						|_btrfs_contol_ioctl()
>> 						  |_btrfs_scan_one_device(... &fs_devices)
>> 						    |_::
>> 						      |_btrfs_free_stale_devices()
>>
>>    |_btrfs_open_devices(fs_devices ..) <-- stale fs_devices.
>>
>> Fix this with a mutually exclusive flag BTRFS_VOL_FLAG_EXCL_OPS.
>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>> ---
>>   fs/btrfs/super.c   |  6 ++++++
>>   fs/btrfs/volumes.c | 10 +++++++++-
>>   fs/btrfs/volumes.h |  1 +
>>   3 files changed, 16 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
>> index f0c13defc9eb..b60e7cbe39f5 100644
>> --- a/fs/btrfs/super.c
>> +++ b/fs/btrfs/super.c
>> @@ -1565,7 +1565,13 @@ static struct dentry *btrfs_mount_root(struct file_system_type *fs_type,
>>   		goto error_fs_info;
>>   	}
>>   
>> +	if (test_and_set_bit(BTRFS_VOLUME_STATE_EXCL_OPS, &fs_devices->volume_state)) {
>> +		error = -EBUSY;
> 
> We'd need to wait until the bit is not set instead of BUSY, as the
> parallel scan is not really a reason to fail the whole mount.

> I'll post the patch series to address this problem today, it utilizes
> the uuid_mutex in a similar way you try to do with the new bit, but it
> will not lead to EBUSY.

  Ok. Shall review.

Thanks, Anand

> --
> 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
>