dm: fix add_disk() NULL pointer due to race with free_dev()
Commit c4db59d31e
("fs: don't reassign dirty inodes to
default_backing_dev_info") exposed DM to a latent race in free_dev() vs
add_disk() in relation to management of the device's minor number.
Fix this by refactoring free_dev() to match cleanup order of the
alloc_dev() error path. Move cleanup of the gendisk, queue, and bdev
to _before_ the cleanup of the idr managed minor number.
Also, purely due to cleanup that fell out during the free_dev() audit:
- adjust dm_blk_close() to access the gendisk's private_data under
the _minor_lock spinlock.
- move __dm_destroy()'s dm_get_live_table() call out from under the
_minor_lock spinlock.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1202449
Reported-by: Zdenek Kabelac <zkabelac@redhat.com>
Reported-by: Jeff Moyer <jmoyer@redhat.com>
Signed-off-by: Mike Snitzer <snitzer@redhat.com>
This commit is contained in:
parent
e5db29806b
commit
63a4f065ec
1 changed files with 16 additions and 10 deletions
|
@ -433,7 +433,6 @@ static int dm_blk_open(struct block_device *bdev, fmode_t mode)
|
|||
|
||||
dm_get(md);
|
||||
atomic_inc(&md->open_count);
|
||||
|
||||
out:
|
||||
spin_unlock(&_minor_lock);
|
||||
|
||||
|
@ -442,16 +441,20 @@ static int dm_blk_open(struct block_device *bdev, fmode_t mode)
|
|||
|
||||
static void dm_blk_close(struct gendisk *disk, fmode_t mode)
|
||||
{
|
||||
struct mapped_device *md = disk->private_data;
|
||||
struct mapped_device *md;
|
||||
|
||||
spin_lock(&_minor_lock);
|
||||
|
||||
md = disk->private_data;
|
||||
if (WARN_ON(!md))
|
||||
goto out;
|
||||
|
||||
if (atomic_dec_and_test(&md->open_count) &&
|
||||
(test_bit(DMF_DEFERRED_REMOVE, &md->flags)))
|
||||
queue_work(deferred_remove_workqueue, &deferred_remove_work);
|
||||
|
||||
dm_put(md);
|
||||
|
||||
out:
|
||||
spin_unlock(&_minor_lock);
|
||||
}
|
||||
|
||||
|
@ -2241,7 +2244,6 @@ static void free_dev(struct mapped_device *md)
|
|||
int minor = MINOR(disk_devt(md->disk));
|
||||
|
||||
unlock_fs(md);
|
||||
bdput(md->bdev);
|
||||
destroy_workqueue(md->wq);
|
||||
|
||||
if (md->kworker_task)
|
||||
|
@ -2252,19 +2254,22 @@ static void free_dev(struct mapped_device *md)
|
|||
mempool_destroy(md->rq_pool);
|
||||
if (md->bs)
|
||||
bioset_free(md->bs);
|
||||
blk_integrity_unregister(md->disk);
|
||||
del_gendisk(md->disk);
|
||||
|
||||
cleanup_srcu_struct(&md->io_barrier);
|
||||
free_table_devices(&md->table_devices);
|
||||
free_minor(minor);
|
||||
dm_stats_cleanup(&md->stats);
|
||||
|
||||
spin_lock(&_minor_lock);
|
||||
md->disk->private_data = NULL;
|
||||
spin_unlock(&_minor_lock);
|
||||
|
||||
if (blk_get_integrity(md->disk))
|
||||
blk_integrity_unregister(md->disk);
|
||||
del_gendisk(md->disk);
|
||||
put_disk(md->disk);
|
||||
blk_cleanup_queue(md->queue);
|
||||
dm_stats_cleanup(&md->stats);
|
||||
bdput(md->bdev);
|
||||
free_minor(minor);
|
||||
|
||||
module_put(THIS_MODULE);
|
||||
kfree(md);
|
||||
}
|
||||
|
@ -2642,8 +2647,9 @@ static void __dm_destroy(struct mapped_device *md, bool wait)
|
|||
|
||||
might_sleep();
|
||||
|
||||
spin_lock(&_minor_lock);
|
||||
map = dm_get_live_table(md, &srcu_idx);
|
||||
|
||||
spin_lock(&_minor_lock);
|
||||
idr_replace(&_minor_idr, MINOR_ALLOCED, MINOR(disk_devt(dm_disk(md))));
|
||||
set_bit(DMF_FREEING, &md->flags);
|
||||
spin_unlock(&_minor_lock);
|
||||
|
|
Loading…
Reference in a new issue