Received-SPF: pass (google.com: domain of linux-kernel+bounces-14934-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Precedence: bulk
MIME-Version: 1.0
References: <1704157631-3814-1-git-send-email-zhouzhouyi@gmail.com> <20240102194209.39dac963@jic23-huawei>
In-Reply-To: <20240102194209.39dac963@jic23-huawei>
From: Zhouyi Zhou <zhouzhouyi@gmail.com>
Date: Wed, 3 Jan 2024 06:21:46 +0800
Message-ID: <CAABZP2y1ptNm2KMQ7tGttMgFFCMPS0=CjyfxYJkR8P1thEWA9Q@mail.gmail.com>
Subject: Re: [PATCH v3] iio: magnetometer: rm3100: add boundary check for the
 value read from RM3100_REG_TMRC
To: Jonathan Cameron <jic23@kernel.org>
Cc: songqiang1304521@gmail.com, lars@metafoo.de, linux-iio@vger.kernel.org, 
	linux-kernel@vger.kernel.org, "zhili.liu" <zhili.liu@ucas.com.cn>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Wed, Jan 3, 2024 at 3:42=E2=80=AFAM Jonathan Cameron <jic23@kernel.org> =
wrote:
>
> On Tue,  2 Jan 2024 09:07:11 +0800
> zhouzhouyi@gmail.com wrote:
>
> > From: "zhili.liu" <zhili.liu@ucas.com.cn>
> >
> > Recently, we encounter kernel crash in function rm3100_common_probe
> > caused by out of bound access of array rm3100_samp_rates (because of
> > underlying hardware failures). Add boundary check to prevent out of
> > bound access.
> >
> > Fixes: 121354b2eceb ("iio: magnetometer: Add driver support for PNI RM3=
100")
> Fixes is a formal tag so needs to be part of the main tags block
> (i.e. No blank line here!)
>
> I'll fix that whilst applying.
Thank you for helping us modify the patch, I learned a lot during this
process, thanks
>
> >
> > Suggested-by: Zhouyi Zhou <zhouzhouyi@gmail.com>
> > Signed-off-by: zhili.liu <zhili.liu@ucas.com.cn>
> > ---
> >  drivers/iio/magnetometer/rm3100-core.c | 9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/iio/magnetometer/rm3100-core.c b/drivers/iio/magne=
tometer/rm3100-core.c
> > index 69938204456f..12c2e3b0aeb6 100644
> > --- a/drivers/iio/magnetometer/rm3100-core.c
> > +++ b/drivers/iio/magnetometer/rm3100-core.c
> > @@ -530,6 +530,7 @@ int rm3100_common_probe(struct device *dev, struct =
regmap *regmap, int irq)
> >       struct rm3100_data *data;
> >       unsigned int tmp;
> >       int ret;
> > +     int samp_rate_index;
> >
> >       indio_dev =3D devm_iio_device_alloc(dev, sizeof(*data));
> >       if (!indio_dev)
> > @@ -586,8 +587,14 @@ int rm3100_common_probe(struct device *dev, struct=
 regmap *regmap, int irq)
> >       ret =3D regmap_read(regmap, RM3100_REG_TMRC, &tmp);
> >       if (ret < 0)
> >               return ret;
> > +
> > +     samp_rate_index =3D tmp - RM3100_TMRC_OFFSET;
> > +     if (samp_rate_index < 0 || samp_rate_index >=3D  RM3100_SAMP_NUM)=
 {
> > +             dev_err(dev, "The value read from RM3100_REG_TMRC is inva=
lid!\n");
> > +             return -EINVAL;
> > +     }
> >       /* Initializing max wait time, which is double conversion time. *=
/
> > -     data->conversion_time =3D rm3100_samp_rates[tmp - RM3100_TMRC_OFF=
SET][2]
> > +     data->conversion_time =3D rm3100_samp_rates[samp_rate_index][2]
> >                               * 2;
> I've rewrapped this to be on one line whilst applying. Make sure to check=
 for
> side effects like this when updating code.  If we don't tidy it up at the=
 time
> we end up with gradually worse formatting over a long period!
Thank you for fixing that for us.
>
> Applied to the fixes-togreg branch of iio.git and marked for stable.
I saw the commit on the fixes-togreg branch of iio.git, exciting!
I learned a lot from it ;-)

Thanks again
Thanks,
Zhouyi
>
> Thanks,
>
> Jonathan
>
> >
> >       /* Cycle count values may not be what we want. */
>