Messages in this thread Patch in this message | | | From | Ben Hutchings <> | Date | Mon, 09 Oct 2017 13:30:34 +0100 | Subject | [PATCH 3.2 03/74] mwifiex: fixup error cases in mwifiex_add_virtual_intf() |
| |
3.2.94-rc1 review patch. If anyone has any objections, please let me know.
------------------
From: Brian Norris <briannorris@chromium.org>
commit 8535107aa4ef92520cbb9a4739563b389c5f8e2c upstream.
If we fail to add an interface in mwifiex_add_virtual_intf(), we might hit a BUG_ON() in the networking code, because we didn't tear things down properly. Among the problems:
(a) when failing to allocate workqueues, we fail to unregister the netdev before calling free_netdev() (b) even if we do try to unregister the netdev, we're still holding the rtnl lock, so the device never properly unregistered; we'll be at state NETREG_UNREGISTERING, and then hit free_netdev()'s: BUG_ON(dev->reg_state != NETREG_UNREGISTERED); (c) we're allocating some dependent resources (e.g., DFS workqueues) after we've registered the interface; this may or may not cause problems, but it's good practice to allocate these before registering (d) we're not even trying to unwind anything when mwifiex_send_cmd() or mwifiex_sta_init_cmd() fail
To fix these issues, let's:
* add a stacked set of error handling labels, to keep error handling consistent and properly ordered (resolving (a) and (d)) * move the workqueue allocations before the registration (to resolve (c); also resolves (b) by avoiding error cases where we have to unregister)
[Incidentally, it's pretty easy to interrupt the alloc_workqueue() in, e.g., the following:
iw phy phy0 interface add mlan0 type station
by sending it SIGTERM.]
This bugfix covers commits like commit 7d652034d1a0 ("mwifiex: channel switch support for mwifiex"), but parts of this bug exist all the way back to the introduction of dynamic interface handling in commit 93a1df48d224 ("mwifiex: add cfg80211 handlers add/del_virtual_intf").
Signed-off-by: Brian Norris <briannorris@chromium.org> Signed-off-by: Kalle Valo <kvalo@codeaurora.org> [bwh: Backported to 3.2: - There is no workqueue allocation or cleanup needed here - Add 'ret' variable - Keep logging errors with wiphy_err() - Adjust filename] Signed-off-by: Ben Hutchings <ben@decadent.org.uk> --- --- a/drivers/net/wireless/mwifiex/cfg80211.c +++ b/drivers/net/wireless/mwifiex/cfg80211.c @@ -1176,6 +1176,7 @@ struct net_device *mwifiex_add_virtual_i struct mwifiex_adapter *adapter; struct net_device *dev; void *mdev_priv; + int ret; if (!priv) return ERR_PTR(-EFAULT); @@ -1216,8 +1217,8 @@ struct net_device *mwifiex_add_virtual_i ether_setup, 1); if (!dev) { wiphy_err(wiphy, "no memory available for netdevice\n"); - priv->bss_mode = NL80211_IFTYPE_UNSPECIFIED; - return ERR_PTR(-ENOMEM); + ret = -ENOMEM; + goto err_alloc_netdev; } dev_net_set(dev, wiphy_net(wiphy)); @@ -1239,23 +1240,29 @@ struct net_device *mwifiex_add_virtual_i SET_NETDEV_DEV(dev, adapter->dev); + sema_init(&priv->async_sem, 1); + priv->scan_pending_on_block = false; + /* Register network device */ if (register_netdevice(dev)) { wiphy_err(wiphy, "cannot register virtual network device\n"); - free_netdev(dev); - priv->bss_mode = NL80211_IFTYPE_UNSPECIFIED; - return ERR_PTR(-EFAULT); + ret = -EFAULT; + goto err_reg_netdev; } - sema_init(&priv->async_sem, 1); - priv->scan_pending_on_block = false; - dev_dbg(adapter->dev, "info: %s: Marvell 802.11 Adapter\n", dev->name); #ifdef CONFIG_DEBUG_FS mwifiex_dev_debugfs_init(priv); #endif return dev; + +err_reg_netdev: + free_netdev(dev); + priv->netdev = NULL; +err_alloc_netdev: + priv->bss_mode = NL80211_IFTYPE_UNSPECIFIED; + return ERR_PTR(ret); } EXPORT_SYMBOL_GPL(mwifiex_add_virtual_intf);
| |