[PATCH 1/2] btrfs: device list could grow infinite

[PATCH 1/2] btrfs: device list could grow infinite

[Anand Jain]

Reproducer 1:
modprobe -r btrfs; modprobe btrfs

while true ; do mkfs.btrfs -f /dev/sde > /dev/null 2>&1; done
CTLR-C
we keep stale FSIDs.

btrfs-devlist | egrep "/dev/sde" | wc -l
41

Reproducer 2:
mkfs.btrfs -d raid1 -m raid1 /dev/sdd /dev/sdf
mkfs.btrfs -f /dev/sdf
btrfs dev ready /dev/sdd
echo $?
0 <-- wrong

fix this at device_list_add() to check and delete if the newly added disk path
is already in the device list

Signed-off-by: Anand Jain <Anand.Jain@oracle.com>
---
 fs/btrfs/volumes.c | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index b107ad8..91ba2cf 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -598,6 +598,40 @@ static void pending_bios_fn(struct btrfs_work *work)
 	run_scheduled_bios(device);
 }
 
+static void find_delete_duplicate_dev(struct btrfs_device *new_dev)
+{
+	struct btrfs_fs_devices *fs_devs;
+	struct btrfs_fs_devices *cur_fs_devs;
+	struct btrfs_device	*dev;
+
+	list_for_each_entry(fs_devs, &fs_uuids, list) {
+
+		if (new_dev->fs_devices == fs_devs || fs_devs->opened)
+			continue;
+
+		cur_fs_devs = fs_devs;
+		list_for_each_entry(dev, &cur_fs_devs->devices, dev_list) {
+
+			if (dev->name && !strcmp(new_dev->name->str,
+							dev->name->str)) {
+				if (cur_fs_devs->num_devices == 1) {
+					free_fs_devices(cur_fs_devs);
+				} else {
+					mutex_lock(&cur_fs_devs->device_list_mutex);
+					list_del_rcu(&dev->dev_list);
+					cur_fs_devs->num_devices--;
+					if (dev->missing)
+						cur_fs_devs->missing_devices--;
+					mutex_unlock(&cur_fs_devs->device_list_mutex);
+					rcu_string_free(dev->name);
+					kfree(dev);
+				}
+				return;
+			}
+		}
+	}
+}
+
 static noinline int device_list_add(const char *path,
 			   struct btrfs_super_block *disk_super,
 			   u64 devid, struct btrfs_fs_devices **fs_devices_ret)
@@ -708,6 +742,13 @@ static noinline int device_list_add(const char *path,
 		fs_devices->latest_devid = devid;
 		fs_devices->latest_trans = found_transid;
 	}
+
+	/*
+	 * if new fs is created on the same path we need to clean
+	 * the old one.
+	 */
+	find_delete_duplicate_dev(device);
+
 	*fs_devices_ret = fs_devices;
 	return 0;
 }
-- 
2.0.0.257.g75cc6c6

[PATCH 2/2] btrfs: we are not yet ready if device is missing

[Anand Jain]

reproducer:

mkfs.btrfs -f -draid1 -mraid1 /dev/sdf /dev/sdd
modprobe -r btrfs && modprobe btrfs
mount -o degraded /dev/sdd /btrfs  <-- calls add_missing_dev() to add missing btrfs_device
umount /btrfs
btrfs dev ready /dev/sdd
echo $?
0
mount /dev/sdd /btrfs
mount: wrong fs type, bad option, bad superblock on /dev/sdd,
       missing codepage or helper program, or other error

fix this by checking if the device name is present in the BTRFS_IOC_DEVICES_READY
ioctl

Signed-off-by: Anand Jain <Anand.Jain@oracle.com>
---
 fs/btrfs/super.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index b9efe58..74ef9bf 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -1857,6 +1857,7 @@ static long btrfs_control_ioctl(struct file *file, unsigned int cmd,
 	struct btrfs_fs_devices *fs_devices;
 	int ret = -ENOTTY;
 	void __user *argp = (void __user *)arg;
+	struct btrfs_device *dev;
 
 	if (!capable(CAP_SYS_ADMIN))
 		return -EPERM;
@@ -1882,6 +1883,14 @@ static long btrfs_control_ioctl(struct file *file, unsigned int cmd,
 		kfree(vol);
 		if (ret)
 			break;
+
+		list_for_each_entry(dev, &fs_devices->devices, dev_list) {
+			if (dev->name)
+				continue;
+
+			return !0;
+		}
+
 		ret = !(fs_devices->num_devices == fs_devices->total_devices);
 		break;
 	case BTRFS_IOC_GET_FSLIST:
-- 
2.0.0.257.g75cc6c6

Re: [PATCH 1/2] btrfs: device list could grow infinite

[Anand Jain]

NACK for this. I have null pointer deference with this patch.
extremely sorry. I am investigating..

Anand


On 24/06/2014 20:51, Anand Jain wrote:
> Reproducer 1:
> modprobe -r btrfs; modprobe btrfs
>
> while true ; do mkfs.btrfs -f /dev/sde > /dev/null 2>&1; done
> CTLR-C
> we keep stale FSIDs.
>
> btrfs-devlist | egrep "/dev/sde" | wc -l
> 41
>
> Reproducer 2:
> mkfs.btrfs -d raid1 -m raid1 /dev/sdd /dev/sdf
> mkfs.btrfs -f /dev/sdf
> btrfs dev ready /dev/sdd
> echo $?
> 0 <-- wrong
>
> fix this at device_list_add() to check and delete if the newly added disk path
> is already in the device list
>
> Signed-off-by: Anand Jain <Anand.Jain@oracle.com>
> ---
>   fs/btrfs/volumes.c | 41 +++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 41 insertions(+)
>
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index b107ad8..91ba2cf 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -598,6 +598,40 @@ static void pending_bios_fn(struct btrfs_work *work)
>   	run_scheduled_bios(device);
>   }
>
> +static void find_delete_duplicate_dev(struct btrfs_device *new_dev)
> +{
> +	struct btrfs_fs_devices *fs_devs;
> +	struct btrfs_fs_devices *cur_fs_devs;
> +	struct btrfs_device	*dev;
> +
> +	list_for_each_entry(fs_devs, &fs_uuids, list) {
> +
> +		if (new_dev->fs_devices == fs_devs || fs_devs->opened)
> +			continue;
> +
> +		cur_fs_devs = fs_devs;
> +		list_for_each_entry(dev, &cur_fs_devs->devices, dev_list) {
> +
> +			if (dev->name && !strcmp(new_dev->name->str,
> +							dev->name->str)) {
> +				if (cur_fs_devs->num_devices == 1) {
> +					free_fs_devices(cur_fs_devs);
> +				} else {
> +					mutex_lock(&cur_fs_devs->device_list_mutex);
> +					list_del_rcu(&dev->dev_list);
> +					cur_fs_devs->num_devices--;
> +					if (dev->missing)
> +						cur_fs_devs->missing_devices--;
> +					mutex_unlock(&cur_fs_devs->device_list_mutex);
> +					rcu_string_free(dev->name);
> +					kfree(dev);
> +				}
> +				return;
> +			}
> +		}
> +	}
> +}
> +
>   static noinline int device_list_add(const char *path,
>   			   struct btrfs_super_block *disk_super,
>   			   u64 devid, struct btrfs_fs_devices **fs_devices_ret)
> @@ -708,6 +742,13 @@ static noinline int device_list_add(const char *path,
>   		fs_devices->latest_devid = devid;
>   		fs_devices->latest_trans = found_transid;
>   	}
> +
> +	/*
> +	 * if new fs is created on the same path we need to clean
> +	 * the old one.
> +	 */
> +	find_delete_duplicate_dev(device);
> +
>   	*fs_devices_ret = fs_devices;
>   	return 0;
>   }
>