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@xxxxxxx>
> > ---
> > 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.
