3.16.49-rc1 review patch. If anyone has any objections, please let me know.
------------------
From: Brian Norris <briannor...@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 <briannor...@chromium.org>
Signed-off-by: Kalle Valo <kv...@codeaurora.org>
[bwh: Backported to 3.16:
- 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 <b...@decadent.org.uk>
---
--- a/drivers/net/wireless/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/mwifiex/cfg80211.c
@@ -2160,6 +2160,7 @@ struct wireless_dev *mwifiex_add_virtual
struct net_device *dev;
void *mdev_priv;
struct wireless_dev *wdev;
+ int ret;
if (!adapter)
return ERR_PTR(-EFAULT);
@@ -2254,8 +2255,8 @@ struct wireless_dev *mwifiex_add_virtual
priv->bss_num = 0;
if (mwifiex_cfg80211_init_p2p_client(priv)) {
- wdev = ERR_PTR(-EFAULT);
- goto done;
+ ret = -EFAULT;
+ goto err_set_bss_mode;
}
break;
@@ -2268,9 +2269,8 @@ struct wireless_dev *mwifiex_add_virtual
ether_setup, IEEE80211_NUM_ACS, 1);
if (!dev) {
wiphy_err(wiphy, "no memory available for netdevice\n");
- priv->bss_mode = NL80211_IFTYPE_UNSPECIFIED;
- wdev = ERR_PTR(-ENOMEM);
- goto done;
+ ret = -ENOMEM;
+ goto err_alloc_netdev;
}
mwifiex_init_priv_params(priv, dev);
@@ -2305,31 +2305,32 @@ struct wireless_dev *mwifiex_add_virtual
SET_NETDEV_DEV(dev, adapter->dev);
+ sema_init(&priv->async_sem, 1);
+
/* 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;
- priv->netdev = NULL;
- wdev = ERR_PTR(-EFAULT);
- goto done;
+ ret = -EFAULT;
+ goto err_reg_netdev;
}
- sema_init(&priv->async_sem, 1);
-
dev_dbg(adapter->dev, "info: %s: Marvell 802.11 Adapter\n", dev->name);
#ifdef CONFIG_DEBUG_FS
mwifiex_dev_debugfs_init(priv);
#endif
-done:
- if (IS_ERR(wdev)) {
- kfree(priv->wdev);
- priv->wdev = NULL;
- }
-
return wdev;
+
+err_reg_netdev:
+ free_netdev(dev);
+ priv->netdev = NULL;
+err_set_bss_mode:
+err_alloc_netdev:
+ kfree(priv->wdev);
+ priv->wdev = NULL;
+ priv->bss_mode = NL80211_IFTYPE_UNSPECIFIED;
+ return ERR_PTR(ret);
}
EXPORT_SYMBOL_GPL(mwifiex_add_virtual_intf);
