diff mbox

mwifiex: fix null pointer deference when adapter is null

Message ID 20160915134238.5167-1-colin.king@canonical.com
State Awaiting Upstream, archived
Delegated to: David Miller
Headers show

Commit Message

Colin Ian King Sept. 15, 2016, 1:42 p.m. UTC 
From: Colin Ian King <colin.king@canonical.com>

If adapter is null the error exit path in mwifiex_shutdown_sw is
to down the semaphore sem and print some debug via mwifiex_dbg.
However, passing a NULL adapter to mwifiex_dbg causes a null
pointer deference when accessing adapter->dev.  This fix checks
for a null adapter at the start of the function and to exit
without the need to up the semaphore and we also skip the debug
to avoid the null pointer dereference.

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/net/wireless/marvell/mwifiex/main.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Comments

Julian Calaby Sept. 15, 2016, 1:56 p.m. UTC | #1 
Hi All,

On Thu, Sep 15, 2016 at 11:42 PM, Colin King <colin.king@canonical.com> wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> If adapter is null the error exit path in mwifiex_shutdown_sw is
> to down the semaphore sem and print some debug via mwifiex_dbg.
> However, passing a NULL adapter to mwifiex_dbg causes a null
> pointer deference when accessing adapter->dev.  This fix checks
> for a null adapter at the start of the function and to exit
> without the need to up the semaphore and we also skip the debug
> to avoid the null pointer dereference.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>

Reviewed-by: Julian Calaby <julian.calaby@gmail.com>

Thanks,
kernel test robot Sept. 15, 2016, 4:29 p.m. UTC | #2 
Hi Colin,

[auto build test WARNING on wireless-drivers-next/master]
[also build test WARNING on next-20160915]
[cannot apply to v4.8-rc6]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
[Suggest to use git(>=2.9.0) format-patch --base=<commit> (or --base=auto for convenience) to record what (public, well-known) commit your patch series was built on]
[Check https://git-scm.com/docs/git-format-patch for more information]

url:    https://github.com/0day-ci/linux/commits/Colin-King/mwifiex-fix-null-pointer-deference-when-adapter-is-null/20160915-231625
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next.git master
config: x86_64-randconfig-x013-201637 (attached as .config)
compiler: gcc-6 (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   drivers/net/wireless/marvell/mwifiex/main.c: In function 'mwifiex_shutdown_sw':
>> drivers/net/wireless/marvell/mwifiex/main.c:1433:1: warning: label 'exit_remove' defined but not used [-Wunused-label]
    exit_remove:
    ^~~~~~~~~~~
   Cyclomatic Complexity 5 include/linux/compiler.h:__read_once_size
   Cyclomatic Complexity 5 include/linux/compiler.h:__write_once_size
   Cyclomatic Complexity 2 arch/x86/include/asm/bitops.h:set_bit
   Cyclomatic Complexity 1 arch/x86/include/asm/bitops.h:constant_test_bit
   Cyclomatic Complexity 1 arch/x86/include/asm/bitops.h:fls64
   Cyclomatic Complexity 1 include/linux/log2.h:__ilog2_u64
   Cyclomatic Complexity 1 include/linux/list.h:INIT_LIST_HEAD
   Cyclomatic Complexity 1 include/linux/list.h:list_empty
   Cyclomatic Complexity 1 include/asm-generic/getorder.h:__get_order
   Cyclomatic Complexity 1 arch/x86/include/asm/atomic.h:atomic_read
   Cyclomatic Complexity 1 arch/x86/include/asm/atomic.h:atomic_inc
   Cyclomatic Complexity 1 arch/x86/include/asm/atomic.h:atomic_dec
   Cyclomatic Complexity 1 arch/x86/include/asm/atomic.h:atomic_add_return
   Cyclomatic Complexity 1 include/linux/spinlock.h:spinlock_check
   Cyclomatic Complexity 1 include/linux/spinlock.h:spin_unlock_irqrestore
   Cyclomatic Complexity 1 include/linux/kasan.h:kasan_kmalloc
   Cyclomatic Complexity 28 include/linux/slab.h:kmalloc_index
   Cyclomatic Complexity 1 include/linux/slab.h:kmem_cache_alloc_trace
   Cyclomatic Complexity 1 include/linux/slab.h:kmalloc_order_trace
   Cyclomatic Complexity 68 include/linux/slab.h:kmalloc_large
   Cyclomatic Complexity 5 include/linux/slab.h:kmalloc
   Cyclomatic Complexity 1 include/linux/slab.h:kzalloc
   Cyclomatic Complexity 1 include/linux/skbuff.h:skb_end_pointer
   Cyclomatic Complexity 1 include/linux/skbuff.h:skb_queue_empty
   Cyclomatic Complexity 1 include/linux/skbuff.h:skb_shared
   Cyclomatic Complexity 1 include/linux/skbuff.h:skb_headroom
   Cyclomatic Complexity 1 include/linux/netdevice.h:netdev_get_tx_queue
   Cyclomatic Complexity 1 include/linux/netdevice.h:netdev_priv
   Cyclomatic Complexity 1 include/linux/netdevice.h:netif_tx_stop_queue
   Cyclomatic Complexity 1 include/linux/netdevice.h:netif_tx_queue_stopped
   Cyclomatic Complexity 1 include/linux/netdevice.h:netif_carrier_ok
   Cyclomatic Complexity 1 include/linux/etherdevice.h:is_multicast_ether_addr
   Cyclomatic Complexity 1 include/linux/etherdevice.h:ether_addr_copy
   Cyclomatic Complexity 1 include/linux/etherdevice.h:ether_addr_equal
   Cyclomatic Complexity 1 include/linux/etherdevice.h:ether_addr_equal_unaligned
   Cyclomatic Complexity 1 drivers/net/wireless/marvell/mwifiex/util.h:MWIFIEX_SKB_TXCB
   Cyclomatic Complexity 6 drivers/net/wireless/marvell/mwifiex/main.h:mwifiex_get_priv
   Cyclomatic Complexity 1 drivers/net/wireless/marvell/mwifiex/main.h:mwifiex_netdev_get_priv
   Cyclomatic Complexity 1 drivers/net/wireless/marvell/mwifiex/main.h:mwifiex_is_skb_mgmt_frame
   Cyclomatic Complexity 1 drivers/net/wireless/marvell/mwifiex/main.c:mwifiex_get_stats
   Cyclomatic Complexity 1 include/linux/workqueue.h:queue_work
   Cyclomatic Complexity 2 drivers/net/wireless/marvell/mwifiex/main.c:mwifiex_queue_rx_work
   Cyclomatic Complexity 4 drivers/net/wireless/marvell/mwifiex/main.c:mwifiex_set_multicast_list
   Cyclomatic Complexity 1 drivers/net/wireless/marvell/mwifiex/main.c:mwifiex_netdev_select_wmm_queue
   Cyclomatic Complexity 1 include/linux/err.h:IS_ERR
   Cyclomatic Complexity 1 include/linux/timekeeping.h:ktime_get_real
   Cyclomatic Complexity 1 include/linux/skbuff.h:__net_timestamp
   Cyclomatic Complexity 1 drivers/net/wireless/marvell/mwifiex/main.c:mwifiex_open
   Cyclomatic Complexity 2 drivers/net/wireless/marvell/mwifiex/util.h:MWIFIEX_SKB_RXCB
   Cyclomatic Complexity 1 include/linux/netdevice.h:dev_kfree_skb_any
   Cyclomatic Complexity 6 drivers/net/wireless/marvell/mwifiex/main.c:mwifiex_unregister
   Cyclomatic Complexity 2 drivers/net/wireless/marvell/mwifiex/main.c:mwifiex_free_adapter
   Cyclomatic Complexity 3 drivers/net/wireless/marvell/mwifiex/main.c:mwifiex_terminate_workqueue
   Cyclomatic Complexity 1 drivers/net/wireless/marvell/mwifiex/main.c:mwifiex_init_module
   Cyclomatic Complexity 1 drivers/net/wireless/marvell/mwifiex/main.c:mwifiex_cleanup_module
   Cyclomatic Complexity 2 drivers/net/wireless/marvell/mwifiex/main.c:mwifiex_queue_main_work
   Cyclomatic Complexity 9 drivers/net/wireless/marvell/mwifiex/main.c:mwifiex_process_rx
   Cyclomatic Complexity 2 drivers/net/wireless/marvell/mwifiex/main.c:mwifiex_rx_work_queue
   Cyclomatic Complexity 5 drivers/net/wireless/marvell/mwifiex/main.c:mwifiex_clone_skb_for_tx_status
   Cyclomatic Complexity 2 drivers/net/wireless/marvell/mwifiex/main.c:mwifiex_init_priv_params
   Cyclomatic Complexity 1 drivers/net/wireless/marvell/mwifiex/main.c:is_command_pending
   Cyclomatic Complexity 81 drivers/net/wireless/marvell/mwifiex/main.c:mwifiex_main_process
   Cyclomatic Complexity 2 drivers/net/wireless/marvell/mwifiex/main.c:mwifiex_main_work_queue
   Cyclomatic Complexity 3 drivers/net/wireless/marvell/mwifiex/main.c:_mwifiex_dbg
   Cyclomatic Complexity 6 drivers/net/wireless/marvell/mwifiex/main.c:mwifiex_bypass_tx_queue
   Cyclomatic Complexity 4 drivers/net/wireless/marvell/mwifiex/main.c:mwifiex_queue_tx_pkt
   Cyclomatic Complexity 4 drivers/net/wireless/marvell/mwifiex/main.c:mwifiex_multi_chan_resync
   Cyclomatic Complexity 19 drivers/net/wireless/marvell/mwifiex/main.c:mwifiex_drv_info_dump
   Cyclomatic Complexity 9 drivers/net/wireless/marvell/mwifiex/main.c:mwifiex_upload_device_dump
   Cyclomatic Complexity 3 drivers/net/wireless/marvell/mwifiex/main.c:mwifiex_tx_timeout
   Cyclomatic Complexity 2 drivers/net/wireless/marvell/mwifiex/main.c:mwifiex_set_mac_address
   Cyclomatic Complexity 16 drivers/net/wireless/marvell/mwifiex/main.c:mwifiex_hard_start_xmit
   Cyclomatic Complexity 3 drivers/net/wireless/marvell/mwifiex/main.c:mwifiex_close
   Cyclomatic Complexity 34 drivers/net/wireless/marvell/mwifiex/main.c:mwifiex_fw_dpc
   Cyclomatic Complexity 6 drivers/net/wireless/marvell/mwifiex/main.c:mwifiex_init_hw_fw
   Cyclomatic Complexity 14 drivers/net/wireless/marvell/mwifiex/main.c:mwifiex_reinit_sw
   Cyclomatic Complexity 20 drivers/net/wireless/marvell/mwifiex/main.c:mwifiex_shutdown_sw
   Cyclomatic Complexity 2 drivers/net/wireless/marvell/mwifiex/main.c:mwifiex_do_flr
   Cyclomatic Complexity 7 drivers/net/wireless/marvell/mwifiex/main.c:mwifiex_register
   Cyclomatic Complexity 15 drivers/net/wireless/marvell/mwifiex/main.c:mwifiex_add_card
   Cyclomatic Complexity 20 drivers/net/wireless/marvell/mwifiex/main.c:mwifiex_remove_card

vim +/exit_remove +1433 drivers/net/wireless/marvell/mwifiex/main.c

4c5dae59 Amitkumar Karwar 2016-07-26  1417  			    atomic_read(&adapter->rx_pending),
4c5dae59 Amitkumar Karwar 2016-07-26  1418  			    atomic_read(&adapter->tx_pending),
4c5dae59 Amitkumar Karwar 2016-07-26  1419  			    atomic_read(&adapter->cmd_pending));
4c5dae59 Amitkumar Karwar 2016-07-26  1420  	}
4c5dae59 Amitkumar Karwar 2016-07-26  1421  
4c5dae59 Amitkumar Karwar 2016-07-26  1422  	for (i = 0; i < adapter->priv_num; i++) {
4c5dae59 Amitkumar Karwar 2016-07-26  1423  		priv = adapter->priv[i];
4c5dae59 Amitkumar Karwar 2016-07-26  1424  		if (!priv)
4c5dae59 Amitkumar Karwar 2016-07-26  1425  			continue;
4c5dae59 Amitkumar Karwar 2016-07-26  1426  		rtnl_lock();
4c5dae59 Amitkumar Karwar 2016-07-26  1427  		if (priv->netdev &&
4c5dae59 Amitkumar Karwar 2016-07-26  1428  		    priv->wdev.iftype != NL80211_IFTYPE_UNSPECIFIED)
4c5dae59 Amitkumar Karwar 2016-07-26  1429  			mwifiex_del_virtual_intf(adapter->wiphy, &priv->wdev);
4c5dae59 Amitkumar Karwar 2016-07-26  1430  		rtnl_unlock();
4c5dae59 Amitkumar Karwar 2016-07-26  1431  	}
4c5dae59 Amitkumar Karwar 2016-07-26  1432  
4c5dae59 Amitkumar Karwar 2016-07-26 @1433  exit_remove:
4c5dae59 Amitkumar Karwar 2016-07-26  1434  	up(sem);
4c5dae59 Amitkumar Karwar 2016-07-26  1435  exit_sem_err:
4c5dae59 Amitkumar Karwar 2016-07-26  1436  	mwifiex_dbg(adapter, INFO, "%s, successful\n", __func__);
2d1cb5d4 Colin Ian King   2016-09-15  1437  exit_return:
4c5dae59 Amitkumar Karwar 2016-07-26  1438  	return 0;
4c5dae59 Amitkumar Karwar 2016-07-26  1439  }
4c5dae59 Amitkumar Karwar 2016-07-26  1440  
4c5dae59 Amitkumar Karwar 2016-07-26  1441  /* This function gets called during PCIe function level reset. Required

:::::: The code at line 1433 was first introduced by commit
:::::: 4c5dae59d2e9386c706a2f3c7c2746ae277bf568 mwifiex: add PCIe function level reset support

:::::: TO: Amitkumar Karwar <akarwar@marvell.com>
:::::: CC: Kalle Valo <kvalo@codeaurora.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Kalle Valo Sept. 16, 2016, 3:56 a.m. UTC | #3 
kbuild test robot <lkp@intel.com> writes:

> url:    https://github.com/0day-ci/linux/commits/Colin-King/mwifiex-fix-null-pointer-deference-when-adapter-is-null/20160915-231625
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next.git master
> config: x86_64-randconfig-x013-201637 (attached as .config)
> compiler: gcc-6 (Debian 6.1.1-9) 6.1.1 20160705
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=x86_64 
>
> All warnings (new ones prefixed by >>):
>
>    drivers/net/wireless/marvell/mwifiex/main.c: In function 'mwifiex_shutdown_sw':
>>> drivers/net/wireless/marvell/mwifiex/main.c:1433:1: warning: label 'exit_remove' defined but not used [-Wunused-label]
>     exit_remove:
>     ^~~~~~~~~~~

Looks like a valid warning to me, so please resend.
diff mbox

Patch

diff --git a/drivers/net/wireless/marvell/mwifiex/main.c b/drivers/net/wireless/marvell/mwifiex/main.c
index 9b2e98c..7a4f8cc 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.c
+++ b/drivers/net/wireless/marvell/mwifiex/main.c
@@ -1369,12 +1369,12 @@  mwifiex_shutdown_sw(struct mwifiex_adapter *adapter, struct semaphore *sem)
 	struct mwifiex_private *priv;
 	int i;
 
+	if (!adapter)
+		goto exit_return;
+
 	if (down_interruptible(sem))
 		goto exit_sem_err;
 
-	if (!adapter)
-		goto exit_remove;
-
 	priv = mwifiex_get_priv(adapter, MWIFIEX_BSS_ROLE_ANY);
 	mwifiex_deauthenticate(priv, NULL);
 
@@ -1434,6 +1434,7 @@  mwifiex_shutdown_sw(struct mwifiex_adapter *adapter, struct semaphore *sem)
 	up(sem);
 exit_sem_err:
 	mwifiex_dbg(adapter, INFO, "%s, successful\n", __func__);
+exit_return:
 	return 0;
 }