diff mbox series

net: atlantic: fix aq_vec index out of range error

Message ID 20220805093319.3722179-1-acelan.kao@canonical.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series net: atlantic: fix aq_vec index out of range error | expand

Checks

Context Check Description
netdev/tree_selection success Guessed tree name to be net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix warning Target tree name not specified in the subject
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit fail Errors and warnings before: 0 this patch: 2
netdev/cc_maintainers fail 1 blamed authors not CCed: Pavel.Belous@aquantia.com; 1 maintainers not CCed: Pavel.Belous@aquantia.com
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn fail Errors and warnings before: 0 this patch: 2
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 33 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

AceLan Kao Aug. 5, 2022, 9:33 a.m. UTC
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>

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

Comments

kernel test robot Aug. 5, 2022, 2:53 p.m. UTC | #1
Hi AceLan,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]
[also build test WARNING on net/master linus/master horms-ipvs/master v5.19 next-20220804]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/AceLan-Kao/net-atlantic-fix-aq_vec-index-out-of-range-error/20220805-173434
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git f86d1fbbe7858884d6754534a0afbb74fc30bc26
config: i386-allyesconfig (https://download.01.org/0day-ci/archive/20220805/202208052211.yWBUaHIc-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-3) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/23a65a8ebdb1e74cf7fc03a89741246de646622b
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review AceLan-Kao/net-atlantic-fix-aq_vec-index-out-of-range-error/20220805-173434
        git checkout 23a65a8ebdb1e74cf7fc03a89741246de646622b
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/net/ethernet/aquantia/atlantic/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/net/ethernet/aquantia/atlantic/aq_nic.c: In function 'aq_nic_polling_timer_cb':
>> drivers/net/ethernet/aquantia/atlantic/aq_nic.c:268:26: warning: variable 'aq_vec' set but not used [-Wunused-but-set-variable]
     268 |         struct aq_vec_s *aq_vec = NULL;
         |                          ^~~~~~
   drivers/net/ethernet/aquantia/atlantic/aq_nic.c: In function 'aq_nic_stop':
   drivers/net/ethernet/aquantia/atlantic/aq_nic.c:1384:26: warning: variable 'aq_vec' set but not used [-Wunused-but-set-variable]
    1384 |         struct aq_vec_s *aq_vec = NULL;
         |                          ^~~~~~


vim +/aq_vec +268 drivers/net/ethernet/aquantia/atlantic/aq_nic.c

97bde5c4f909a55a David VomLehn         2017-01-23  264  
e99e88a9d2b06746 Kees Cook             2017-10-16  265  static void aq_nic_polling_timer_cb(struct timer_list *t)
97bde5c4f909a55a David VomLehn         2017-01-23  266  {
e99e88a9d2b06746 Kees Cook             2017-10-16  267  	struct aq_nic_s *self = from_timer(self, t, polling_timer);
97bde5c4f909a55a David VomLehn         2017-01-23 @268  	struct aq_vec_s *aq_vec = NULL;
97bde5c4f909a55a David VomLehn         2017-01-23  269  	unsigned int i = 0U;
97bde5c4f909a55a David VomLehn         2017-01-23  270  
97bde5c4f909a55a David VomLehn         2017-01-23  271  	for (i = 0U, aq_vec = self->aq_vec[0];
23a65a8ebdb1e74c Chia-Lin Kao (AceLan  2022-08-05  272) 		self->aq_vecs > i; ++i)
23a65a8ebdb1e74c Chia-Lin Kao (AceLan  2022-08-05  273) 		aq_vec_isr(i, (void *)self->aq_vec[i]);
97bde5c4f909a55a David VomLehn         2017-01-23  274  
97bde5c4f909a55a David VomLehn         2017-01-23  275  	mod_timer(&self->polling_timer, jiffies +
97bde5c4f909a55a David VomLehn         2017-01-23  276  		  AQ_CFG_POLLING_TIMER_INTERVAL);
97bde5c4f909a55a David VomLehn         2017-01-23  277  }
97bde5c4f909a55a David VomLehn         2017-01-23  278
diff mbox series

Patch

diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
index e11cc29d3264..b0064b54d334 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
@@ -269,8 +269,8 @@  static void aq_nic_polling_timer_cb(struct timer_list *t)
 	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);
+		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 +1065,9 @@  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);
+			count = aq_vec_get_sw_stats(self->aq_vec[i], tc, data);
 		}
 	}
 
@@ -1400,9 +1399,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, aq_vec = self->aq_vec[0]; self->aq_vecs > i; ++i)
+		aq_vec_stop(self->aq_vec[i]);
 
 	aq_ptp_ring_stop(self);