On Tue, 18 Feb 2025 17:17:45 -0600
David Lechner <dlechner@xxxxxxxxxxxx> wrote:
> Fix some out of bounds array access of st->channels_cfg in the ad4695
> driver. This array only has elements for voltage channels, but it was
> also being accessed for the temperature channel in a few cases causing
> reading past the end of the array.
>
> In some cases, this was harmless because the value was read but not
> used. However, the in_temp_sampling_frequency attribute shares code
> with the in_voltageY_sampling_frequency attributes and was trying to
> read the oversampling ratio from the st->channels_cfg array. This
> resulted in a garbage value being used in the calculation and the
> resulting in_temp_sampling_frequency value was incorrect.
>
> To fix, make sure we always check that we are dealing with a voltage
> channel before accessing the st->channels_cfg array and use an
> oversampling ratio of 1 for the temperature channel (multiplicative
> identity value) since that channel doesn't support oversampling.
>
> Fixes: 67d63185db79 ("iio: adc: ad4695: add offload-based oversampling support")
> Signed-off-by: David Lechner <dlechner@xxxxxxxxxxxx>
This got a little fiddly as I applied the iio_device_claim_direct() patch
first and that includes making use of osr in calibbias for the voltage
channel. I switched that back to accessing it via cfg-> now there isn't a convenient
global copy in read_raw()
So with that tweak both patches applied to the togreg branch of iio.git
Thanks,
Jonathan
> ---
> drivers/iio/adc/ad4695.c | 24 +++++++++++++++++++-----
> 1 file changed, 19 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/iio/adc/ad4695.c b/drivers/iio/adc/ad4695.c
> index 3a1a6f96480fd383d32397f4d3c979069111c5c9..8721cbd2af34c53f0cea32e307b9ef2da46b0cfb 100644
> --- a/drivers/iio/adc/ad4695.c
> +++ b/drivers/iio/adc/ad4695.c
> @@ -1035,12 +1035,14 @@ static int ad4695_read_raw(struct iio_dev *indio_dev,
> {
> struct ad4695_state *st = iio_priv(indio_dev);
> const struct iio_scan_type *scan_type;
> - struct ad4695_channel_config *cfg = &st->channels_cfg[chan->scan_index];
> - unsigned int osr = st->channels_cfg[chan->scan_index].oversampling_ratio;
> + struct ad4695_channel_config *cfg;
> unsigned int reg_val;
> int ret, tmp;
> u8 realbits;
>
> + if (chan->type == IIO_VOLTAGE)
> + cfg = &st->channels_cfg[chan->scan_index];
> +
> scan_type = iio_get_current_scan_type(indio_dev, chan);
> if (IS_ERR(scan_type))
> return PTR_ERR(scan_type);
> @@ -1169,6 +1171,10 @@ static int ad4695_read_raw(struct iio_dev *indio_dev,
> }
> case IIO_CHAN_INFO_SAMP_FREQ: {
> struct pwm_state state;
> + unsigned int osr = 1;
> +
> + if (chan->type == IIO_VOLTAGE)
> + osr = cfg->oversampling_ratio;
>
> ret = pwm_get_state_hw(st->cnv_pwm, &state);
> if (ret)
> @@ -1261,7 +1267,10 @@ static int ad4695_write_raw(struct iio_dev *indio_dev,
> {
> struct ad4695_state *st = iio_priv(indio_dev);
> unsigned int reg_val;
> - unsigned int osr = st->channels_cfg[chan->scan_index].oversampling_ratio;
> + unsigned int osr = 1;
> +
> + if (chan->type == IIO_VOLTAGE)
> + osr = st->channels_cfg[chan->scan_index].oversampling_ratio;
>
> iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> switch (mask) {
> @@ -1361,7 +1370,10 @@ static int ad4695_read_avail(struct iio_dev *indio_dev,
> },
> };
> struct ad4695_state *st = iio_priv(indio_dev);
> - unsigned int osr = st->channels_cfg[chan->scan_index].oversampling_ratio;
> + unsigned int osr = 1;
> +
> + if (chan->type == IIO_VOLTAGE)
> + osr = st->channels_cfg[chan->scan_index].oversampling_ratio;
>
> switch (mask) {
> case IIO_CHAN_INFO_CALIBSCALE:
> @@ -1713,7 +1725,7 @@ static int ad4695_probe_spi_offload(struct iio_dev *indio_dev,
>
> for (i = 0; i < indio_dev->num_channels; i++) {
> struct iio_chan_spec *chan = &st->iio_chan[i];
> - struct ad4695_channel_config *cfg = &st->channels_cfg[i];
> + struct ad4695_channel_config *cfg;
>
> /*
> * NB: When using offload support, all channels need to have the
> @@ -1734,6 +1746,8 @@ static int ad4695_probe_spi_offload(struct iio_dev *indio_dev,
> if (chan->type != IIO_VOLTAGE)
> continue;
>
> + cfg = &st->channels_cfg[i];
> +
> chan->info_mask_separate |= BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO);
> chan->info_mask_separate_available |=
> BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO);
>
