mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v5 0/2] iio: adc: ad_sigma_delta: fix CS assertion and registerless device handling
@ 2026-05-27  9:38 Radu Sabau via B4 Relay
  2026-05-27  9:38 ` [PATCH v5 1/2] iio: adc: ad_sigma_delta: fix CS held asserted and state leaks Radu Sabau via B4 Relay
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Radu Sabau via B4 Relay @ 2026-05-27  9:38 UTC (permalink / raw)
  To: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	David Lechner, Nuno Sá,
	Andy Shevchenko, Uwe Kleine-König
  Cc: linux-iio, linux-kernel, Radu Sabau, Jonathan Cameron

This series fixes two independent bugs in the ad_sigma_delta framework.

Patch 1 fixes CS being left permanently asserted after single conversion
and in the error path of ad_sd_buffer_postenable(). In
ad_sigma_delta_single_conversion(), set_mode(AD_SD_MODE_IDLE) and
disable_one() were executing while keep_cs_asserted was still true,
causing any SPI transfer they issued to carry cs_change=1. The
postenable() error path also failed to call set_mode(AD_SD_MODE_IDLE),
leaving the device in continuous conversion mode with bus_locked
incorrectly set, opening a window for concurrent SPI access.

Patch 2 fixes ad_sigma_delta_clear_pending_event() for devices with                                     
has_registers = false and no rdy_gpiod (currently AD7191, AD7780, and
MAX11205). These devices fall through to the status register read path,                                 
but since has_registers is false, ad_sd_read_reg() transmits no address                               
byte and blindly clocks raw MISO bytes — indistinguishable from reading
conversion data, partially consuming any pending result and corrupting the
stream. With num_resetclks = 0 on these devices a further hazard exists:
if pending_event is set, the drain path attempts memset of SIZE_MAX bytes,
corrupting the heap. The fix returns 0 immediately for registerless
devices. This is safe for all current instances: AD7191 and AD7780 (with
powerdown GPIO) are reset between conversions by CS deassertion; AD7780
(without powerdown GPIO) and MAX11205 are continuously-converting and
cycle ~DRDY regardless, so the next falling edge fires naturally. A future
registerless device that holds ~DRDY asserted until data is read would
need num_resetclks set or a rdy-gpio instead. The same heap corruption can
be triggered on any device with rdy_gpiod set but num_resetclks = 0, so
an explicit data_read_len == 0 guard is added independently.

Signed-off-by: Radu Sabau <radu.sabau@analog.com>
---
Changes in v5:
- Removed the IRQ_DISABLE_UNLAZY paragraph entirely from the commit
  message — it was backwards and based on an implementation-defined assumption
- Added concrete per-device reasoning (CS=PDOWN reset for ad7191/ad7780,
  continuous-converting for ad7780/max11205)
- Added the pitfall paragraph for future registerless devices
- Last paragraph: removed "let the latched IRQ edge fire" — replaced with the
  correct explanation that the stale result is consumed by ad_sigma_delta_single_conversion()
- Link to v4: https://lore.kernel.org/r/20260521-ad_sigma_delta-fix-v4-0-bfb3df3e36da@analog.com

Changes in v4:
- set_mode(AD_SD_MODE_IDLE) was accidentally placed in patch 2
  in v3; moved to the correct commit via rebase.
- add data_read_len == 0 guard to cover the heap corruption path
  reachable via rdy_gpiod on devices with num_resetclks = 0;
  set_mode(AD_SD_MODE_IDLE) moved to patch 1.
- Link to v3: https://lore.kernel.org/r/20260518-ad_sigma_delta-fix-v3-0-a2a92b0c36f3@analog.com

Changes in v3:
- add ad_sigma_delta_set_mode(AD_SD_MODE_IDLE) to the err_unlock
  path in ad_sd_buffer_postenable() to revert the device
  from continuous mode and deassert CS; previously only the flag resets
  were added. Update commit message to remove the inaccurate "in all
  cases" claim and note that CS-less devices such as MAX11205 are
  unaffected since no physical line is toggled.
- Patch 2: new patch fixing ad_sigma_delta_clear_pending_event() for
  devices with has_registers = false and no rdy_gpiod, where the
  existing status register read path blindly clocks raw MISO bytes,
  partially consuming pending conversion data and potentially
  corrupting the heap via a SIZE_MAX memset.
- Link to v2: https://lore.kernel.org/r/20260507-ad_sigma_delta-fix-v2-1-ec86eb0463bd@analog.com

Changes in v2:
- Move set_mode(AD_SD_MODE_IDLE) into out_unlock: as well, not only
  disable_one(); v1 left set_mode() above the label where
  keep_cs_asserted is still true, so devices without the optional
  disable_one callback still had CS stuck after that transfer.
- Fix pre-existing state leak in ad_sd_buffer_postenable() err_unlock:
  reset bus_locked and keep_cs_asserted before spi_bus_unlock() to
  prevent spi_sync_locked() being called on an unlocked controller.
- Link to v1: https://lore.kernel.org/r/20260428-ad_sigma_delta-fix-v1-1-8e3f925ee8d2@analog.com

---
Radu Sabau (2):
      iio: adc: ad_sigma_delta: fix CS held asserted and state leaks
      iio: adc: ad_sigma_delta: fix clear_pending_event for registerless devices

 drivers/iio/adc/ad_sigma_delta.c | 39 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 34 insertions(+), 5 deletions(-)
---
base-commit: 3b3bea6d4b9c162f9e555905d96b8c1da67ecd5b
change-id: 20260428-ad_sigma_delta-fix-bb65d56ccbb0

Best regards,
-- 
Radu Sabau <radu.sabau@analog.com>



^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v5 1/2] iio: adc: ad_sigma_delta: fix CS held asserted and state leaks
  2026-05-27  9:38 [PATCH v5 0/2] iio: adc: ad_sigma_delta: fix CS assertion and registerless device handling Radu Sabau via B4 Relay
@ 2026-05-27  9:38 ` Radu Sabau via B4 Relay
  2026-05-29  8:38   ` Sabau, Radu bogdan
  2026-05-27  9:38 ` [PATCH v5 2/2] iio: adc: ad_sigma_delta: fix clear_pending_event for registerless devices Radu Sabau via B4 Relay
  2026-05-27 11:16 ` [PATCH v5 0/2] iio: adc: ad_sigma_delta: fix CS assertion and registerless device handling Jonathan Cameron
  2 siblings, 1 reply; 8+ messages in thread
From: Radu Sabau via B4 Relay @ 2026-05-27  9:38 UTC (permalink / raw)
  To: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	David Lechner, Nuno Sá,
	Andy Shevchenko, Uwe Kleine-König
  Cc: linux-iio, linux-kernel, Radu Sabau, Jonathan Cameron

From: Radu Sabau <radu.sabau@analog.com>

In ad_sigma_delta_single_conversion(), set_mode(AD_SD_MODE_IDLE) and
disable_one() were called from the out: block while keep_cs_asserted
was still true. This caused any SPI transfer issued by those callbacks
to carry cs_change=1, leaving CS permanently asserted after the
conversion. Fix by moving both calls into the out_unlock: block, after
keep_cs_asserted is cleared, matching the pattern already used in
ad_sd_calibrate().

In the error path of ad_sd_buffer_postenable(), if an operation fails
after set_mode(AD_SD_MODE_CONTINUOUS) has already succeeded (e.g.
spi_offload_trigger_enable()), the device is left in continuous
conversion mode with CS physically asserted. Additionally,
bus_locked remaining true after spi_bus_unlock() causes subsequent
SPI operations to call spi_sync_locked() without the bus lock actually
held, allowing concurrent SPI access.

Fix the error path by clearing keep_cs_asserted first, then calling
set_mode(AD_SD_MODE_IDLE) to revert the device mode and deassert CS,
then clearing bus_locked before releasing the bus.

For devices that implement neither set_mode nor disable_one (such as
MAX11205, which has no physical CS pin), no SPI transfer is issued
during cleanup and the cs_change flag has no effect on any physical
line.

Signed-off-by: Radu Sabau <radu.sabau@analog.com>
---
 drivers/iio/adc/ad_sigma_delta.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/adc/ad_sigma_delta.c b/drivers/iio/adc/ad_sigma_delta.c
index a955556f9ec8..651ade67ad2e 100644
--- a/drivers/iio/adc/ad_sigma_delta.c
+++ b/drivers/iio/adc/ad_sigma_delta.c
@@ -441,11 +441,10 @@ int ad_sigma_delta_single_conversion(struct iio_dev *indio_dev,
 out:
 	ad_sd_disable_irq(sigma_delta);
 
-	ad_sigma_delta_set_mode(sigma_delta, AD_SD_MODE_IDLE);
-	ad_sigma_delta_disable_one(sigma_delta, chan->address);
-
 out_unlock:
 	sigma_delta->keep_cs_asserted = false;
+	ad_sigma_delta_set_mode(sigma_delta, AD_SD_MODE_IDLE);
+	ad_sigma_delta_disable_one(sigma_delta, chan->address);
 	sigma_delta->bus_locked = false;
 	spi_bus_unlock(sigma_delta->spi->controller);
 out_release:
@@ -578,6 +577,9 @@ static int ad_sd_buffer_postenable(struct iio_dev *indio_dev)
 	return 0;
 
 err_unlock:
+	sigma_delta->keep_cs_asserted = false;
+	ad_sigma_delta_set_mode(sigma_delta, AD_SD_MODE_IDLE);
+	sigma_delta->bus_locked = false;
 	spi_bus_unlock(sigma_delta->spi->controller);
 	spi_unoptimize_message(&sigma_delta->sample_msg);
 

-- 
2.43.0



^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v5 2/2] iio: adc: ad_sigma_delta: fix clear_pending_event for registerless devices
  2026-05-27  9:38 [PATCH v5 0/2] iio: adc: ad_sigma_delta: fix CS assertion and registerless device handling Radu Sabau via B4 Relay
  2026-05-27  9:38 ` [PATCH v5 1/2] iio: adc: ad_sigma_delta: fix CS held asserted and state leaks Radu Sabau via B4 Relay
@ 2026-05-27  9:38 ` Radu Sabau via B4 Relay
  2026-05-29  8:40   ` Sabau, Radu bogdan
  2026-05-27 11:16 ` [PATCH v5 0/2] iio: adc: ad_sigma_delta: fix CS assertion and registerless device handling Jonathan Cameron
  2 siblings, 1 reply; 8+ messages in thread
From: Radu Sabau via B4 Relay @ 2026-05-27  9:38 UTC (permalink / raw)
  To: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	David Lechner, Nuno Sá,
	Andy Shevchenko, Uwe Kleine-König
  Cc: linux-iio, linux-kernel, Radu Sabau, Jonathan Cameron

From: Radu Sabau <radu.sabau@analog.com>

ad_sigma_delta_clear_pending_event() falls through to the status register
read path for devices with has_registers = false and no rdy_gpiod. For
such devices, ad_sd_read_reg() skips the address byte entirely and clocks
raw MISO bytes with no address phase — making it byte-for-byte identical
to reading conversion data. If a pending conversion result is present,
this partially consumes it and corrupts the data stream for the subsequent
ad_sd_read_reg() call in ad_sigma_delta_single_conversion().

Furthermore, with num_resetclks = 0 on these devices, data_read_len
evaluates to 0. If the clocked byte has bit 7 clear, pending_event is set
and the code attempts memset(data + 2, 0xff, 0 - 1), overflowing to
SIZE_MAX and corrupting the heap.

Fix by returning 0 immediately when neither rdy_gpiod nor has_registers
is set. This is safe for all current registerless devices: ad7191 and
ad7780 (with powerdown GPIO) are reset between conversions by CS
deassertion, so there is no stale result to drain; ad7780 (without
powerdown GPIO) and max11205 are continuously-converting and cycle ~DRDY
at the output data rate regardless of whether the previous result was
read, so the next falling edge fires naturally.

A future registerless device that holds ~DRDY asserted until data is read
would be broken by this early return and would require either
num_resetclks set or a rdy-gpio.

The same heap corruption is reachable on any device with rdy_gpiod set
but num_resetclks = 0: if the GPIO indicates a pending event, the drain
path executes memset(data + 2, 0xff, 0 - 1) regardless of has_registers.
Add an explicit data_read_len == 0 guard after the pending event check;
the stale result is then consumed by the first ad_sd_read_reg() call in
ad_sigma_delta_single_conversion().

Signed-off-by: Radu Sabau <radu.sabau@analog.com>
---
 drivers/iio/adc/ad_sigma_delta.c | 31 +++++++++++++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/adc/ad_sigma_delta.c b/drivers/iio/adc/ad_sigma_delta.c
index 651ade67ad2e..1b410291da53 100644
--- a/drivers/iio/adc/ad_sigma_delta.c
+++ b/drivers/iio/adc/ad_sigma_delta.c
@@ -262,11 +262,25 @@ static int ad_sigma_delta_clear_pending_event(struct ad_sigma_delta *sigma_delta
 
 	/*
 	 * Read R̅D̅Y̅ pin (if possible) or status register to check if there is an
-	 * old event.
+	 * old event. For devices with neither an RDY GPIO nor registers,
+	 * ad_sd_read_reg() transmits no address byte and clocks raw MISO bytes,
+	 * which is indistinguishable from reading conversion data and would
+	 * partially consume a pending result. Skip the check for such devices.
+	 *
+	 * This is safe for all current registerless devices: ad7191 and ad7780
+	 * (with powerdown GPIO) are reset between conversions by CS deassertion,
+	 * so there is no stale result to drain; ad7780 (without powerdown GPIO)
+	 * and max11205 are continuously-converting and cycle ~DRDY at the output
+	 * data rate regardless of whether the previous result was read, so the
+	 * next falling edge fires naturally.
+	 *
+	 * A future registerless device that holds ~DRDY asserted until data is
+	 * read would be broken by this early return and would need either
+	 * num_resetclks set or a rdy-gpio.
 	 */
 	if (sigma_delta->rdy_gpiod) {
 		pending_event = gpiod_get_value(sigma_delta->rdy_gpiod);
-	} else {
+	} else if (sigma_delta->info->has_registers) {
 		unsigned int status_reg;
 
 		ret = ad_sd_read_reg(sigma_delta, AD_SD_REG_STATUS, 1, &status_reg);
@@ -274,11 +288,24 @@ static int ad_sigma_delta_clear_pending_event(struct ad_sigma_delta *sigma_delta
 			return ret;
 
 		pending_event = !(status_reg & AD_SD_REG_STATUS_RDY);
+	} else {
+		return 0;
 	}
 
 	if (!pending_event)
 		return 0;
 
+	/*
+	 * With num_resetclks = 0, data_read_len is 0 and the drain sequence
+	 * below would compute memset(data + 2, 0xff, 0 - 1), underflowing to
+	 * SIZE_MAX and corrupting the heap. There is no safe way to drain the
+	 * stale result without knowing the data register size; it will be
+	 * consumed by the first ad_sd_read_reg() call in
+	 * ad_sigma_delta_single_conversion().
+	 */
+	if (!data_read_len)
+		return 0;
+
 	/*
 	 * In general the size of the data register is unknown. It varies from
 	 * device to device, might be one byte longer if CONTROL.DATA_STATUS is

-- 
2.43.0



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v5 0/2] iio: adc: ad_sigma_delta: fix CS assertion and registerless device handling
  2026-05-27  9:38 [PATCH v5 0/2] iio: adc: ad_sigma_delta: fix CS assertion and registerless device handling Radu Sabau via B4 Relay
  2026-05-27  9:38 ` [PATCH v5 1/2] iio: adc: ad_sigma_delta: fix CS held asserted and state leaks Radu Sabau via B4 Relay
  2026-05-27  9:38 ` [PATCH v5 2/2] iio: adc: ad_sigma_delta: fix clear_pending_event for registerless devices Radu Sabau via B4 Relay
@ 2026-05-27 11:16 ` Jonathan Cameron
  2026-05-27 11:18   ` Jonathan Cameron
  2 siblings, 1 reply; 8+ messages in thread
From: Jonathan Cameron @ 2026-05-27 11:16 UTC (permalink / raw)
  To: Radu Sabau via B4 Relay
  Cc: radu.sabau, Lars-Peter Clausen, Michael Hennerich, David Lechner,
	Nuno Sá,
	Andy Shevchenko, Uwe Kleine-König, linux-iio, linux-kernel

On Wed, 27 May 2026 12:38:37 +0300
Radu Sabau via B4 Relay <devnull+radu.sabau.analog.com@kernel.org> wrote:

> This series fixes two independent bugs in the ad_sigma_delta framework.
> 
> Patch 1 fixes CS being left permanently asserted after single conversion
> and in the error path of ad_sd_buffer_postenable(). In
> ad_sigma_delta_single_conversion(), set_mode(AD_SD_MODE_IDLE) and
> disable_one() were executing while keep_cs_asserted was still true,
> causing any SPI transfer they issued to carry cs_change=1. The
> postenable() error path also failed to call set_mode(AD_SD_MODE_IDLE),
> leaving the device in continuous conversion mode with bus_locked
> incorrectly set, opening a window for concurrent SPI access.
> 
> Patch 2 fixes ad_sigma_delta_clear_pending_event() for devices with                                     
> has_registers = false and no rdy_gpiod (currently AD7191, AD7780, and
> MAX11205). These devices fall through to the status register read path,                                 
> but since has_registers is false, ad_sd_read_reg() transmits no address                               
> byte and blindly clocks raw MISO bytes — indistinguishable from reading
> conversion data, partially consuming any pending result and corrupting the
> stream. With num_resetclks = 0 on these devices a further hazard exists:
> if pending_event is set, the drain path attempts memset of SIZE_MAX bytes,
> corrupting the heap. The fix returns 0 immediately for registerless
> devices. This is safe for all current instances: AD7191 and AD7780 (with
> powerdown GPIO) are reset between conversions by CS deassertion; AD7780
> (without powerdown GPIO) and MAX11205 are continuously-converting and
> cycle ~DRDY regardless, so the next falling edge fires naturally. A future
> registerless device that holds ~DRDY asserted until data is read would
> need num_resetclks set or a rdy-gpio instead. The same heap corruption can
> be triggered on any device with rdy_gpiod set but num_resetclks = 0, so
> an explicit data_read_len == 0 guard is added independently.
> 
> Signed-off-by: Radu Sabau <radu.sabau@analog.com>
Hi Radu,

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

Note that as this is all a bit fiddly in the ideal world I'd like some
more eyes on this and will be happy to add tags or indeed pull the patch
in response to any reviews in the next few days.

Sashiko is now 'happy' I think and it found a lot more issues than I identified
in earlier versions.

Thanks,

Jonathan

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v5 0/2] iio: adc: ad_sigma_delta: fix CS assertion and registerless device handling
  2026-05-27 11:16 ` [PATCH v5 0/2] iio: adc: ad_sigma_delta: fix CS assertion and registerless device handling Jonathan Cameron
@ 2026-05-27 11:18   ` Jonathan Cameron
  2026-05-29  8:58     ` Jonathan Cameron
  0 siblings, 1 reply; 8+ messages in thread
From: Jonathan Cameron @ 2026-05-27 11:18 UTC (permalink / raw)
  To: Radu Sabau via B4 Relay
  Cc: radu.sabau, Lars-Peter Clausen, Michael Hennerich, David Lechner,
	Nuno Sá,
	Andy Shevchenko, Uwe Kleine-König, linux-iio, linux-kernel

On Wed, 27 May 2026 12:16:48 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Wed, 27 May 2026 12:38:37 +0300
> Radu Sabau via B4 Relay <devnull+radu.sabau.analog.com@kernel.org> wrote:
> 
> > This series fixes two independent bugs in the ad_sigma_delta framework.
> > 
> > Patch 1 fixes CS being left permanently asserted after single conversion
> > and in the error path of ad_sd_buffer_postenable(). In
> > ad_sigma_delta_single_conversion(), set_mode(AD_SD_MODE_IDLE) and
> > disable_one() were executing while keep_cs_asserted was still true,
> > causing any SPI transfer they issued to carry cs_change=1. The
> > postenable() error path also failed to call set_mode(AD_SD_MODE_IDLE),
> > leaving the device in continuous conversion mode with bus_locked
> > incorrectly set, opening a window for concurrent SPI access.
> > 
> > Patch 2 fixes ad_sigma_delta_clear_pending_event() for devices with                                     
> > has_registers = false and no rdy_gpiod (currently AD7191, AD7780, and
> > MAX11205). These devices fall through to the status register read path,                                 
> > but since has_registers is false, ad_sd_read_reg() transmits no address                               
> > byte and blindly clocks raw MISO bytes — indistinguishable from reading
> > conversion data, partially consuming any pending result and corrupting the
> > stream. With num_resetclks = 0 on these devices a further hazard exists:
> > if pending_event is set, the drain path attempts memset of SIZE_MAX bytes,
> > corrupting the heap. The fix returns 0 immediately for registerless
> > devices. This is safe for all current instances: AD7191 and AD7780 (with
> > powerdown GPIO) are reset between conversions by CS deassertion; AD7780
> > (without powerdown GPIO) and MAX11205 are continuously-converting and
> > cycle ~DRDY regardless, so the next falling edge fires naturally. A future
> > registerless device that holds ~DRDY asserted until data is read would
> > need num_resetclks set or a rdy-gpio instead. The same heap corruption can
> > be triggered on any device with rdy_gpiod set but num_resetclks = 0, so
> > an explicit data_read_len == 0 guard is added independently.
> > 
> > Signed-off-by: Radu Sabau <radu.sabau@analog.com>  
> Hi Radu,
> 
> Applied to the fixes-togreg branch of iio.git and marked for stable.
> 
> Note that as this is all a bit fiddly in the ideal world I'd like some
> more eyes on this and will be happy to add tags or indeed pull the patch
> in response to any reviews in the next few days.
> 
> Sashiko is now 'happy' I think and it found a lot more issues than I identified
> in earlier versions.
> 
Actually scratch that - these both need Fixes tags.  Please reply to each email
with whatever seems most likely.  I know it can be hard to find the point where
a complex bug got introduced but we should still be providing some guidance
on how far to backport.

Thanks,

Jonathan

> Thanks,
> 
> Jonathan


^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH v5 1/2] iio: adc: ad_sigma_delta: fix CS held asserted and state leaks
  2026-05-27  9:38 ` [PATCH v5 1/2] iio: adc: ad_sigma_delta: fix CS held asserted and state leaks Radu Sabau via B4 Relay
@ 2026-05-29  8:38   ` Sabau, Radu bogdan
  0 siblings, 0 replies; 8+ messages in thread
From: Sabau, Radu bogdan @ 2026-05-29  8:38 UTC (permalink / raw)
  To: Sabau, Radu bogdan, Lars-Peter Clausen, Hennerich, Michael,
	Jonathan Cameron, David Lechner, Sa, Nuno, Andy Shevchenko,
	Uwe Kleine König
  Cc: linux-iio, linux-kernel

> -----Original Message-----
> From: Radu Sabau via B4 Relay <devnull+radu.sabau.analog.com@kernel.org>
> Sent: Wednesday, May 27, 2026 12:39 PM
> To: Lars-Peter Clausen <lars@metafoo.de>; Hennerich, Michael
> <Michael.Hennerich@analog.com>; Jonathan Cameron <jic23@kernel.org>;
> David Lechner <dlechner@baylibre.com>; Sa, Nuno <Nuno.Sa@analog.com>;
> Andy Shevchenko <andy@kernel.org>; Uwe Kleine König <u.kleine-
> koenig@baylibre.com>
> Cc: linux-iio@vger.kernel.org; linux-kernel@vger.kernel.org; Sabau, Radu
> bogdan <Radu.Sabau@analog.com>; Jonathan Cameron <jic23@kernel.org>
> Subject: [PATCH v5 1/2] iio: adc: ad_sigma_delta: fix CS held asserted and
> state leaks
> 
> From: Radu Sabau <radu.sabau@analog.com>
> 
> In ad_sigma_delta_single_conversion(), set_mode(AD_SD_MODE_IDLE) and
> disable_one() were called from the out: block while keep_cs_asserted
> was still true. This caused any SPI transfer issued by those callbacks
> to carry cs_change=1, leaving CS permanently asserted after the
> conversion. Fix by moving both calls into the out_unlock: block, after
> keep_cs_asserted is cleared, matching the pattern already used in
> ad_sd_calibrate().
> 
> In the error path of ad_sd_buffer_postenable(), if an operation fails
> after set_mode(AD_SD_MODE_CONTINUOUS) has already succeeded (e.g.
> spi_offload_trigger_enable()), the device is left in continuous
> conversion mode with CS physically asserted. Additionally,
> bus_locked remaining true after spi_bus_unlock() causes subsequent
> SPI operations to call spi_sync_locked() without the bus lock actually
> held, allowing concurrent SPI access.
> 
> Fix the error path by clearing keep_cs_asserted first, then calling
> set_mode(AD_SD_MODE_IDLE) to revert the device mode and deassert CS,
> then clearing bus_locked before releasing the bus.
> 
> For devices that implement neither set_mode nor disable_one (such as
> MAX11205, which has no physical CS pin), no SPI transfer is issued
> during cleanup and the cs_change flag has no effect on any physical
> line.
> 

Fixes tag needed

Fixes: 132d44dc6966 ("iio: adc: ad_sigma_delta: Check for previous ready signals")

That commit moved keep_cs_asserted = false to the new out_unlock: label that runs
after set_mode(IDLE) and disable_one(). It also introduced the err_unlock: path in
ad_sd_buffer_postenable() without cleanup of those flags.

> Signed-off-by: Radu Sabau <radu.sabau@analog.com>
> ---
>  drivers/iio/adc/ad_sigma_delta.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad_sigma_delta.c
> b/drivers/iio/adc/ad_sigma_delta.c 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH v5 2/2] iio: adc: ad_sigma_delta: fix clear_pending_event for registerless devices
  2026-05-27  9:38 ` [PATCH v5 2/2] iio: adc: ad_sigma_delta: fix clear_pending_event for registerless devices Radu Sabau via B4 Relay
@ 2026-05-29  8:40   ` Sabau, Radu bogdan
  0 siblings, 0 replies; 8+ messages in thread
From: Sabau, Radu bogdan @ 2026-05-29  8:40 UTC (permalink / raw)
  To: Sabau, Radu bogdan, Lars-Peter Clausen, Hennerich, Michael,
	Jonathan Cameron, David Lechner, Sa, Nuno, Andy Shevchenko,
	Uwe Kleine König
  Cc: linux-iio, linux-kernel

> -----Original Message-----
> From: Radu Sabau via B4 Relay <devnull+radu.sabau.analog.com@kernel.org>
> Sent: Wednesday, May 27, 2026 12:39 PM
> To: Lars-Peter Clausen <lars@metafoo.de>; Hennerich, Michael
> <Michael.Hennerich@analog.com>; Jonathan Cameron <jic23@kernel.org>;
> David Lechner <dlechner@baylibre.com>; Sa, Nuno <Nuno.Sa@analog.com>;
> Andy Shevchenko <andy@kernel.org>; Uwe Kleine König <u.kleine-
> koenig@baylibre.com>
> Cc: linux-iio@vger.kernel.org; linux-kernel@vger.kernel.org; Sabau, Radu
> bogdan <Radu.Sabau@analog.com>; Jonathan Cameron <jic23@kernel.org>
> Subject: [PATCH v5 2/2] iio: adc: ad_sigma_delta: fix clear_pending_event for
> registerless devices
> 
> From: Radu Sabau <radu.sabau@analog.com>
> 
> ad_sigma_delta_clear_pending_event() falls through to the status register
> read path for devices with has_registers = false and no rdy_gpiod. For
> such devices, ad_sd_read_reg() skips the address byte entirely and clocks
> raw MISO bytes with no address phase — making it byte-for-byte identical
> to reading conversion data. If a pending conversion result is present,
> this partially consumes it and corrupts the data stream for the subsequent
> ad_sd_read_reg() call in ad_sigma_delta_single_conversion().
> 
> Furthermore, with num_resetclks = 0 on these devices, data_read_len
> evaluates to 0. If the clocked byte has bit 7 clear, pending_event is set
> and the code attempts memset(data + 2, 0xff, 0 - 1), overflowing to
> SIZE_MAX and corrupting the heap.
> 
> Fix by returning 0 immediately when neither rdy_gpiod nor has_registers
> is set. This is safe for all current registerless devices: ad7191 and
> ad7780 (with powerdown GPIO) are reset between conversions by CS
> deassertion, so there is no stale result to drain; ad7780 (without
> powerdown GPIO) and max11205 are continuously-converting and cycle
> ~DRDY
> at the output data rate regardless of whether the previous result was
> read, so the next falling edge fires naturally.
> 
> A future registerless device that holds ~DRDY asserted until data is read
> would be broken by this early return and would require either
> num_resetclks set or a rdy-gpio.
> 
> The same heap corruption is reachable on any device with rdy_gpiod set
> but num_resetclks = 0: if the GPIO indicates a pending event, the drain
> path executes memset(data + 2, 0xff, 0 - 1) regardless of has_registers.
> Add an explicit data_read_len == 0 guard after the pending event check;
> the stale result is then consumed by the first ad_sd_read_reg() call in
> ad_sigma_delta_single_conversion().

Fixes tag needed

Fixes: 132d44dc6966 ("iio: adc: ad_sigma_delta: Check for previous ready signals")

The same commit introduced ad_sigma_delta_clear_pending_event() itself with an
unconditional else that calls ad_sd_read_reg() for any device without rdy_gpiod,
regardless of has_registers.

> 
> Signed-off-by: Radu Sabau <radu.sabau@analog.com>
> --- 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v5 0/2] iio: adc: ad_sigma_delta: fix CS assertion and registerless device handling
  2026-05-27 11:18   ` Jonathan Cameron
@ 2026-05-29  8:58     ` Jonathan Cameron
  0 siblings, 0 replies; 8+ messages in thread
From: Jonathan Cameron @ 2026-05-29  8:58 UTC (permalink / raw)
  To: Radu Sabau via B4 Relay
  Cc: radu.sabau, Lars-Peter Clausen, Michael Hennerich, David Lechner,
	Nuno Sá,
	Andy Shevchenko, Uwe Kleine-König, linux-iio, linux-kernel

On Wed, 27 May 2026 12:18:41 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Wed, 27 May 2026 12:16:48 +0100
> Jonathan Cameron <jic23@kernel.org> wrote:
> 
> > On Wed, 27 May 2026 12:38:37 +0300
> > Radu Sabau via B4 Relay <devnull+radu.sabau.analog.com@kernel.org> wrote:
> >   
> > > This series fixes two independent bugs in the ad_sigma_delta framework.
> > > 
> > > Patch 1 fixes CS being left permanently asserted after single conversion
> > > and in the error path of ad_sd_buffer_postenable(). In
> > > ad_sigma_delta_single_conversion(), set_mode(AD_SD_MODE_IDLE) and
> > > disable_one() were executing while keep_cs_asserted was still true,
> > > causing any SPI transfer they issued to carry cs_change=1. The
> > > postenable() error path also failed to call set_mode(AD_SD_MODE_IDLE),
> > > leaving the device in continuous conversion mode with bus_locked
> > > incorrectly set, opening a window for concurrent SPI access.
> > > 
> > > Patch 2 fixes ad_sigma_delta_clear_pending_event() for devices with                                     
> > > has_registers = false and no rdy_gpiod (currently AD7191, AD7780, and
> > > MAX11205). These devices fall through to the status register read path,                                 
> > > but since has_registers is false, ad_sd_read_reg() transmits no address                               
> > > byte and blindly clocks raw MISO bytes — indistinguishable from reading
> > > conversion data, partially consuming any pending result and corrupting the
> > > stream. With num_resetclks = 0 on these devices a further hazard exists:
> > > if pending_event is set, the drain path attempts memset of SIZE_MAX bytes,
> > > corrupting the heap. The fix returns 0 immediately for registerless
> > > devices. This is safe for all current instances: AD7191 and AD7780 (with
> > > powerdown GPIO) are reset between conversions by CS deassertion; AD7780
> > > (without powerdown GPIO) and MAX11205 are continuously-converting and
> > > cycle ~DRDY regardless, so the next falling edge fires naturally. A future
> > > registerless device that holds ~DRDY asserted until data is read would
> > > need num_resetclks set or a rdy-gpio instead. The same heap corruption can
> > > be triggered on any device with rdy_gpiod set but num_resetclks = 0, so
> > > an explicit data_read_len == 0 guard is added independently.
> > > 
> > > Signed-off-by: Radu Sabau <radu.sabau@analog.com>    
> > Hi Radu,
> > 
> > Applied to the fixes-togreg branch of iio.git and marked for stable.
> > 
> > Note that as this is all a bit fiddly in the ideal world I'd like some
> > more eyes on this and will be happy to add tags or indeed pull the patch
> > in response to any reviews in the next few days.
> > 
> > Sashiko is now 'happy' I think and it found a lot more issues than I identified
> > in earlier versions.
> >   
> Actually scratch that - these both need Fixes tags.  Please reply to each email
> with whatever seems most likely.  I know it can be hard to find the point where
> a complex bug got introduced but we should still be providing some guidance
> on how far to backport.
> 
Thanks for the tags, applied to the fixes-togreg branch of iio.git and marked
for stable.

Jonathan

> Thanks,
> 
> Jonathan
> 
> > Thanks,
> > 
> > Jonathan  
> 
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-05-29  8:58 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-05-27  9:38 [PATCH v5 0/2] iio: adc: ad_sigma_delta: fix CS assertion and registerless device handling Radu Sabau via B4 Relay
2026-05-27  9:38 ` [PATCH v5 1/2] iio: adc: ad_sigma_delta: fix CS held asserted and state leaks Radu Sabau via B4 Relay
2026-05-29  8:38   ` Sabau, Radu bogdan
2026-05-27  9:38 ` [PATCH v5 2/2] iio: adc: ad_sigma_delta: fix clear_pending_event for registerless devices Radu Sabau via B4 Relay
2026-05-29  8:40   ` Sabau, Radu bogdan
2026-05-27 11:16 ` [PATCH v5 0/2] iio: adc: ad_sigma_delta: fix CS assertion and registerless device handling Jonathan Cameron
2026-05-27 11:18   ` Jonathan Cameron
2026-05-29  8:58     ` Jonathan Cameron

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®