Re: [PATCH v2 3/7] btrfs: handle allocation failure in strdup

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

On 13/11/19 6:27 PM, Johannes Thumshirn wrote:
> Gracefully handle allocation failures in btrfs_close_one_device()'s
> rcu_string_strdup() instead of crashing the machine.
> 
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
> ---
>   fs/btrfs/volumes.c | 22 ++++++++++++++--------
>   1 file changed, 14 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 0a2a73907563..e5864ca3bb3b 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -1064,7 +1064,7 @@ static void btrfs_close_bdev(struct btrfs_device *device)
>   static int btrfs_close_one_device(struct btrfs_device *device)
>   {
>   	struct btrfs_fs_devices *fs_devices = device->fs_devices;
> -	struct btrfs_device *new_device;
> +	struct btrfs_device *new_device = NULL;
>   	struct rcu_string *name;
>   
>   	new_device = btrfs_alloc_device(NULL, &device->devid,
> @@ -1072,6 +1072,15 @@ static int btrfs_close_one_device(struct btrfs_device *device)
>   	if (IS_ERR(new_device))
>   		goto err_close_device;
>   
> +	/* Safe because we are under uuid_mutex */
> +	if (device->name) {
> +		name = rcu_string_strdup(device->name->str, GFP_NOFS);
> +		if (!name)
> +			goto err_free_device;
> +
> +		rcu_assign_pointer(new_device->name, name);
> +	}
> +


  Any idea why do we need to strdup() at all to close a device?

Thanks, Anand


>   	if (test_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state) &&
>   	    device->devid != BTRFS_DEV_REPLACE_DEVID) {
>   		list_del_init(&device->dev_alloc_list);
> @@ -1085,13 +1094,6 @@ static int btrfs_close_one_device(struct btrfs_device *device)
>   	if (device->bdev)
>   		fs_devices->open_devices--;
>   
> -	/* Safe because we are under uuid_mutex */
> -	if (device->name) {
> -		name = rcu_string_strdup(device->name->str, GFP_NOFS);
> -		BUG_ON(!name); /* -ENOMEM */
> -		rcu_assign_pointer(new_device->name, name);
> -	}
> -
>   	list_replace_rcu(&device->dev_list, &new_device->dev_list);
>   	new_device->fs_devices = device->fs_devices;
>   
> @@ -1100,6 +1102,10 @@ static int btrfs_close_one_device(struct btrfs_device *device)
>   
>   	return 0;
>   
> +err_free_device:
> +	if (new_device)
> +		btrfs_free_device(new_device);
> +
>   err_close_device:
>   	btrfs_close_bdev(device);
>   	if (device->bdev) {
>