* [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@vger.kernel.org, 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@vger.kernel.org, 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