Skip to contentSkip to conversation

iio: adc: meson-saradc: unlock on error in meson_sar_adc_lock()

PATCH Applied Dan Carpenter 3 participants 4 messages Mar 8, 2018 – Mar 13, 2018

Subsystems (3)
Kernel janitors Arm/Amlogic Meson SoC support IIO
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/1 iio: adc: meson-saradc: unlock on error in meson_sar_adc_lock()

The meson_sar_adc_lock() function is not supposed to hold the "indio_dev->mlock" on the error path.

Fixes 3adbf3427330 ("iio: adc: add a driver for the SAR ADC found in Amlogic Meson SoCs")
Signed-off-by Dan Carpenter <[email protected]>
drivers/iio/adc/meson_saradc.c +3 -1
index 29fa7736d80c..ede955d9b2a4 100644
@@ -462,8 +462,10 @@ static int meson_sar_adc_lock(struct iio_dev *indio_dev)
 			regmap_read(priv->regmap, MESON_SAR_ADC_DELAY, &val);
 		} while (val & MESON_SAR_ADC_DELAY_BL30_BUSY && timeout--);
 
-		if (timeout < 0)
+		if (timeout < 0) {
+			mutex_unlock(&indio_dev->mlock);
 			return -ETIMEDOUT;
+		}
 	}
 
 	return 0;
maintainer Applied
#1 lore raw
Quoted 6 lines · On Thu, 8 Mar 2018 12:31:53 +0300 Dan Carpenter <[email protected]>…

The meson_sar_adc_lock() function is not supposed to hold the "indio_dev->mlock" on the error path.

Fixes 3adbf3427330 ("iio: adc: add a driver for the SAR ADC found in Amlogic Meson SoCs")
Signed-off-by Dan Carpenter <[email protected]>

Applied to the fixes-togreg branch of iio.git and marked for stable.

Thanks

Jonathan

Quoted 16 lines
drivers/iio/adc/meson_saradc.c +3 -1
index 29fa7736d80c..ede955d9b2a4 100644
@@ -462,8 +462,10 @@ static int meson_sar_adc_lock(struct iio_dev *indio_dev)
 			regmap_read(priv->regmap, MESON_SAR_ADC_DELAY, &val);
 		} while (val & MESON_SAR_ADC_DELAY_BL30_BUSY && timeout--);
 
-		if (timeout < 0)
+		if (timeout < 0) {
+			mutex_unlock(&indio_dev->mlock);
 			return -ETIMEDOUT;
+		}
 	}
 
 	return 0;
reviewer Acked-by on v1 1/1
#2 lore raw
In reply to Dan Carpenter

Hi Dan,

Quoted 5 lines · Dan Carpenter

The meson_sar_adc_lock() function is not supposed to hold the "indio_dev->mlock" on the error path.

Fixes 3adbf3427330 ("iio: adc: add a driver for the SAR ADC found in Amlogic Meson SoCs")
Signed-off-by Dan Carpenter <[email protected]>
Acked-by Martin Blumenstingl<[email protected]>

(even if I might be too late already since Jonathan already applied this patch)

thank you for finding and fixing this!
out of curiosity: did you find this with some tool (sparse?) or by looking at the code?

Regards
Martin

author
#3 lore raw
Quoted 8 lines · Martin Blumenstingl

Hi Dan,

Quoted 5 lines · Dan Carpenter

The meson_sar_adc_lock() function is not supposed to hold the "indio_dev->mlock" on the error path.

Fixes 3adbf3427330 ("iio: adc: add a driver for the SAR ADC found in Amlogic Meson SoCs")
Signed-off-by Dan Carpenter <[email protected]>
Quoted 8 lines
Acked-by Martin Blumenstingl<[email protected]>

(even if I might be too late already since Jonathan already applied this patch)

thank you for finding and fixing this!
out of curiosity: did you find this with some tool (sparse?) or by looking at the code?

This was some Smatch stuff that I haven't pushed. I re-wrote the locking check to use cross function analysis, and it actually warns about the callers not meson_sar_adc_lock(). That code works ok basically for locks but it generates a ton of false positives about double unlocks so I haven't pushed it.

regards,
dan carpenter