* [PATCH] iio: filter: admv8818: fix out-of-bounds read in __admv8818_read_[h|l]pf_freq()
@ 2022-09-22 11:58 Wei Yongjun
2022-09-24 16:53 ` Jonathan Cameron
0 siblings, 1 reply; 4+ messages in thread
From: Wei Yongjun @ 2022-09-22 11:58 UTC (permalink / raw)
To: linux-iio
Cc: Wei Yongjun, Antoniu Miclaus, Lars-Peter Clausen,
Michael Hennerich, Jonathan Cameron
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>
---
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;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] iio: filter: admv8818: fix out-of-bounds read in __admv8818_read_[h|l]pf_freq()
2022-09-22 11:58 [PATCH] iio: filter: admv8818: fix out-of-bounds read in __admv8818_read_[h|l]pf_freq() Wei Yongjun
@ 2022-09-24 16:53 ` Jonathan Cameron
2022-09-26 6:20 ` Miclaus, Antoniu
0 siblings, 1 reply; 4+ messages in thread
From: Jonathan Cameron @ 2022-09-24 16:53 UTC (permalink / raw)
To: Wei Yongjun
Cc: linux-iio, Wei Yongjun, Antoniu Miclaus, Lars-Peter Clausen,
Michael Hennerich
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>
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;
> }
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH] iio: filter: admv8818: fix out-of-bounds read in __admv8818_read_[h|l]pf_freq()
2022-09-24 16:53 ` Jonathan Cameron
@ 2022-09-26 6:20 ` Miclaus, Antoniu
2022-10-02 11:28 ` Jonathan Cameron
0 siblings, 1 reply; 4+ messages in thread
From: Miclaus, Antoniu @ 2022-09-26 6:20 UTC (permalink / raw)
To: Jonathan Cameron, Wei Yongjun
Cc: linux-iio, Wei Yongjun, Lars-Peter Clausen, Hennerich, Michael
> -----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>
>
> 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;
> > }
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] iio: filter: admv8818: fix out-of-bounds read in __admv8818_read_[h|l]pf_freq()
2022-09-26 6:20 ` Miclaus, Antoniu
@ 2022-10-02 11:28 ` Jonathan Cameron
0 siblings, 0 replies; 4+ messages in thread
From: Jonathan Cameron @ 2022-10-02 11:28 UTC (permalink / raw)
To: Miclaus, Antoniu
Cc: Wei Yongjun, linux-iio, Wei Yongjun, Lars-Peter Clausen,
Hennerich, Michael
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;
> > > }
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-10-02 11:28 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-22 11:58 [PATCH] iio: filter: admv8818: fix out-of-bounds read in __admv8818_read_[h|l]pf_freq() Wei Yongjun
2022-09-24 16:53 ` Jonathan Cameron
2022-09-26 6:20 ` Miclaus, Antoniu
2022-10-02 11:28 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).