The threaded IRQ function in this driver is reading the flag twice: once to lock a mutex and once to unlock it. Even though the code setting the flag is designed to prevent it, there are subtle cases where the flag could be true at the mutex_lock stage and false at the mutex_unlock stage. This results in the mutex not being unlocked, resulting in a deadlock.
Fix it by making the opt3001_irq() code generally more robust, reading the flag into a variable and using the variable value at both stages.
Fixes: 94a9b7b1809f ("iio: light: add support for TI's opt3001 light sensor") Cc: stable@vger.kernel.org Signed-off-by: Luca Ceresoli luca.ceresoli@bootlin.com --- drivers/iio/light/opt3001.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/iio/light/opt3001.c b/drivers/iio/light/opt3001.c index 65b295877b41588d40234ca7681bfee291e937c2..393a3d2fbe1d7320a243d3b6720e98b90f17baca 100644 --- a/drivers/iio/light/opt3001.c +++ b/drivers/iio/light/opt3001.c @@ -788,8 +788,9 @@ static irqreturn_t opt3001_irq(int irq, void *_iio) int ret; bool wake_result_ready_queue = false; enum iio_chan_type chan_type = opt->chip_info->chan_type; + bool ok_to_ignore_lock = opt->ok_to_ignore_lock;
- if (!opt->ok_to_ignore_lock) + if (!ok_to_ignore_lock) mutex_lock(&opt->lock);
ret = i2c_smbus_read_word_swapped(opt->client, OPT3001_CONFIGURATION); @@ -826,7 +827,7 @@ static irqreturn_t opt3001_irq(int irq, void *_iio) }
out: - if (!opt->ok_to_ignore_lock) + if (!ok_to_ignore_lock) mutex_unlock(&opt->lock);
if (wake_result_ready_queue)
--- base-commit: 250a4b882cf37d9719874253f055aad211f2c317 change-id: 20250321-opt3001-irq-fix-f7eecd4e2e9c
Best regards,
On Fri, 21 Mar 2025 19:10:00 +0100 Luca Ceresoli luca.ceresoli@bootlin.com wrote:
The threaded IRQ function in this driver is reading the flag twice: once to lock a mutex and once to unlock it. Even though the code setting the flag is designed to prevent it, there are subtle cases where the flag could be true at the mutex_lock stage and false at the mutex_unlock stage. This results in the mutex not being unlocked, resulting in a deadlock.
From a quick look I think that only occurs if the triggering write reports an error but the device accepts it anyway (maybe a bus error on the ack going back to the host).
I'm not a fan of this locking dance, but I'm also not keen on touching it too much without test hardware. Let's go with your fix which looks safe to me.
Applied.
Fix it by making the opt3001_irq() code generally more robust, reading the flag into a variable and using the variable value at both stages.
Fixes: 94a9b7b1809f ("iio: light: add support for TI's opt3001 light sensor") Cc: stable@vger.kernel.org Signed-off-by: Luca Ceresoli luca.ceresoli@bootlin.com
drivers/iio/light/opt3001.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/iio/light/opt3001.c b/drivers/iio/light/opt3001.c index 65b295877b41588d40234ca7681bfee291e937c2..393a3d2fbe1d7320a243d3b6720e98b90f17baca 100644 --- a/drivers/iio/light/opt3001.c +++ b/drivers/iio/light/opt3001.c @@ -788,8 +788,9 @@ static irqreturn_t opt3001_irq(int irq, void *_iio) int ret; bool wake_result_ready_queue = false; enum iio_chan_type chan_type = opt->chip_info->chan_type;
- bool ok_to_ignore_lock = opt->ok_to_ignore_lock;
- if (!opt->ok_to_ignore_lock)
- if (!ok_to_ignore_lock) mutex_lock(&opt->lock);
ret = i2c_smbus_read_word_swapped(opt->client, OPT3001_CONFIGURATION); @@ -826,7 +827,7 @@ static irqreturn_t opt3001_irq(int irq, void *_iio) } out:
- if (!opt->ok_to_ignore_lock)
- if (!ok_to_ignore_lock) mutex_unlock(&opt->lock);
if (wake_result_ready_queue)
base-commit: 250a4b882cf37d9719874253f055aad211f2c317 change-id: 20250321-opt3001-irq-fix-f7eecd4e2e9c
Best regards,
linux-stable-mirror@lists.linaro.org
-
Jonathan Cameron -
Luca Ceresoli