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]
Message-ID: <20250618031638.26477-4-andrew.lopes@alumni.usp.br>
Date: Wed, 18 Jun 2025 00:12:18 -0300
From: Andrew Ijano <andrew.ijano@...il.com>
To: jic23@...nel.org
Cc: andrew.lopes@...mni.usp.br,
	gustavobastos@....br,
	dlechner@...libre.com,
	nuno.sa@...log.com,
	andy@...nel.org,
	jstephan@...libre.com,
	linux-iio@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: [PATCH v6 3/4] iio: accel: sca3000: use lock guards

Use guard() and scoped_guard() for handling mutex lock instead of
manually locking and unlocking the mutex. This prevents forgotten
locks due to early exits and remove the need of gotos.

Signed-off-by: Andrew Ijano <andrew.lopes@...mni.usp.br>
Co-developed-by: Gustavo Bastos <gustavobastos@....br>
Signed-off-by: Gustavo Bastos <gustavobastos@....br>
Suggested-by: Jonathan Cameron <jic23@...nel.org>
---
 drivers/iio/accel/sca3000.c | 205 +++++++++++++-----------------------
 1 file changed, 73 insertions(+), 132 deletions(-)

diff --git a/drivers/iio/accel/sca3000.c b/drivers/iio/accel/sca3000.c
index 7145b4541264..058a2d67c91c 100644
--- a/drivers/iio/accel/sca3000.c
+++ b/drivers/iio/accel/sca3000.c
@@ -405,17 +405,14 @@ static int sca3000_print_rev(struct iio_dev *indio_dev)
 	int ret;
 	struct sca3000_state *st = iio_priv(indio_dev);
 
-	mutex_lock(&st->lock);
+	guard(mutex)(&st->lock);
 	ret = spi_w8r8(st->us, SCA3000_READ_REG(SCA3000_REG_REVID_ADDR));
 	if (ret < 0)
-		goto error_ret;
+		return ret;
 	dev_info(&indio_dev->dev,
 		 "sca3000 revision major=%lu, minor=%lu\n",
 		 ret & SCA3000_REG_REVID_MAJOR_MASK,
 		 ret & SCA3000_REG_REVID_MINOR_MASK);
-error_ret:
-	mutex_unlock(&st->lock);
-
 	return ret;
 }
 
@@ -698,33 +695,27 @@ static int sca3000_read_raw(struct iio_dev *indio_dev,
 	u8 address;
 
 	switch (mask) {
-	case IIO_CHAN_INFO_RAW:
-		mutex_lock(&st->lock);
+	case IIO_CHAN_INFO_RAW: {
+		guard(mutex)(&st->lock);
 		if (chan->type == IIO_ACCEL) {
-			if (st->mo_det_use_count) {
-				mutex_unlock(&st->lock);
+			if (st->mo_det_use_count)
 				return -EBUSY;
-			}
 			address = sca3000_addresses[chan->address][0];
 			ret = spi_w8r16be(st->us, SCA3000_READ_REG(address));
-			if (ret < 0) {
-				mutex_unlock(&st->lock);
+			if (ret < 0)
 				return ret;
-			}
 			*val = sign_extend32(ret >> chan->scan_type.shift,
 					     chan->scan_type.realbits - 1);
 		} else {
 			/* get the temperature when available */
 			ret = spi_w8r16be(st->us, SCA3000_READ_REG(SCA3000_REG_TEMP_MSB_ADDR));
-			if (ret < 0) {
-				mutex_unlock(&st->lock);
+			if (ret < 0)
 				return ret;
-			}
 			*val = (ret >> chan->scan_type.shift) &
 				GENMASK(chan->scan_type.realbits - 1, 0);
 		}
-		mutex_unlock(&st->lock);
 		return IIO_VAL_INT;
+	}
 	case IIO_CHAN_INFO_SCALE:
 		*val = 0;
 		if (chan->type == IIO_ACCEL)
@@ -736,16 +727,16 @@ static int sca3000_read_raw(struct iio_dev *indio_dev,
 		*val = -214;
 		*val2 = 600000;
 		return IIO_VAL_INT_PLUS_MICRO;
-	case IIO_CHAN_INFO_SAMP_FREQ:
-		mutex_lock(&st->lock);
+	case IIO_CHAN_INFO_SAMP_FREQ: {
+		guard(mutex)(&st->lock);
 		ret = sca3000_read_raw_samp_freq(st, val);
-		mutex_unlock(&st->lock);
 		return ret ? ret : IIO_VAL_INT;
-	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
-		mutex_lock(&st->lock);
+	}
+	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY: {
+		guard(mutex)(&st->lock);
 		ret = sca3000_read_3db_freq(st, val);
-		mutex_unlock(&st->lock);
 		return ret;
+	}
 	default:
 		return -EINVAL;
 	}
@@ -756,28 +747,23 @@ static int sca3000_write_raw(struct iio_dev *indio_dev,
 			     int val, int val2, long mask)
 {
 	struct sca3000_state *st = iio_priv(indio_dev);
-	int ret;
 
 	switch (mask) {
-	case IIO_CHAN_INFO_SAMP_FREQ:
+	case IIO_CHAN_INFO_SAMP_FREQ: {
+		guard(mutex)(&st->lock);
 		if (val2)
 			return -EINVAL;
-		mutex_lock(&st->lock);
-		ret = sca3000_write_raw_samp_freq(st, val);
-		mutex_unlock(&st->lock);
-		return ret;
-	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
+		return sca3000_write_raw_samp_freq(st, val);
+	}
+	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY: {
+		guard(mutex)(&st->lock);
 		if (val2)
 			return -EINVAL;
-		mutex_lock(&st->lock);
-		ret = sca3000_write_3db_freq(st, val);
-		mutex_unlock(&st->lock);
-		return ret;
+		return sca3000_write_3db_freq(st, val);
+	}
 	default:
 		return -EINVAL;
 	}
-
-	return ret;
 }
 
 /**
@@ -800,9 +786,9 @@ static ssize_t sca3000_read_av_freq(struct device *dev,
 	unsigned int len = 0;
 	int ret;
 
-	mutex_lock(&st->lock);
-	ret = spi_w8r8(st->us, SCA3000_READ_REG(SCA3000_REG_MODE_ADDR));
-	mutex_unlock(&st->lock);
+	scoped_guard(mutex, &st->lock) {
+		ret = spi_w8r8(st->us, SCA3000_READ_REG(SCA3000_REG_MODE_ADDR));
+	}
 	if (ret)
 		return ret;
 
@@ -850,11 +836,10 @@ static int sca3000_read_event_value(struct iio_dev *indio_dev,
 	int i;
 
 	switch (info) {
-	case IIO_EV_INFO_VALUE:
-		mutex_lock(&st->lock);
-		ret = sca3000_read_ctrl_reg(st,
-					    sca3000_addresses[chan->address][1]);
-		mutex_unlock(&st->lock);
+	case IIO_EV_INFO_VALUE: {
+		scoped_guard(mutex, &st->lock) {
+			ret = sca3000_read_ctrl_reg(st, sca3000_addresses[chan->address][1]);
+		}
 		if (ret < 0)
 			return ret;
 		*val = 0;
@@ -868,6 +853,7 @@ static int sca3000_read_event_value(struct iio_dev *indio_dev,
 				*val += st->info->mot_det_mult_xz[i];
 
 		return IIO_VAL_INT;
+	}
 	case IIO_EV_INFO_PERIOD:
 		*val = 0;
 		*val2 = 226000;
@@ -898,7 +884,6 @@ static int sca3000_write_event_value(struct iio_dev *indio_dev,
 				     int val, int val2)
 {
 	struct sca3000_state *st = iio_priv(indio_dev);
-	int ret;
 	int i;
 	u8 nonlinear = 0;
 
@@ -918,13 +903,10 @@ static int sca3000_write_event_value(struct iio_dev *indio_dev,
 			}
 	}
 
-	mutex_lock(&st->lock);
-	ret = sca3000_write_ctrl_reg(st,
+	guard(mutex)(&st->lock);
+	return sca3000_write_ctrl_reg(st,
 				     sca3000_addresses[chan->address][1],
 				     nonlinear);
-	mutex_unlock(&st->lock);
-
-	return ret;
 }
 
 static struct attribute *sca3000_attributes[] = {
@@ -969,12 +951,12 @@ static void sca3000_ring_int_process(u8 val, struct iio_dev *indio_dev)
 	struct sca3000_state *st = iio_priv(indio_dev);
 	int ret, i, num_available;
 
-	mutex_lock(&st->lock);
+	guard(mutex)(&st->lock);
 
 	if (val & SCA3000_REG_INT_STATUS_HALF) {
 		ret = spi_w8r8(st->us, SCA3000_READ_REG(SCA3000_REG_BUF_COUNT_ADDR));
 		if (ret)
-			goto error_ret;
+			return;
 		num_available = ret;
 		/*
 		 * num_available is the total number of samples available
@@ -982,7 +964,7 @@ static void sca3000_ring_int_process(u8 val, struct iio_dev *indio_dev)
 		 */
 		ret = sca3000_read_data(st, SCA3000_REG_RING_OUT_ADDR, num_available * 2);
 		if (ret)
-			goto error_ret;
+			return;
 		for (i = 0; i < num_available / 3; i++) {
 			/*
 			 * Dirty hack to cover for 11 bit in fifo, 13 bit
@@ -994,8 +976,6 @@ static void sca3000_ring_int_process(u8 val, struct iio_dev *indio_dev)
 			iio_push_to_buffers(indio_dev, st->rx + i * 3 * 2);
 		}
 	}
-error_ret:
-	mutex_unlock(&st->lock);
 }
 
 /**
@@ -1021,9 +1001,8 @@ static irqreturn_t sca3000_event_handler(int irq, void *private)
 	 * Could lead if badly timed to an extra read of status reg,
 	 * but ensures no interrupt is missed.
 	 */
-	mutex_lock(&st->lock);
+	guard(mutex)(&st->lock);
 	ret = spi_w8r8(st->us, SCA3000_READ_REG(SCA3000_REG_INT_STATUS_ADDR));
-	mutex_unlock(&st->lock);
 	if (ret)
 		goto done;
 
@@ -1080,16 +1059,15 @@ static int sca3000_read_event_config(struct iio_dev *indio_dev,
 	struct sca3000_state *st = iio_priv(indio_dev);
 	int ret;
 	/* read current value of mode register */
-	mutex_lock(&st->lock);
+	guard(mutex)(&st->lock);
 
 	ret = spi_w8r8(st->us, SCA3000_READ_REG(SCA3000_REG_MODE_ADDR));
 	if (ret)
-		goto error_ret;
+		return ret;
 
 	switch (chan->channel2) {
 	case IIO_MOD_X_AND_Y_AND_Z:
-		ret = !!(ret & SCA3000_REG_MODE_FREE_FALL_DETECT);
-		break;
+		return !!(ret & SCA3000_REG_MODE_FREE_FALL_DETECT);
 	case IIO_MOD_X:
 	case IIO_MOD_Y:
 	case IIO_MOD_Z:
@@ -1099,24 +1077,18 @@ static int sca3000_read_event_config(struct iio_dev *indio_dev,
 		 */
 		if ((ret & SCA3000_REG_MODE_MODE_MASK)
 		    != SCA3000_REG_MODE_MEAS_MODE_MOT_DET) {
-			ret = 0;
+			return 0;
 		} else {
 			ret = sca3000_read_ctrl_reg(st,
 						SCA3000_REG_CTRL_SEL_MD_CTRL);
 			if (ret < 0)
-				goto error_ret;
+				return ret;
 			/* only supporting logical or's for now */
-			ret = !!(ret & sca3000_addresses[chan->address][2]);
+			return !!(ret & sca3000_addresses[chan->address][2]);
 		}
-		break;
 	default:
-		ret = -EINVAL;
+		return -EINVAL;
 	}
-
-error_ret:
-	mutex_unlock(&st->lock);
-
-	return ret;
 }
 
 static int sca3000_freefall_set_state(struct iio_dev *indio_dev, bool state)
@@ -1217,28 +1189,20 @@ static int sca3000_write_event_config(struct iio_dev *indio_dev,
 				      bool state)
 {
 	struct sca3000_state *st = iio_priv(indio_dev);
-	int ret;
 
-	mutex_lock(&st->lock);
+	guard(mutex)(&st->lock);
 	switch (chan->channel2) {
 	case IIO_MOD_X_AND_Y_AND_Z:
-		ret = sca3000_freefall_set_state(indio_dev, state);
-		break;
-
+		return sca3000_freefall_set_state(indio_dev, state);
 	case IIO_MOD_X:
 	case IIO_MOD_Y:
 	case IIO_MOD_Z:
-		ret = sca3000_motion_detect_set_state(indio_dev,
+		return sca3000_motion_detect_set_state(indio_dev,
 						      chan->address,
 						      state);
-		break;
 	default:
-		ret = -EINVAL;
-		break;
+		return -EINVAL;
 	}
-	mutex_unlock(&st->lock);
-
-	return ret;
 }
 
 static inline
@@ -1247,23 +1211,19 @@ int __sca3000_hw_ring_state_set(struct iio_dev *indio_dev, bool state)
 	struct sca3000_state *st = iio_priv(indio_dev);
 	int ret;
 
-	mutex_lock(&st->lock);
 	ret = spi_w8r8(st->us, SCA3000_READ_REG(SCA3000_REG_MODE_ADDR));
 	if (ret)
-		goto error_ret;
+		return ret;
+
 	if (state) {
 		dev_info(&indio_dev->dev, "supposedly enabling ring buffer\n");
-		ret = sca3000_write_reg(st,
+		return sca3000_write_reg(st,
 			SCA3000_REG_MODE_ADDR,
 			ret | SCA3000_REG_MODE_RING_BUF_ENABLE);
-	} else
-		ret = sca3000_write_reg(st,
-			SCA3000_REG_MODE_ADDR,
-			ret & ~SCA3000_REG_MODE_RING_BUF_ENABLE);
-error_ret:
-	mutex_unlock(&st->lock);
-
-	return ret;
+	}
+	return sca3000_write_reg(st,
+		SCA3000_REG_MODE_ADDR,
+		ret & ~SCA3000_REG_MODE_RING_BUF_ENABLE);
 }
 
 /**
@@ -1280,26 +1240,18 @@ static int sca3000_hw_ring_preenable(struct iio_dev *indio_dev)
 	int ret;
 	struct sca3000_state *st = iio_priv(indio_dev);
 
-	mutex_lock(&st->lock);
-
+	guard(mutex)(&st->lock);
 	/* Enable the 50% full interrupt */
 	ret = spi_w8r8(st->us, SCA3000_READ_REG(SCA3000_REG_INT_MASK_ADDR));
 	if (ret)
-		goto error_unlock;
+		return ret;
 	ret = sca3000_write_reg(st,
 				SCA3000_REG_INT_MASK_ADDR,
 				ret | SCA3000_REG_INT_MASK_RING_HALF);
 	if (ret)
-		goto error_unlock;
-
-	mutex_unlock(&st->lock);
+		return ret;
 
 	return __sca3000_hw_ring_state_set(indio_dev, 1);
-
-error_unlock:
-	mutex_unlock(&st->lock);
-
-	return ret;
 }
 
 static int sca3000_hw_ring_postdisable(struct iio_dev *indio_dev)
@@ -1307,22 +1259,18 @@ static int sca3000_hw_ring_postdisable(struct iio_dev *indio_dev)
 	int ret;
 	struct sca3000_state *st = iio_priv(indio_dev);
 
+	guard(mutex)(&st->lock);
 	ret = __sca3000_hw_ring_state_set(indio_dev, 0);
 	if (ret)
 		return ret;
 
 	/* Disable the 50% full interrupt */
-	mutex_lock(&st->lock);
-
 	ret = spi_w8r8(st->us, SCA3000_READ_REG(SCA3000_REG_INT_MASK_ADDR));
 	if (ret)
-		goto unlock;
-	ret = sca3000_write_reg(st,
+		return ret;
+	return sca3000_write_reg(st,
 				SCA3000_REG_INT_MASK_ADDR,
 				ret & ~SCA3000_REG_INT_MASK_RING_HALF);
-unlock:
-	mutex_unlock(&st->lock);
-	return ret;
 }
 
 static const struct iio_buffer_setup_ops sca3000_ring_setup_ops = {
@@ -1342,25 +1290,25 @@ static int sca3000_clean_setup(struct sca3000_state *st)
 {
 	int ret;
 
-	mutex_lock(&st->lock);
+	guard(mutex)(&st->lock);
 	/* Ensure all interrupts have been acknowledged */
 	ret = spi_w8r8(st->us, SCA3000_READ_REG(SCA3000_REG_INT_MASK_ADDR));
 	if (ret)
-		goto error_ret;
+		return ret;
 
 	/* Turn off all motion detection channels */
 	ret = sca3000_read_ctrl_reg(st, SCA3000_REG_CTRL_SEL_MD_CTRL);
 	if (ret < 0)
-		goto error_ret;
+		return ret;
 	ret = sca3000_write_ctrl_reg(st, SCA3000_REG_CTRL_SEL_MD_CTRL,
 				     ret & SCA3000_MD_CTRL_PROT_MASK);
 	if (ret)
-		goto error_ret;
+		return ret;
 
 	/* Disable ring buffer */
 	ret = sca3000_read_ctrl_reg(st, SCA3000_REG_CTRL_SEL_OUT_CTRL);
 	if (ret < 0)
-		goto error_ret;
+		return ret;
 	ret = sca3000_write_ctrl_reg(st, SCA3000_REG_CTRL_SEL_OUT_CTRL,
 				     (ret & SCA3000_REG_OUT_CTRL_PROT_MASK)
 				     | SCA3000_REG_OUT_CTRL_BUF_X_EN
@@ -1368,17 +1316,17 @@ static int sca3000_clean_setup(struct sca3000_state *st)
 				     | SCA3000_REG_OUT_CTRL_BUF_Z_EN
 				     | SCA3000_REG_OUT_CTRL_BUF_DIV_4);
 	if (ret)
-		goto error_ret;
+		return ret;
 	/* Enable interrupts, relevant to mode and set up as active low */
 	ret = spi_w8r8(st->us, SCA3000_READ_REG(SCA3000_REG_INT_MASK_ADDR));
 	if (ret)
-		goto error_ret;
+		return ret;
 	ret = sca3000_write_reg(st,
 				SCA3000_REG_INT_MASK_ADDR,
 				(ret & SCA3000_REG_INT_MASK_PROT_MASK)
 				| SCA3000_REG_INT_MASK_ACTIVE_LOW);
 	if (ret)
-		goto error_ret;
+		return ret;
 	/*
 	 * Select normal measurement mode, free fall off, ring off
 	 * Ring in 12 bit mode - it is fine to overwrite reserved bits 3,5
@@ -1386,13 +1334,9 @@ static int sca3000_clean_setup(struct sca3000_state *st)
 	 */
 	ret = spi_w8r8(st->us, SCA3000_READ_REG(SCA3000_REG_MODE_ADDR));
 	if (ret)
-		goto error_ret;
-	ret = sca3000_write_reg(st, SCA3000_REG_MODE_ADDR,
+		return ret;
+	return sca3000_write_reg(st, SCA3000_REG_MODE_ADDR,
 				ret & SCA3000_MODE_PROT_MASK);
-
-error_ret:
-	mutex_unlock(&st->lock);
-	return ret;
 }
 
 static const struct iio_info sca3000_info = {
@@ -1470,19 +1414,16 @@ static int sca3000_stop_all_interrupts(struct sca3000_state *st)
 {
 	int ret;
 
-	mutex_lock(&st->lock);
+	guard(mutex)(&st->lock);
 	ret = spi_w8r8(st->us, SCA3000_READ_REG(SCA3000_REG_INT_MASK_ADDR));
 	if (ret)
-		goto error_ret;
+		return ret;
 
-	ret = sca3000_write_reg(st, SCA3000_REG_INT_MASK_ADDR,
+	return sca3000_write_reg(st, SCA3000_REG_INT_MASK_ADDR,
 				ret &
 				~(SCA3000_REG_INT_MASK_RING_THREE_QUARTER |
 				  SCA3000_REG_INT_MASK_RING_HALF |
 				  SCA3000_REG_INT_MASK_ALL_INTS));
-error_ret:
-	mutex_unlock(&st->lock);
-	return ret;
 }
 
 static void sca3000_remove(struct spi_device *spi)
-- 
2.49.0


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ