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

[David Sterba <dsterba@suse.cz>]

On Thu, Nov 14, 2019 at 07:00:54PM +0800, Anand Jain wrote:
> 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?

It shouldn't be needed but that's how it got implemented since the
beginning in e4404d6e8da678d852. The device on close is duplicated, so
has to be the name.