mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git
synced 2024-09-06 02:50:43 +00:00
ubi: block: Fix locking for idr_alloc/idr_remove
commit7f29ae9f97
upstream. This fixes a race with idr_alloc where gd->first_minor can be set to the same value for two simultaneous calls to ubiblock_create. Each instance calls device_add_disk with the same first_minor. device_add_disk calls bdi_register_owner which generates several warnings. WARNING: CPU: 1 PID: 179 at kernel-source/fs/sysfs/dir.c:31 sysfs_warn_dup+0x68/0x88 sysfs: cannot create duplicate filename '/devices/virtual/bdi/252:2' WARNING: CPU: 1 PID: 179 at kernel-source/lib/kobject.c:240 kobject_add_internal+0x1ec/0x2f8 kobject_add_internal failed for 252:2 with -EEXIST, don't try to register things with the same name in the same directory WARNING: CPU: 1 PID: 179 at kernel-source/fs/sysfs/dir.c:31 sysfs_warn_dup+0x68/0x88 sysfs: cannot create duplicate filename '/dev/block/252:2' However, device_add_disk does not error out when bdi_register_owner returns an error. Control continues until reaching blk_register_queue. It then BUGs. kernel BUG at kernel-source/fs/sysfs/group.c:113! [<c01e26cc>] (internal_create_group) from [<c01e2950>] (sysfs_create_group+0x20/0x24) [<c01e2950>] (sysfs_create_group) from [<c00e3d38>] (blk_trace_init_sysfs+0x18/0x20) [<c00e3d38>] (blk_trace_init_sysfs) from [<c02bdfbc>] (blk_register_queue+0xd8/0x154) [<c02bdfbc>] (blk_register_queue) from [<c02cec84>] (device_add_disk+0x194/0x44c) [<c02cec84>] (device_add_disk) from [<c0436ec8>] (ubiblock_create+0x284/0x2e0) [<c0436ec8>] (ubiblock_create) from [<c0427bb8>] (vol_cdev_ioctl+0x450/0x554) [<c0427bb8>] (vol_cdev_ioctl) from [<c0189110>] (vfs_ioctl+0x30/0x44) [<c0189110>] (vfs_ioctl) from [<c01892e0>] (do_vfs_ioctl+0xa0/0x790) [<c01892e0>] (do_vfs_ioctl) from [<c0189a14>] (SyS_ioctl+0x44/0x68) [<c0189a14>] (SyS_ioctl) from [<c0010640>] (ret_fast_syscall+0x0/0x34) Locking idr_alloc/idr_remove removes the race and keeps gd->first_minor unique. Fixes:2bf50d42f3
("UBI: block: Dynamically allocate minor numbers") Signed-off-by: Bradley Bolen <bradleybolen@gmail.com> Reviewed-by: Boris Brezillon <boris.brezillon@free-electrons.com> Signed-off-by: Richard Weinberger <richard@nod.at> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
parent
5793f39de7
commit
e9a35f8b73
1 changed files with 26 additions and 16 deletions
|
@ -99,6 +99,8 @@ struct ubiblock {
|
|||
|
||||
/* Linked list of all ubiblock instances */
|
||||
static LIST_HEAD(ubiblock_devices);
|
||||
static DEFINE_IDR(ubiblock_minor_idr);
|
||||
/* Protects ubiblock_devices and ubiblock_minor_idr */
|
||||
static DEFINE_MUTEX(devices_mutex);
|
||||
static int ubiblock_major;
|
||||
|
||||
|
@ -351,8 +353,6 @@ static const struct blk_mq_ops ubiblock_mq_ops = {
|
|||
.init_request = ubiblock_init_request,
|
||||
};
|
||||
|
||||
static DEFINE_IDR(ubiblock_minor_idr);
|
||||
|
||||
int ubiblock_create(struct ubi_volume_info *vi)
|
||||
{
|
||||
struct ubiblock *dev;
|
||||
|
@ -365,14 +365,15 @@ int ubiblock_create(struct ubi_volume_info *vi)
|
|||
/* Check that the volume isn't already handled */
|
||||
mutex_lock(&devices_mutex);
|
||||
if (find_dev_nolock(vi->ubi_num, vi->vol_id)) {
|
||||
mutex_unlock(&devices_mutex);
|
||||
return -EEXIST;
|
||||
ret = -EEXIST;
|
||||
goto out_unlock;
|
||||
}
|
||||
mutex_unlock(&devices_mutex);
|
||||
|
||||
dev = kzalloc(sizeof(struct ubiblock), GFP_KERNEL);
|
||||
if (!dev)
|
||||
return -ENOMEM;
|
||||
if (!dev) {
|
||||
ret = -ENOMEM;
|
||||
goto out_unlock;
|
||||
}
|
||||
|
||||
mutex_init(&dev->dev_mutex);
|
||||
|
||||
|
@ -437,14 +438,13 @@ int ubiblock_create(struct ubi_volume_info *vi)
|
|||
goto out_free_queue;
|
||||
}
|
||||
|
||||
mutex_lock(&devices_mutex);
|
||||
list_add_tail(&dev->list, &ubiblock_devices);
|
||||
mutex_unlock(&devices_mutex);
|
||||
|
||||
/* Must be the last step: anyone can call file ops from now on */
|
||||
add_disk(dev->gd);
|
||||
dev_info(disk_to_dev(dev->gd), "created from ubi%d:%d(%s)",
|
||||
dev->ubi_num, dev->vol_id, vi->name);
|
||||
mutex_unlock(&devices_mutex);
|
||||
return 0;
|
||||
|
||||
out_free_queue:
|
||||
|
@ -457,6 +457,8 @@ int ubiblock_create(struct ubi_volume_info *vi)
|
|||
put_disk(dev->gd);
|
||||
out_free_dev:
|
||||
kfree(dev);
|
||||
out_unlock:
|
||||
mutex_unlock(&devices_mutex);
|
||||
|
||||
return ret;
|
||||
}
|
||||
|
@ -478,30 +480,36 @@ static void ubiblock_cleanup(struct ubiblock *dev)
|
|||
int ubiblock_remove(struct ubi_volume_info *vi)
|
||||
{
|
||||
struct ubiblock *dev;
|
||||
int ret;
|
||||
|
||||
mutex_lock(&devices_mutex);
|
||||
dev = find_dev_nolock(vi->ubi_num, vi->vol_id);
|
||||
if (!dev) {
|
||||
mutex_unlock(&devices_mutex);
|
||||
return -ENODEV;
|
||||
ret = -ENODEV;
|
||||
goto out_unlock;
|
||||
}
|
||||
|
||||
/* Found a device, let's lock it so we can check if it's busy */
|
||||
mutex_lock(&dev->dev_mutex);
|
||||
if (dev->refcnt > 0) {
|
||||
mutex_unlock(&dev->dev_mutex);
|
||||
mutex_unlock(&devices_mutex);
|
||||
return -EBUSY;
|
||||
ret = -EBUSY;
|
||||
goto out_unlock_dev;
|
||||
}
|
||||
|
||||
/* Remove from device list */
|
||||
list_del(&dev->list);
|
||||
mutex_unlock(&devices_mutex);
|
||||
|
||||
ubiblock_cleanup(dev);
|
||||
mutex_unlock(&dev->dev_mutex);
|
||||
mutex_unlock(&devices_mutex);
|
||||
|
||||
kfree(dev);
|
||||
return 0;
|
||||
|
||||
out_unlock_dev:
|
||||
mutex_unlock(&dev->dev_mutex);
|
||||
out_unlock:
|
||||
mutex_unlock(&devices_mutex);
|
||||
return ret;
|
||||
}
|
||||
|
||||
static int ubiblock_resize(struct ubi_volume_info *vi)
|
||||
|
@ -630,6 +638,7 @@ static void ubiblock_remove_all(void)
|
|||
struct ubiblock *next;
|
||||
struct ubiblock *dev;
|
||||
|
||||
mutex_lock(&devices_mutex);
|
||||
list_for_each_entry_safe(dev, next, &ubiblock_devices, list) {
|
||||
/* The module is being forcefully removed */
|
||||
WARN_ON(dev->desc);
|
||||
|
@ -638,6 +647,7 @@ static void ubiblock_remove_all(void)
|
|||
ubiblock_cleanup(dev);
|
||||
kfree(dev);
|
||||
}
|
||||
mutex_unlock(&devices_mutex);
|
||||
}
|
||||
|
||||
int __init ubiblock_init(void)
|
||||
|
|
Loading…
Reference in a new issue