[PATCH] UBI: block: Must use a mutex when using idr_alloc/idr_remove

[Ezequiel Garcia]

On 14 January 2018 at 11:39, Boris Brezillon
<boris.brezillon at free-electrons.com> wrote:
> +Ezequiel
>
> Hi Bradley,
>
> On Tue,  2 Jan 2018 21:26:09 -0500
> Bradley Bolen <bradleybolen at gmail.com> wrote:
>
>> This fixes a race condition where running ubiblock on multiple volumes
>> simultaneously produces the following splat.
>>
>> kernel BUG at kernel-source/fs/sysfs/group.c:113!
>> Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM
>> [<c01e4160>] (internal_create_group) from [<c01e43fc>]
>> (sysfs_create_group+0x20/0x24)
>> [<c01e43fc>] (sysfs_create_group) from [<c00e4800>]
>> (blk_trace_init_sysfs+0x18/0x20)
>> [<c00e4800>] (blk_trace_init_sysfs) from [<c02bc734>]
>> (blk_register_queue+0x6c/0x154)
>> [<c02bc734>] (blk_register_queue) from [<c02cd610>]
>> (device_add_disk+0x2c8/0x450)
>> [<c02cd610>] (device_add_disk) from [<c0430fac>]
>> (ubiblock_create+0x254/0x2e4)
>> [<c0430fac>] (ubiblock_create) from [<c0421a3c>]
>> (vol_cdev_ioctl+0x3e0/0x564)
>> [<c0421a3c>] (vol_cdev_ioctl) from [<c018a55c>] (vfs_ioctl+0x30/0x44)
>> [<c018a55c>] (vfs_ioctl) from [<c018ad2c>] (do_vfs_ioctl+0x694/0x798)
>> [<c018ad2c>] (do_vfs_ioctl) from [<c018ae74>] (SyS_ioctl+0x44/0x6c)
>> [<c018ae74>] (SyS_ioctl) from [<c0010720>] (ret_fast_syscall+0x0/0x34)
>>

I'm a little confused, how does this stack trace relates to idr?

>
> Missing Fixes and Cc-stable tags.
>
>> Signed-off-by: Bradley Bolen <bradleybolen at gmail.com>
>
>
> Reviewed-by: Boris Brezillon <boris.brezillon at free-electrons.com>
>
> BTW, just had a quick look at the code, and I think the locking can be
> simplified a bit by keeping the devices_lock for the whole
> ubiblock_create() operation instead of acquiring/releasing it several
> times in the function (see [1]). It makes sense to do fine grained
> locking when operations protected by the lock are time sensitive, but I
> don't think this is the case here.
>

Agreed. Something like this perhaps http://ix.io/E8G ?
Notice that ubiblock_remove_all seemed to require locking as well.
-- 
Ezequiel García, VanguardiaSur
www.vanguardiasur.com.ar