linux-kernel - Re: [RFC PATCH 9/9] iio: imu: inv_mpu6050: Fix deadlock between i2c adapter lock and mpu lock

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 1 Mar 2016 21:50:52 +0100
From:	Wolfram Sang <wsa@...-dreams.de>
To:	Daniel Baluta <daniel.baluta@...el.com>
Cc:	jic23@...nel.org, knaack.h@....de, lars@...afoo.de,
	pmeerw@...erw.net, linux-iio@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-i2c@...r.kernel.org,
	lucas.demarchi@...el.com, srinivas.pandruvada@...ux.intel.com,
	ggao@...ensense.com, adi.reus@...il.com, cmo@...exis.com,
	mwelling@...e.org
Subject: Re: [RFC PATCH 9/9] iio: imu: inv_mpu6050: Fix deadlock between i2c
 adapter lock and mpu lock

On Thu, Feb 18, 2016 at 05:53:14PM +0200, Daniel Baluta wrote:
> From: Adriana Reus <adriana.reus@...el.com>
> 
> This deadlock occurs if the accel/gyro and the sensor on the auxiliary
> I2C (in my setup it's an ak8975) are working at the same time.
> 
> Scenario:
> 
>       T1					T2
>      ====				       ====
> inv_mpu6050_read_fifo                  aux sensor op (eg. ak8975_read_raw)
>         |                                     |
> mutex_lock(&indio_dev->mlock)           i2c_transfer
>         |                                     |
> i2c transaction                         i2c adapter lock
>         |                                     |
> i2c adapter lock                        i2c_mux_master_xfer
>                                               |
>                                         inv_mpu6050_select_bypass
>                                               |
>                                         mutex_lock(&indio_dev->mlock)
> 
> When we operate on an mpu sensor the order of locking is mpu lock
> followed by the i2c adapter lock. However, when we operate the auxiliary
> sensor the order of locking is the other way around.
> 
> In order to avoid this enable the bypass mux bit once in the beginning
> and remove the select/deselect_bypass functions.
> 
> This patch moves the bypass enabling in a separate function
> that is called once at probe and removes the functionality from
> inv_mpu_select/deselect_bypass.
> 
> Another advantage of this approach is that power-wise the mpu chip isn't
> powered up at each auxiliary sensor i2c transaction so if only the
> compass is used this would be more power efficient.

I think the comments (power must be enabled on select) rendered this
solution not acceptable. What about using mutex_trylock in
inv_mpu6050_select_bypass() and returning -EAGAIN if the lock is already
held?


Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)