On Mon, 26 Sep 2022 06:20:20 +0000
"Miclaus, Antoniu" <Antoniu.Miclaus@xxxxxxxxxx> wrote:
> > -----Original Message-----
> > From: Jonathan Cameron <jic23@xxxxxxxxxx>
> > Sent: Saturday, September 24, 2022 7:54 PM
> > To: Wei Yongjun <weiyongjun@xxxxxxxxxxxxxxx>
> > Cc: linux-iio@xxxxxxxxxxxxxxx; Wei Yongjun <weiyongjun1@xxxxxxxxxx>;
> > Miclaus, Antoniu <Antoniu.Miclaus@xxxxxxxxxx>; Lars-Peter Clausen
> > <lars@xxxxxxxxxx>; Hennerich, Michael <Michael.Hennerich@xxxxxxxxxx>
> > 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@xxxxxxxxxxxxxxx> wrote:
> >
> > > From: Wei Yongjun <weiyongjun1@xxxxxxxxxx>
> > >
> > > 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@xxxxxxxxxx>
> Reviewed-by: Antoniu Miclaus <antoniu.miclaus@xxxxxxxxxx>
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;
> > > }
>
