lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Sun, 7 Aug 2022 04:51:07 +0000 From: Sudarsana Reddy Kalluru <skalluru@...vell.com> To: AceLan Kao <acelan.kao@...onical.com>, Igor Russkikh <irusskikh@...vell.com>, "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Dmitrii Tarakanov <Dmitrii.Tarakanov@...antia.com>, Alexander Loktionov <Alexander.Loktionov@...antia.com>, David VomLehn <vomlehn@...as.net>, Dmitry Bezrukov <Dmitry.Bezrukov@...antia.com>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org> Subject: RE: [PATCH v2] net: atlantic: fix aq_vec index out of range error > -----Original Message----- > From: AceLan Kao <acelan@...il.com> On Behalf Of AceLan Kao > Sent: Saturday, August 6, 2022 7:06 PM > To: Igor Russkikh <irusskikh@...vell.com>; David S. Miller > <davem@...emloft.net>; Eric Dumazet <edumazet@...gle.com>; Jakub > Kicinski <kuba@...nel.org>; Paolo Abeni <pabeni@...hat.com>; Dmitrii > Tarakanov <Dmitrii.Tarakanov@...antia.com>; Alexander Loktionov > <Alexander.Loktionov@...antia.com>; David VomLehn > <vomlehn@...as.net>; Dmitry Bezrukov <Dmitry.Bezrukov@...antia.com>; > netdev@...r.kernel.org; linux-kernel@...r.kernel.org > Subject: [PATCH v2] net: atlantic: fix aq_vec index out of range error > > From: "Chia-Lin Kao (AceLan)" <acelan.kao@...onical.com> > > The final update statement of the for loop exceeds the array range, the > dereference of self->aq_vec[i] is not checked and then leads to the index out > of range error. > Also fixed this kind of coding style in other for loop. > > [ 97.937604] UBSAN: array-index-out-of-bounds in > drivers/net/ethernet/aquantia/atlantic/aq_nic.c:1404:48 > [ 97.937607] index 8 is out of range for type 'aq_vec_s *[8]' > [ 97.937608] CPU: 38 PID: 3767 Comm: kworker/u256:18 Not tainted 5.19.0+ > #2 > [ 97.937610] Hardware name: Dell Inc. Precision 7865 Tower/, BIOS 1.0.0 > 06/12/2022 > [ 97.937611] Workqueue: events_unbound async_run_entry_fn > [ 97.937616] Call Trace: > [ 97.937617] <TASK> > [ 97.937619] dump_stack_lvl+0x49/0x63 > [ 97.937624] dump_stack+0x10/0x16 > [ 97.937626] ubsan_epilogue+0x9/0x3f > [ 97.937627] __ubsan_handle_out_of_bounds.cold+0x44/0x49 > [ 97.937629] ? __scm_send+0x348/0x440 > [ 97.937632] ? aq_vec_stop+0x72/0x80 [atlantic] > [ 97.937639] aq_nic_stop+0x1b6/0x1c0 [atlantic] > [ 97.937644] aq_suspend_common+0x88/0x90 [atlantic] > [ 97.937648] aq_pm_suspend_poweroff+0xe/0x20 [atlantic] > [ 97.937653] pci_pm_suspend+0x7e/0x1a0 > [ 97.937655] ? pci_pm_suspend_noirq+0x2b0/0x2b0 > [ 97.937657] dpm_run_callback+0x54/0x190 > [ 97.937660] __device_suspend+0x14c/0x4d0 > [ 97.937661] async_suspend+0x23/0x70 > [ 97.937663] async_run_entry_fn+0x33/0x120 > [ 97.937664] process_one_work+0x21f/0x3f0 > [ 97.937666] worker_thread+0x4a/0x3c0 > [ 97.937668] ? process_one_work+0x3f0/0x3f0 > [ 97.937669] kthread+0xf0/0x120 > [ 97.937671] ? kthread_complete_and_exit+0x20/0x20 > [ 97.937672] ret_from_fork+0x22/0x30 > [ 97.937676] </TASK> > > v2. fixed "warning: variable 'aq_vec' set but not used" > > Fixes: 97bde5c4f909 ("net: ethernet: aquantia: Support for NIC-specific > code") > Signed-off-by: Chia-Lin Kao (AceLan) <acelan.kao@...onical.com> > --- > drivers/net/ethernet/aquantia/atlantic/aq_nic.c | 17 +++++++---------- > 1 file changed, 7 insertions(+), 10 deletions(-) > > diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c > b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c > index e11cc29d3264..6986f0080072 100644 > --- a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c > +++ b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c > @@ -265,12 +265,10 @@ static void aq_nic_service_timer_cb(struct > timer_list *t) static void aq_nic_polling_timer_cb(struct timer_list *t) { > struct aq_nic_s *self = from_timer(self, t, polling_timer); > - struct aq_vec_s *aq_vec = NULL; > unsigned int i = 0U; > > - for (i = 0U, aq_vec = self->aq_vec[0]; > - self->aq_vecs > i; ++i, aq_vec = self->aq_vec[i]) > - aq_vec_isr(i, (void *)aq_vec); > + for (i = 0U; self->aq_vecs > i; ++i) > + aq_vec_isr(i, (void *)self->aq_vec[i]); > > mod_timer(&self->polling_timer, jiffies + > AQ_CFG_POLLING_TIMER_INTERVAL); > @@ -1065,10 +1063,11 @@ u64 *aq_nic_get_stats(struct aq_nic_s *self, u64 > *data) > > for (tc = 0U; tc < self->aq_nic_cfg.tcs; tc++) { > for (i = 0U, aq_vec = self->aq_vec[0]; > - aq_vec && self->aq_vecs > i; > - ++i, aq_vec = self->aq_vec[i]) { > + aq_vec && self->aq_vecs > i; ++i) { > data += count; > count = aq_vec_get_sw_stats(aq_vec, tc, data); > + if (self->aq_vecs > i) > + aq_vec = self->aq_vec[i]; > } AeLan, thanks for the changes. Could you please simplify this logic as well (like other instances). Say, for (i = 0U; self->aq_vecs > i; ++i) { ... count = aq_vec_get_sw_stats(self->aq_vec[i], tc, data); > } > > @@ -1382,7 +1381,6 @@ int aq_nic_set_loopback(struct aq_nic_s *self) > > int aq_nic_stop(struct aq_nic_s *self) > { > - struct aq_vec_s *aq_vec = NULL; > unsigned int i = 0U; > > netif_tx_disable(self->ndev); > @@ -1400,9 +1398,8 @@ int aq_nic_stop(struct aq_nic_s *self) > > aq_ptp_irq_free(self); > > - for (i = 0U, aq_vec = self->aq_vec[0]; > - self->aq_vecs > i; ++i, aq_vec = self->aq_vec[i]) > - aq_vec_stop(aq_vec); > + for (i = 0U; self->aq_vecs > i; ++i) > + aq_vec_stop(self->aq_vec[i]); > > aq_ptp_ring_stop(self); > > -- > 2.25.1
Powered by blists - more mailing lists