Linux IIO development
 help / color / mirror / Atom feed
From: Taha Ed-Dafili <0rayn.dev@gmail.com>
To: jic23@kernel.org, lars@metafoo.de
Cc: Michael.Hennerich@analog.com, robh@kernel.org,
	krzk+dt@kernel.org, conor+dt@kernel.org, dlechner@baylibre.com,
	nuno.sa@analog.com, andy@kernel.org, skhan@linuxfoundation.org,
	linux@analog.com, linux-iio@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	Taha Ed-Dafili <0rayn.dev@gmail.com>
Subject: [PATCH v4 4/6] iio: dac: ad5504: introduce local lock to protect state and spi transfers
Date: Mon, 17 Aug 2026 17:11:13 -0400	[thread overview]
Message-ID: <20260817211118.21833-5-0rayn.dev@gmail.com> (raw)
In-Reply-To: <20260817211118.21833-1-0rayn.dev@gmail.com>

The AD5504 driver currently lacks locking, exposing it to several
multi-threading race conditions:
1. The shared DMA-safe SPI transfer buffers (st->data) can be
   corrupted if multiple threads trigger read_raw or write_raw
   simultaneously.
2. The ad5504_write_dac_powerdown() routine executes a sequence of
   back-to-back SPI writes (a CTRL register update followed by a
   mandatory NOOP). This entire sequence must be atomic.
3. Internal state variables like pwr_down_mask and pwr_down_mode
   can be read and modified concurrently.

Introduce a mutex in the ad5504_state structure and initialize it via
devm_mutex_init() in probe. Use the modern scoped guard(mutex) macro
at the top-level public IIO callbacks (read_raw, write_raw, and the
powerdown attributes) to safely serialize access to the device state
and the SPI bus.

In ad5504_read_raw() and ad5504_write_raw(), guard(mutex) is scoped to
the IIO_CHAN_INFO_RAW case only, since IIO_CHAN_INFO_SCALE merely reads
vref_mv, which is fixed at probe time and never modified afterward and
therefore needs no serialization. Because guard(mutex) declares a
cleanup-scoped variable, it cannot appear directly after a case label;
wrap the case body in a compound statement (case IIO_CHAN_INFO_RAW: {
... }) to give it the block scope it requires.

Signed-off-by: Taha Ed-Dafili <0rayn.dev@gmail.com>
---
Note: The concurrency race conditions addressed in this patch were 
originally reported by the Sashiko bot:
https://sashiko.dev/#/patchset/20260509142047.30302-1-0rayn.dev@gmail.com

 drivers/iio/dac/ad5504.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/dac/ad5504.c b/drivers/iio/dac/ad5504.c
index 55ce7e49e0e0..87946cdf69e7 100644
--- a/drivers/iio/dac/ad5504.c
+++ b/drivers/iio/dac/ad5504.c
@@ -7,12 +7,14 @@
 
 #include <linux/array_size.h>
 #include <linux/bits.h>
+#include <linux/cleanup.h>
 #include <linux/dev_printk.h>
 #include <linux/errno.h>
 #include <linux/interrupt.h>
 #include <linux/kstrtox.h>
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
+#include <linux/mutex.h>
 #include <linux/regulator/consumer.h>
 #include <linux/spi/spi.h>
 #include <linux/sysfs.h>
@@ -49,10 +51,12 @@
  * @pwr_down_mask:	power down mask
  * @pwr_down_mode:	current power down mode
  * @data:		transfer buffer
+ * @lock:		lock to protect state and spi transfers
  */
 struct ad5504_state {
 	struct spi_device		*spi;
 	struct regulator		*reg;
+	struct mutex			lock;
 	unsigned short			vref_mv;
 	unsigned			pwr_down_mask;
 	unsigned			pwr_down_mode;
@@ -103,7 +107,8 @@ static int ad5504_read_raw(struct iio_dev *indio_dev,
 	int ret;
 
 	switch (m) {
-	case IIO_CHAN_INFO_RAW:
+	case IIO_CHAN_INFO_RAW: {
+		guard(mutex)(&st->lock);
 		ret = ad5504_spi_read(st, chan->address);
 		if (ret < 0)
 			return ret;
@@ -111,6 +116,7 @@ static int ad5504_read_raw(struct iio_dev *indio_dev,
 		*val = ret;
 
 		return IIO_VAL_INT;
+	}
 	case IIO_CHAN_INFO_SCALE:
 		*val = st->vref_mv;
 		*val2 = chan->scan_type.realbits;
@@ -128,11 +134,13 @@ static int ad5504_write_raw(struct iio_dev *indio_dev,
 	struct ad5504_state *st = iio_priv(indio_dev);
 
 	switch (mask) {
-	case IIO_CHAN_INFO_RAW:
+	case IIO_CHAN_INFO_RAW: {
+		guard(mutex)(&st->lock);
 		if (val >= (1 << chan->scan_type.realbits) || val < 0)
 			return -EINVAL;
 
 		return ad5504_spi_write(st, chan->address, val);
+	}
 	default:
 		return -EINVAL;
 	}
@@ -148,6 +156,7 @@ static int ad5504_get_powerdown_mode(struct iio_dev *indio_dev,
 {
 	struct ad5504_state *st = iio_priv(indio_dev);
 
+	guard(mutex)(&st->lock);
 	return st->pwr_down_mode;
 }
 
@@ -156,6 +165,7 @@ static int ad5504_set_powerdown_mode(struct iio_dev *indio_dev,
 {
 	struct ad5504_state *st = iio_priv(indio_dev);
 
+	guard(mutex)(&st->lock);
 	st->pwr_down_mode = mode;
 
 	return 0;
@@ -173,6 +183,7 @@ static ssize_t ad5504_read_dac_powerdown(struct iio_dev *indio_dev,
 {
 	struct ad5504_state *st = iio_priv(indio_dev);
 
+	guard(mutex)(&st->lock);
 	return sysfs_emit(buf, "%d\n",
 			  !(st->pwr_down_mask & (1 << chan->channel)));
 }
@@ -185,6 +196,7 @@ static ssize_t ad5504_write_dac_powerdown(struct iio_dev *indio_dev,
 	int ret;
 	struct ad5504_state *st = iio_priv(indio_dev);
 
+	guard(mutex)(&st->lock);
 	ret = kstrtobool(buf, &pwr_down);
 	if (ret)
 		return ret;
@@ -284,6 +296,10 @@ static int ad5504_probe(struct spi_device *spi)
 
 	st = iio_priv(indio_dev);
 
+	ret = devm_mutex_init(dev, &st->lock);
+	if (ret)
+		return ret;
+
 	ret = devm_regulator_get_enable_read_voltage(dev, "vcc");
 	if (ret < 0)
 		return ret;
-- 
2.55.0


  parent reply	other threads:[~2026-08-17 21:11 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-17 21:11 [PATCH v4 0/6] iio: dac: ad5504: bindings, cleanups, locking, and scale fixes Taha Ed-Dafili
2026-08-17 21:11 ` [PATCH v4 1/6] dt-bindings: iio: dac: ad5504: add output-range and missing gpios Taha Ed-Dafili
2026-08-17 21:11 ` [PATCH v4 2/6] iio: dac: ad5504: Align headers with IWYU principle Taha Ed-Dafili
2026-08-17 21:11 ` [PATCH v4 3/6] iio: dac: ad5504: remove legacy platform data support Taha Ed-Dafili
2026-08-17 21:11 ` Taha Ed-Dafili [this message]
2026-08-17 21:11 ` [PATCH v4 5/6] iio: dac: ad5504: strictly separate ACPI and DT probe paths Taha Ed-Dafili
2026-08-17 21:11 ` [PATCH v4 6/6] iio: dac: ad5504: support scale via output-range-microvolt property Taha Ed-Dafili

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260817211118.21833-5-0rayn.dev@gmail.com \
    --to=0rayn.dev@gmail.com \
    --cc=Michael.Hennerich@analog.com \
    --cc=andy@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dlechner@baylibre.com \
    --cc=jic23@kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=lars@metafoo.de \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@analog.com \
    --cc=nuno.sa@analog.com \
    --cc=robh@kernel.org \
    --cc=skhan@linuxfoundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox