Re: [PATCH v5 04/10] iio: adc: ti-ads1262: support per-channel sampling frequency

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] iio: adc: ti-ads1262: out-of-bounds access on st->channels for timestamp channel
--

commit 7618e3d1bfd0f75640c3a335d284574aab75861b
Author: Kurt Borja <kuurtb@gmail.com>

iio: adc: ti-ads1262: support per-channel sampling frequency

This commit adds per-channel sampling frequency support to the
ti-ads1262 driver. It also assigns the "available" attribute
per-channel in preparation for per-filter availability support.

> diff --git a/drivers/iio/adc/ti-ads1262.c b/drivers/iio/adc/ti-ads1262.c
> index a54f78e88012c..7a6fa009ddadc 100644
> --- a/drivers/iio/adc/ti-ads1262.c
> +++ b/drivers/iio/adc/ti-ads1262.c
[ ... ]
> @@ -436,11 +513,19 @@ static int ads1262_wait_for_conversion(struct ads1262 *st)
>  static int ads1262_channel_enable(struct ads1262 *st,
>  				  const struct iio_chan_spec *spec)
>  {
> +	struct ads1262_channel *chan = &st->channels[spec->scan_index];
> +	int ret;
> +

[Severity: High]
Does this cause an out-of-bounds array access if this function is called for
the soft timestamp channel?

Because st->channels is allocated without space for the timestamp channel,
the timestamp channel is assigned a scan_index equal to st->num_channels,
which is one element past the end of the array.

When triggered buffer sampling is introduced in the follow-up patches,
iio_for_each_active_channel() iterates over all bits in the active scan mask,
including the soft timestamp channel if enabled by userspace.

If the timestamp channel is passed to the preparation functions and reaches
here, it looks like this will read out-of-bounds to get chan->data_rate,
and then send corrupted configuration commands to the hardware via
regmap_update_bits().

>  	guard(mutex)(&st->xfer_lock);
>  	guard(mutex)(&st->chan_lock);
>  
> +	ret = regmap_update_bits(st->regmap, ADS1262_MODE2_REG,
> +				 ADS1262_MODE2_DR_MASK,
> +				 FIELD_PREP(ADS1262_MODE2_DR_MASK, chan->data_rate));
> +	if (ret)
> +		return ret;
> +
>  	return regmap_update_bits(st->regmap, ADS1262_INPMUX_REG,
>  				  ADS1262_INPMUX_MUXN_MASK |
>  				  ADS1262_INPMUX_MUXP_MASK,
[ ... ]
> @@ -744,9 +951,18 @@ static int ads1262_parse_channels(struct iio_dev *indio_dev)
>  	if (!chan_specs)
>  		return -ENOMEM;
>  
> +	st->num_channels = num_fw_channels + ADS1262_MON_CHANNEL_COUNT;
> +	st->channels = devm_kcalloc(dev, st->num_channels, sizeof(*st->channels),
> +				    GFP_KERNEL);
> +	if (!st->channels)
> +		return -ENOMEM;
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261010-ads126x-v5-0-49e331cb2fed@gmail.com?part=4