netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] net: atlantic: fix aq_vec index out of range error
@ 2022-08-08  8:18 AceLan Kao
  2022-08-08  8:53 ` [EXT] " Sudarsana Reddy Kalluru
  2022-08-10  5:20 ` patchwork-bot+netdevbpf
  0 siblings, 2 replies; 3+ messages in thread
From: AceLan Kao @ 2022-08-08  8:18 UTC (permalink / raw)
  To: Igor Russkikh, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Dmitrii Tarakanov, Alexander Loktionov,
	David VomLehn, Dmitry Bezrukov, netdev, linux-kernel,
	Sudarsana Reddy Kalluru

From: "Chia-Lin Kao (AceLan)" <acelan.kao@canonical.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"

v3. simplified a for loop

Fixes: 97bde5c4f909 ("net: ethernet: aquantia: Support for NIC-specific code")
Signed-off-by: Chia-Lin Kao (AceLan) <acelan.kao@canonical.com>
---
 .../net/ethernet/aquantia/atlantic/aq_nic.c   | 21 +++++++------------
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
index e11cc29d3264..06508eebb585 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);
@@ -1014,7 +1012,6 @@ int aq_nic_get_regs_count(struct aq_nic_s *self)
 
 u64 *aq_nic_get_stats(struct aq_nic_s *self, u64 *data)
 {
-	struct aq_vec_s *aq_vec = NULL;
 	struct aq_stats_s *stats;
 	unsigned int count = 0U;
 	unsigned int i = 0U;
@@ -1064,11 +1061,11 @@ u64 *aq_nic_get_stats(struct aq_nic_s *self, u64 *data)
 	data += i;
 
 	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]) {
+		for (i = 0U; self->aq_vecs > i; ++i) {
+			if (!self->aq_vec[i])
+				break;
 			data += count;
-			count = aq_vec_get_sw_stats(aq_vec, tc, data);
+			count = aq_vec_get_sw_stats(self->aq_vec[i], tc, data);
 		}
 	}
 
@@ -1382,7 +1379,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 +1396,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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* RE: [EXT] [PATCH v3] net: atlantic: fix aq_vec index out of range error
  2022-08-08  8:18 [PATCH v3] net: atlantic: fix aq_vec index out of range error AceLan Kao
@ 2022-08-08  8:53 ` Sudarsana Reddy Kalluru
  2022-08-10  5:20 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 3+ messages in thread
From: Sudarsana Reddy Kalluru @ 2022-08-08  8:53 UTC (permalink / raw)
  To: AceLan Kao, Igor Russkikh, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Dmitrii Tarakanov,
	Alexander Loktionov, David VomLehn, Dmitry Bezrukov, netdev,
	linux-kernel

> -----Original Message-----
> From: AceLan Kao <acelan@gmail.com> On Behalf Of AceLan Kao
> Sent: Monday, August 8, 2022 1:49 PM
> To: Igor Russkikh <irusskikh@marvell.com>; David S. Miller
> <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub
> Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; Dmitrii
> Tarakanov <Dmitrii.Tarakanov@aquantia.com>; Alexander Loktionov
> <Alexander.Loktionov@aquantia.com>; David VomLehn
> <vomlehn@texas.net>; Dmitry Bezrukov <Dmitry.Bezrukov@aquantia.com>;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Sudarsana Reddy
> Kalluru <skalluru@marvell.com>
> Subject: [EXT] [PATCH v3] net: atlantic: fix aq_vec index out of range error
> 
> External Email
> 
> ----------------------------------------------------------------------
> From: "Chia-Lin Kao (AceLan)" <acelan.kao@canonical.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"
> 
> v3. simplified a for loop
> 
> Fixes: 97bde5c4f909 ("net: ethernet: aquantia: Support for NIC-specific
> code")
> Signed-off-by: Chia-Lin Kao (AceLan) <acelan.kao@canonical.com>
> ---
>  .../net/ethernet/aquantia/atlantic/aq_nic.c   | 21 +++++++------------
>  1 file changed, 8 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
> b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
> index e11cc29d3264..06508eebb585 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);
> @@ -1014,7 +1012,6 @@ int aq_nic_get_regs_count(struct aq_nic_s *self)
> 
>  u64 *aq_nic_get_stats(struct aq_nic_s *self, u64 *data)  {
> -	struct aq_vec_s *aq_vec = NULL;
>  	struct aq_stats_s *stats;
>  	unsigned int count = 0U;
>  	unsigned int i = 0U;
> @@ -1064,11 +1061,11 @@ u64 *aq_nic_get_stats(struct aq_nic_s *self, u64
> *data)
>  	data += i;
> 
>  	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]) {
> +		for (i = 0U; self->aq_vecs > i; ++i) {
> +			if (!self->aq_vec[i])
> +				break;
>  			data += count;
> -			count = aq_vec_get_sw_stats(aq_vec, tc, data);
> +			count = aq_vec_get_sw_stats(self->aq_vec[i], tc,
> data);
>  		}
>  	}
> 
> @@ -1382,7 +1379,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 +1396,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

Thanks for the change.

Acked-by: Sudarsana Reddy Kalluru <skalluru@marvell.com>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v3] net: atlantic: fix aq_vec index out of range error
  2022-08-08  8:18 [PATCH v3] net: atlantic: fix aq_vec index out of range error AceLan Kao
  2022-08-08  8:53 ` [EXT] " Sudarsana Reddy Kalluru
@ 2022-08-10  5:20 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 3+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-08-10  5:20 UTC (permalink / raw)
  To: AceLan Kao
  Cc: irusskikh, davem, edumazet, kuba, pabeni, Dmitrii.Tarakanov,
	Alexander.Loktionov, vomlehn, Dmitry.Bezrukov, netdev,
	linux-kernel, skalluru

Hello:

This patch was applied to netdev/net.git (master)
by Jakub Kicinski <kuba@kernel.org>:

On Mon,  8 Aug 2022 16:18:45 +0800 you wrote:
> From: "Chia-Lin Kao (AceLan)" <acelan.kao@canonical.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.
> 
> [...]

Here is the summary with links:
  - [v3] net: atlantic: fix aq_vec index out of range error
    https://git.kernel.org/netdev/net/c/2ba5e47fb75f

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-08-10  5:20 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-08  8:18 [PATCH v3] net: atlantic: fix aq_vec index out of range error AceLan Kao
2022-08-08  8:53 ` [EXT] " Sudarsana Reddy Kalluru
2022-08-10  5:20 ` patchwork-bot+netdevbpf

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).