Patch "ubi: block: Fix locking for idr_alloc/idr_remove" has been added to the 4.4-stable tree
This is a note to let you know that I've just added the patch titled
ubi: block: Fix locking for idr_alloc/idr_remove
to the 4.4-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
ubi-block-fix-locking-for-idr_alloc-idr_remove.patch
and it can be found in the queue-4.4 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.
Quoted 1 line
From
7f29ae9f977bcdc3654e68bc36d170223c52fd48Mon Sep 17 00:00:00 2001
From: Bradley Bolen <[email protected]>
Date: Thu, 18 Jan 2018 08:55:20 -0500
Subject: ubi: block: Fix locking for idr_alloc/idr_remove
From: Bradley Bolen <[email protected]>
commit 7f29ae9f977bcdc3654e68bc36d170223c52fd48 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.
--- drivers/mtd/ubi/block.c | 42 ++++++++++++++++++++++++++---------------- 1 file changed, 26 insertions(+), 16 deletions(-)
drivers/mtd/ubi/block.c +26 -16
@@ -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;
@@ -354,8 +356,6 @@ static struct blk_mq_ops ubiblock_mq_ops .map_queue = blk_mq_map_queue, }; -static DEFINE_IDR (ubiblock_minor_idr); -int ubiblock_create (struct ubi_volume_info *vi) {struct ubiblock *dev;
@@ -368,14 +368,15 @@ int ubiblock_create(struct ubi_volume_in/* 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);
@@ -440,14 +441,13 @@ int ubiblock_create(struct ubi_volume_in 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:
@@ -460,6 +460,8 @@ out_put_disk:put_disk (dev->gd); out_free_dev:kfree (dev); +out_unlock: +mutex_unlock (&devices_mutex);return ret; }
@@ -481,30 +483,36 @@ static void ubiblock_cleanup(struct ubibint 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)
@@ -633,6 +641,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);
@@ -641,6 +650,7 @@ static void ubiblock_remove_all(void)ubiblock_cleanup (dev);kfree (dev); } +mutex_unlock (&devices_mutex); }int __init ubiblock_init (void )
Patches currently in stable-queue which might be from [email protected] are
queue-4.4/ubi-block-fix-locking-for-idr_alloc-idr_remove.patch