Re: [PATCH] iio: filter: admv8818: fix out-of-bounds read in __admv8818_read_[h|l]pf_freq()

[Jonathan Cameron <jic23@kernel.org>]

On Mon, 26 Sep 2022 06:20:20 +0000
"Miclaus, Antoniu" <Antoniu.Miclaus@analog.com> wrote:

> > -----Original Message-----
> > From: Jonathan Cameron <jic23@kernel.org>
> > Sent: Saturday, September 24, 2022 7:54 PM
> > To: Wei Yongjun <weiyongjun@huaweicloud.com>
> > Cc: linux-iio@vger.kernel.org; Wei Yongjun <weiyongjun1@huawei.com>;
> > Miclaus, Antoniu <Antoniu.Miclaus@analog.com>; Lars-Peter Clausen
> > <lars@metafoo.de>; Hennerich, Michael <Michael.Hennerich@analog.com>
> > Subject: Re: [PATCH] iio: filter: admv8818: fix out-of-bounds read in
> > __admv8818_read_[h|l]pf_freq()
> > 
> > [External]
> > 
> > On Thu, 22 Sep 2022 11:58:48 +0000
> > Wei Yongjun <weiyongjun@huaweicloud.com> wrote:
> >   
> > > From: Wei Yongjun <weiyongjun1@huawei.com>
> > >
> > > ADMV8818_SW_IN_WR0_MSK and ADMV8818_SW_OUT_WR0_MSK have 3  
> > bits,  
> > > which means a length of 8, but freq_range_hpf and freq_range_lpf
> > > array size is 4, may end up reading 4 elements beyond the end of
> > > those arrays.
> > >
> > > Fix to check value first before access freq_range_hpf and
> > > freq_range_lpf.
> > >
> > > Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>  
> Reviewed-by: Antoniu Miclaus <antoniu.miclaus@analog.com>

I tweaked the commit message a little to have this as hardening rather than 
a fix.  I'm fine with it being backported if that makes sense, but I don't
want it automatically picked up.

Good to harden this though.

Applied to the togreg branch of iio.git. I'll only be pushing that out as
testing until I can rebase on rc1.  In the meantime we should get 0-day
checking it etc.

Thanks,

Jonathan

> > 
> > The datasheet isn't clear on whether the register simply can't take values
> > above b100 or that the behaviour if it does is undefined.  It would take someone
> > to poke the registers on a real device to find out.  Either way, this seems to
> > be a reasonable hardening against corrupt data back from the device.
> > 
> > I will leave it on list a while longer though so it is probably 6.2 material
> > now.
> > 
> > Thanks,
> > 
> > Jonathan
> >   
> > > ---
> > >  drivers/iio/filter/admv8818.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/iio/filter/admv8818.c b/drivers/iio/filter/admv8818.c
> > > index 68de45fe21b4..fe8d46cb7f1d 100644
> > > --- a/drivers/iio/filter/admv8818.c
> > > +++ b/drivers/iio/filter/admv8818.c
> > > @@ -265,7 +265,7 @@ static int __admv8818_read_hpf_freq(struct  
> > admv8818_state *st, u64 *hpf_freq)  
> > >  		return ret;
> > >
> > >  	hpf_band = FIELD_GET(ADMV8818_SW_IN_WR0_MSK, data);
> > > -	if (!hpf_band) {
> > > +	if (!hpf_band || hpf_band > 4) {
> > >  		*hpf_freq = 0;
> > >  		return ret;
> > >  	}
> > > @@ -303,7 +303,7 @@ static int __admv8818_read_lpf_freq(struct  
> > admv8818_state *st, u64 *lpf_freq)  
> > >  		return ret;
> > >
> > >  	lpf_band = FIELD_GET(ADMV8818_SW_OUT_WR0_MSK, data);
> > > -	if (!lpf_band) {
> > > +	if (!lpf_band || lpf_band > 4) {
> > >  		*lpf_freq = 0;
> > >  		return ret;
> > >  	}  
>