• Fabrice Gasnier's avatar
    iio: core: fix a possible circular locking dependency · 7f75591f
    Fabrice Gasnier authored
    This fixes a possible circular locking dependency detected warning seen
    with:
    - CONFIG_PROVE_LOCKING=y
    - consumer/provider IIO devices (ex: "voltage-divider" consumer of "adc")
    
    When using the IIO consumer interface, e.g. iio_channel_get(), the consumer
    device will likely call iio_read_channel_raw() or similar that rely on
    'info_exist_lock' mutex.
    
    typically:
    ...
    	mutex_lock(&chan->indio_dev->info_exist_lock);
    	if (chan->indio_dev->info == NULL) {
    		ret = -ENODEV;
    		goto err_unlock;
    	}
    	ret = do_some_ops()
    err_unlock:
    	mutex_unlock(&chan->indio_dev->info_exist_lock);
    	return ret;
    ...
    
    Same mutex is also hold in iio_device_unregister().
    
    The following deadlock warning happens when:
    - the consumer device has called an API like iio_read_channel_raw()
      at least once.
    - the consumer driver is unregistered, removed (unbind from sysfs)
    
    ======================================================
    WARNING: possible circular locking dependency detected
    4.19.24 #577 Not tainted
    ------------------------------------------------------
    sh/372 is trying to acquire lock:
    (kn->count#30){++++}, at: kernfs_remove_by_name_ns+0x3c/0x84
    
    but task is already holding lock:
    (&dev->info_exist_lock){+.+.}, at: iio_device_unregister+0x18/0x60
    
    which lock already depends on the new lock.
    
    the existing dependency chain (in reverse order) is:
    
    -> #1 (&dev->info_exist_lock){+.+.}:
           __mutex_lock+0x70/0xa3c
           mutex_lock_nested+0x1c/0x24
           iio_read_channel_raw+0x1c/0x60
           iio_read_channel_info+0xa8/0xb0
           dev_attr_show+0x1c/0x48
           sysfs_kf_seq_show+0x84/0xec
           seq_read+0x154/0x528
           __vfs_read+0x2c/0x15c
           vfs_read+0x8c/0x110
           ksys_read+0x4c/0xac
           ret_fast_syscall+0x0/0x28
           0xbedefb60
    
    -> #0 (kn->count#30){++++}:
           lock_acquire+0xd8/0x268
           __kernfs_remove+0x288/0x374
           kernfs_remove_by_name_ns+0x3c/0x84
           remove_files+0x34/0x78
           sysfs_remove_group+0x40/0x9c
           sysfs_remove_groups+0x24/0x34
           device_remove_attrs+0x38/0x64
           device_del+0x11c/0x360
           cdev_device_del+0x14/0x2c
           iio_device_unregister+0x24/0x60
           release_nodes+0x1bc/0x200
           device_release_driver_internal+0x1a0/0x230
           unbind_store+0x80/0x130
           kernfs_fop_write+0x100/0x1e4
           __vfs_write+0x2c/0x160
           vfs_write+0xa4/0x17c
           ksys_write+0x4c/0xac
           ret_fast_syscall+0x0/0x28
           0xbe906840
    
    other info that might help us debug this:
    
     Possible unsafe locking scenario:
    
           CPU0                    CPU1
           ----                    ----
      lock(&dev->info_exist_lock);
                                   lock(kn->count#30);
                                   lock(&dev->info_exist_lock);
      lock(kn->count#30);
    
     *** DEADLOCK ***
    ...
    
    cdev_device_del() can be called without holding the lock. It should be safe
    as info_exist_lock prevents kernelspace consumers to use the exported
    routines during/after provider removal. cdev_device_del() is for userspace.
    
    Help to reproduce:
    See example: Documentation/devicetree/bindings/iio/afe/voltage-divider.txt
    sysv {
    	compatible = "voltage-divider";
    	io-channels = <&adc 0>;
    	output-ohms = <22>;
    	full-ohms = <222>;
    };
    
    First, go to iio:deviceX for the "voltage-divider", do one read:
    $ cd /sys/bus/iio/devices/iio:deviceX
    $ cat in_voltage0_raw
    
    Then, unbind the consumer driver. It triggers above deadlock warning.
    $ cd /sys/bus/platform/drivers/iio-rescale/
    $ echo sysv > unbind
    
    Note I don't actually expect stable will pick this up all the
    way back into IIO being in staging, but if's probably valid that
    far back.
    Signed-off-by: default avatarFabrice Gasnier <fabrice.gasnier@st.com>
    Fixes: ac917a81 ("staging:iio:core set the iio_dev.info pointer to null on unregister")
    Cc: <Stable@vger.kernel.org>
    Signed-off-by: default avatarJonathan Cameron <Jonathan.Cameron@huawei.com>
    7f75591f
industrialio-core.c 45.7 KB