Re: [PATCH] iio: adc: ad4851: fix ad4858 chan pointer handling

[Jonathan Cameron <jic23@kernel.org>]

On Fri, 09 May 2025 13:47:41 +0100
Nuno Sá <noname.nuno@gmail.com> wrote:

> On Fri, 2025-05-09 at 13:16 +0300, Antoniu Miclaus wrote:
> > The pointer returned from ad4851_parse_channels_common() is incremented
> > internally as each channel is populated. In ad4858_parse_channels(),
> > the same pointer was further incremented while setting ext_scan_type
> > fields for each channel. This resulted in indio_dev->channels being set
> > to a pointer past the end of the allocated array, potentially causing
> > memory corruption or undefined behavior.
> > 
> > Fix this by iterating over the channels using an explicit index instead
> > of incrementing the pointer. This preserves the original base pointer
> > and ensures all channel metadata is set correctly.
> > 
> > Fixes: 6250803fe2ec ("iio: adc: ad4851: add ad485x driver")
> > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> > ---  
> 
> LGTM
Applied to the fixes-togreg branch of iio.git.

Thanks

Jonathan

> 
> Reviewed-by: Nuno Sá <nuno.sa@analog.com>
> 
> >  drivers/iio/adc/ad4851.c | 14 +++++++-------
> >  1 file changed, 7 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/iio/adc/ad4851.c b/drivers/iio/adc/ad4851.c
> > index 98ebc853db79..f1d2e2896f2a 100644
> > --- a/drivers/iio/adc/ad4851.c
> > +++ b/drivers/iio/adc/ad4851.c
> > @@ -1034,7 +1034,7 @@ static int ad4858_parse_channels(struct iio_dev
> > *indio_dev)
> >  	struct device *dev = &st->spi->dev;
> >  	struct iio_chan_spec *ad4851_channels;
> >  	const struct iio_chan_spec ad4851_chan = AD4858_IIO_CHANNEL;
> > -	int ret;
> > +	int ret, i = 0;
> >  
> >  	ret = ad4851_parse_channels_common(indio_dev, &ad4851_channels,
> >  					   ad4851_chan);
> > @@ -1042,15 +1042,15 @@ static int ad4858_parse_channels(struct iio_dev
> > *indio_dev)
> >  		return ret;
> >  
> >  	device_for_each_child_node_scoped(dev, child) {
> > -		ad4851_channels->has_ext_scan_type = 1;
> > +		ad4851_channels[i].has_ext_scan_type = 1;
> >  		if (fwnode_property_read_bool(child, "bipolar")) {
> > -			ad4851_channels->ext_scan_type =
> > ad4851_scan_type_20_b;
> > -			ad4851_channels->num_ext_scan_type =
> > ARRAY_SIZE(ad4851_scan_type_20_b);
> > +			ad4851_channels[i].ext_scan_type =
> > ad4851_scan_type_20_b;
> > +			ad4851_channels[i].num_ext_scan_type =
> > ARRAY_SIZE(ad4851_scan_type_20_b);
> >  		} else {
> > -			ad4851_channels->ext_scan_type =
> > ad4851_scan_type_20_u;
> > -			ad4851_channels->num_ext_scan_type =
> > ARRAY_SIZE(ad4851_scan_type_20_u);
> > +			ad4851_channels[i].ext_scan_type =
> > ad4851_scan_type_20_u;
> > +			ad4851_channels[i].num_ext_scan_type =
> > ARRAY_SIZE(ad4851_scan_type_20_u);
> >  		}
> > -		ad4851_channels++;
> > +		i++;
> >  	}
> >  
> >  	indio_dev->channels = ad4851_channels;  
>