Commit 9910159f authored by Rasmus Villemoes's avatar Rasmus Villemoes Committed by Jonathan Cameron
Browse files

iio: core: add separate lockdep class for info_exist_lock 



When one iio device is a consumer of another, it is possible that
the ->info_exist_lock of both ends up being taken when reading the
value of the consumer device.

Since they currently belong to the same lockdep class (being
initialized in a single location with mutex_init()), that results in a
lockdep warning

         CPU0
         ----
    lock(&iio_dev_opaque->info_exist_lock);
    lock(&iio_dev_opaque->info_exist_lock);

   *** DEADLOCK ***

   May be due to missing lock nesting notation

  4 locks held by sensors/414:
   #0: c31fd6dc (&p->lock){+.+.}-{3:3}, at: seq_read_iter+0x44/0x4e4
   #1: c4f5a1c4 (&of->mutex){+.+.}-{3:3}, at: kernfs_seq_start+0x1c/0xac
   #2: c2827548 (kn->active#34){.+.+}-{0:0}, at: kernfs_seq_start+0x30/0xac
   #3: c1dd2b68 (&iio_dev_opaque->info_exist_lock){+.+.}-{3:3}, at: iio_read_channel_processed_scale+0x24/0xd8

  stack backtrace:
  CPU: 0 UID: 0 PID: 414 Comm: sensors Not tainted 6.17.11 #5 NONE
  Hardware name: Generic AM33XX (Flattened Device Tree)
  Call trace:
   unwind_backtrace from show_stack+0x10/0x14
   show_stack from dump_stack_lvl+0x44/0x60
   dump_stack_lvl from print_deadlock_bug+0x2b8/0x334
   print_deadlock_bug from __lock_acquire+0x13a4/0x2ab0
   __lock_acquire from lock_acquire+0xd0/0x2c0
   lock_acquire from __mutex_lock+0xa0/0xe8c
   __mutex_lock from mutex_lock_nested+0x1c/0x24
   mutex_lock_nested from iio_read_channel_raw+0x20/0x6c
   iio_read_channel_raw from rescale_read_raw+0x128/0x1c4
   rescale_read_raw from iio_channel_read+0xe4/0xf4
   iio_channel_read from iio_read_channel_processed_scale+0x6c/0xd8
   iio_read_channel_processed_scale from iio_hwmon_read_val+0x68/0xbc
   iio_hwmon_read_val from dev_attr_show+0x18/0x48
   dev_attr_show from sysfs_kf_seq_show+0x80/0x110
   sysfs_kf_seq_show from seq_read_iter+0xdc/0x4e4
   seq_read_iter from vfs_read+0x238/0x2e4
   vfs_read from ksys_read+0x6c/0xec
   ksys_read from ret_fast_syscall+0x0/0x1c

Just as the mlock_key already has its own lockdep class, add a
lock_class_key for the info_exist mutex.

Note that this has in theory been a problem since before IIO first
left staging, but it only occurs when a chain of consumers is in use
and that is not often done.

Fixes: ac917a81 ("staging:iio:core set the iio_dev.info pointer to null on unregister under lock.")
Signed-off-by: default avatarRasmus Villemoes <ravi@prevas.dk>
Reviewed-by: default avatarPeter Rosin <peda@axentia.se>
Cc: <stable@vger.kernel.org>
Signed-off-by: default avatarJonathan Cameron <Jonathan.Cameron@huawei.com>
parent 81d5a536

drivers/iio/industrialio-core.c

+3 −1
Original line number Diff line number Diff line
@@ -1657,6 +1657,7 @@ static void iio_dev_release(struct device *device) 
	mutex_destroy(&iio_dev_opaque->info_exist_lock);

	mutex_destroy(&iio_dev_opaque->mlock);



	lockdep_unregister_key(&iio_dev_opaque->info_exist_key);

	lockdep_unregister_key(&iio_dev_opaque->mlock_key);



	ida_free(&iio_ida, iio_dev_opaque->id);
@@ -1717,9 +1718,10 @@ struct iio_dev *iio_device_alloc(struct device *parent, int sizeof_priv) 
	INIT_LIST_HEAD(&iio_dev_opaque->ioctl_handlers);



	lockdep_register_key(&iio_dev_opaque->mlock_key);

	lockdep_register_key(&iio_dev_opaque->info_exist_key);



	mutex_init_with_key(&iio_dev_opaque->mlock, &iio_dev_opaque->mlock_key);

	mutex_init(&iio_dev_opaque->info_exist_lock);

	mutex_init_with_key(&iio_dev_opaque->info_exist_lock, &iio_dev_opaque->info_exist_key);



	indio_dev->dev.parent = parent;

	indio_dev->dev.type = &iio_device_type;

include/linux/iio/iio-opaque.h

+2 −0
Original line number Diff line number Diff line
@@ -14,6 +14,7 @@ 
 * @mlock:			lock used to prevent simultaneous device state changes

 * @mlock_key:			lockdep class for iio_dev lock

 * @info_exist_lock:		lock to prevent use during removal

 * @info_exist_key:		lockdep class for info_exist lock

 * @trig_readonly:		mark the current trigger immutable

 * @event_interface:		event chrdevs associated with interrupt lines

 * @attached_buffers:		array of buffers statically attached by the driver
@@ -47,6 +48,7 @@ struct iio_dev_opaque { 
	struct mutex			mlock;

	struct lock_class_key		mlock_key;

	struct mutex			info_exist_lock;

	struct lock_class_key		info_exist_key;

	bool				trig_readonly;

	struct iio_event_interface	*event_interface;

	struct iio_buffer		**attached_buffers;