Re: [PATCH v6 2/4] iio: adc: ltc2378: Add support for LTC2378-20 and similar ADCs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The internal mutex `st->lock` is released before reading the shared SPI transfer buffer, failing to protect the single-sample read procedure.
--

--- Patch [2]: [PATCH v6 2/4] iio: adc: ltc2378: Add support for LTC2378-20 and similar ADCs ---
commit 30c9caf90bdc7e619336a9deac3d3aa86c429417
Author: Marcelo Schmitt <marcelo.schmitt@analog.com>

iio: adc: ltc2378: Add support for LTC2378-20 and similar ADCs

Support for LTC2378-20 and similar analog-to-digital converters.

> diff --git a/drivers/iio/adc/ltc2378.c b/drivers/iio/adc/ltc2378.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/iio/adc/ltc2378.c

[ ... ]

> +static int ltc2378_convert_and_acquire(struct ltc2378_state *st)
> +{
> +	int ret;
> +
> +	guard(mutex)(&st->lock);

[Severity: Medium]
Does the use of guard(mutex) here release the lock before the data is safely
extracted?

The lock is automatically released when this function returns, which means
st->lock drops before ltc2378_channel_single_read() can consume the shared
st->scan.data buffer.

> +	/* Cause a rising edge of CNV to initiate a new ADC conversion */
> +	gpiod_set_value_cansleep(st->cnv_gpio, 1);
> +	fsleep(4);
> +	ret = spi_sync_transfer(st->spi, &st->xfer, 1);
> +	gpiod_set_value_cansleep(st->cnv_gpio, 0);
> +
> +	return ret;
> +}
> +
> +static int ltc2378_channel_single_read(const struct iio_chan_spec *chan,
> +				       struct ltc2378_state *st, int *val)
> +{
> +	const struct iio_scan_type *scan_type = &chan->scan_type;
> +	u32 sample;
> +	int ret;
> +
> +	ret = ltc2378_convert_and_acquire(st);
> +	if (ret)
> +		return ret;
> +
> +	if (chan->scan_type.endianness == IIO_BE) {
> +		if (chan->scan_type.realbits > 16)
> +			sample = be32_to_cpu(st->scan.data.sample_buf32_be);

[Severity: Medium]
Is this read of st->scan.data properly protected against concurrent access?

Since ltc2378_convert_and_acquire() uses guard(mutex) to manage the lock,
the lock is dropped before returning. This leaves the access to
st->scan.data here unprotected by st->lock. 

While the IIO core's direct mode mlock currently serializes these calls,
this early release seems to break the intent stated in the commit message
where the single sample read procedure is protected with a mutex.

> +		else
> +			sample = be16_to_cpu(st->scan.data.sample_buf16_be);

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1783629101.git.marcelo.schmitt@analog.com?part=2