Commit 8535107a authored by Brian Norris's avatar Brian Norris Committed by Kalle Valo

mwifiex: fixup error cases in mwifiex_add_virtual_intf()

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 7d652034 ("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
93a1df48 ("mwifiex: add cfg80211 handlers add/del_virtual_intf").

Cc: <stable@vger.kernel.org>
Signed-off-by: default avatarBrian Norris <briannorris@chromium.org>
Signed-off-by: default avatarKalle Valo <kvalo@codeaurora.org>
parent d41bf5c1
...@@ -2964,10 +2964,8 @@ struct wireless_dev *mwifiex_add_virtual_intf(struct wiphy *wiphy, ...@@ -2964,10 +2964,8 @@ struct wireless_dev *mwifiex_add_virtual_intf(struct wiphy *wiphy,
if (!dev) { if (!dev) {
mwifiex_dbg(adapter, ERROR, mwifiex_dbg(adapter, ERROR,
"no memory available for netdevice\n"); "no memory available for netdevice\n");
memset(&priv->wdev, 0, sizeof(priv->wdev)); ret = -ENOMEM;
priv->wdev.iftype = NL80211_IFTYPE_UNSPECIFIED; goto err_alloc_netdev;
priv->bss_mode = NL80211_IFTYPE_UNSPECIFIED;
return ERR_PTR(-ENOMEM);
} }
mwifiex_init_priv_params(priv, dev); mwifiex_init_priv_params(priv, dev);
...@@ -2976,11 +2974,11 @@ struct wireless_dev *mwifiex_add_virtual_intf(struct wiphy *wiphy, ...@@ -2976,11 +2974,11 @@ struct wireless_dev *mwifiex_add_virtual_intf(struct wiphy *wiphy,
ret = mwifiex_send_cmd(priv, HostCmd_CMD_SET_BSS_MODE, ret = mwifiex_send_cmd(priv, HostCmd_CMD_SET_BSS_MODE,
HostCmd_ACT_GEN_SET, 0, NULL, true); HostCmd_ACT_GEN_SET, 0, NULL, true);
if (ret) if (ret)
return ERR_PTR(ret); goto err_set_bss_mode;
ret = mwifiex_sta_init_cmd(priv, false, false); ret = mwifiex_sta_init_cmd(priv, false, false);
if (ret) if (ret)
return ERR_PTR(ret); goto err_sta_init;
mwifiex_setup_ht_caps(&wiphy->bands[NL80211_BAND_2GHZ]->ht_cap, priv); mwifiex_setup_ht_caps(&wiphy->bands[NL80211_BAND_2GHZ]->ht_cap, priv);
if (adapter->is_hw_11ac_capable) if (adapter->is_hw_11ac_capable)
...@@ -3011,31 +3009,14 @@ struct wireless_dev *mwifiex_add_virtual_intf(struct wiphy *wiphy, ...@@ -3011,31 +3009,14 @@ struct wireless_dev *mwifiex_add_virtual_intf(struct wiphy *wiphy,
SET_NETDEV_DEV(dev, adapter->dev); SET_NETDEV_DEV(dev, adapter->dev);
/* Register network device */
if (register_netdevice(dev)) {
mwifiex_dbg(adapter, ERROR,
"cannot register virtual network device\n");
free_netdev(dev);
priv->bss_mode = NL80211_IFTYPE_UNSPECIFIED;
priv->netdev = NULL;
memset(&priv->wdev, 0, sizeof(priv->wdev));
priv->wdev.iftype = NL80211_IFTYPE_UNSPECIFIED;
return ERR_PTR(-EFAULT);
}
priv->dfs_cac_workqueue = alloc_workqueue("MWIFIEX_DFS_CAC%s", priv->dfs_cac_workqueue = alloc_workqueue("MWIFIEX_DFS_CAC%s",
WQ_HIGHPRI | WQ_HIGHPRI |
WQ_MEM_RECLAIM | WQ_MEM_RECLAIM |
WQ_UNBOUND, 1, name); WQ_UNBOUND, 1, name);
if (!priv->dfs_cac_workqueue) { if (!priv->dfs_cac_workqueue) {
mwifiex_dbg(adapter, ERROR, mwifiex_dbg(adapter, ERROR, "cannot alloc DFS CAC queue\n");
"cannot register virtual network device\n"); ret = -ENOMEM;
free_netdev(dev); goto err_alloc_cac;
priv->bss_mode = NL80211_IFTYPE_UNSPECIFIED;
priv->netdev = NULL;
memset(&priv->wdev, 0, sizeof(priv->wdev));
priv->wdev.iftype = NL80211_IFTYPE_UNSPECIFIED;
return ERR_PTR(-ENOMEM);
} }
INIT_DELAYED_WORK(&priv->dfs_cac_work, mwifiex_dfs_cac_work_queue); INIT_DELAYED_WORK(&priv->dfs_cac_work, mwifiex_dfs_cac_work_queue);
...@@ -3044,16 +3025,9 @@ struct wireless_dev *mwifiex_add_virtual_intf(struct wiphy *wiphy, ...@@ -3044,16 +3025,9 @@ struct wireless_dev *mwifiex_add_virtual_intf(struct wiphy *wiphy,
WQ_HIGHPRI | WQ_UNBOUND | WQ_HIGHPRI | WQ_UNBOUND |
WQ_MEM_RECLAIM, 1, name); WQ_MEM_RECLAIM, 1, name);
if (!priv->dfs_chan_sw_workqueue) { if (!priv->dfs_chan_sw_workqueue) {
mwifiex_dbg(adapter, ERROR, mwifiex_dbg(adapter, ERROR, "cannot alloc DFS channel sw queue\n");
"cannot register virtual network device\n"); ret = -ENOMEM;
free_netdev(dev); goto err_alloc_chsw;
priv->bss_mode = NL80211_IFTYPE_UNSPECIFIED;
priv->netdev = NULL;
memset(&priv->wdev, 0, sizeof(priv->wdev));
priv->wdev.iftype = NL80211_IFTYPE_UNSPECIFIED;
destroy_workqueue(priv->dfs_cac_workqueue);
priv->dfs_cac_workqueue = NULL;
return ERR_PTR(-ENOMEM);
} }
INIT_DELAYED_WORK(&priv->dfs_chan_sw_work, INIT_DELAYED_WORK(&priv->dfs_chan_sw_work,
...@@ -3061,6 +3035,13 @@ struct wireless_dev *mwifiex_add_virtual_intf(struct wiphy *wiphy, ...@@ -3061,6 +3035,13 @@ struct wireless_dev *mwifiex_add_virtual_intf(struct wiphy *wiphy,
sema_init(&priv->async_sem, 1); sema_init(&priv->async_sem, 1);
/* Register network device */
if (register_netdevice(dev)) {
mwifiex_dbg(adapter, ERROR, "cannot register network device\n");
ret = -EFAULT;
goto err_reg_netdev;
}
mwifiex_dbg(adapter, INFO, mwifiex_dbg(adapter, INFO,
"info: %s: Marvell 802.11 Adapter\n", dev->name); "info: %s: Marvell 802.11 Adapter\n", dev->name);
...@@ -3081,11 +3062,29 @@ struct wireless_dev *mwifiex_add_virtual_intf(struct wiphy *wiphy, ...@@ -3081,11 +3062,29 @@ struct wireless_dev *mwifiex_add_virtual_intf(struct wiphy *wiphy,
adapter->curr_iface_comb.p2p_intf++; adapter->curr_iface_comb.p2p_intf++;
break; break;
default: default:
/* This should be dead code; checked above */
mwifiex_dbg(adapter, ERROR, "type not supported\n"); mwifiex_dbg(adapter, ERROR, "type not supported\n");
return ERR_PTR(-EINVAL); return ERR_PTR(-EINVAL);
} }
return &priv->wdev; return &priv->wdev;
err_reg_netdev:
destroy_workqueue(priv->dfs_chan_sw_workqueue);
priv->dfs_chan_sw_workqueue = NULL;
err_alloc_chsw:
destroy_workqueue(priv->dfs_cac_workqueue);
priv->dfs_cac_workqueue = NULL;
err_alloc_cac:
free_netdev(dev);
priv->netdev = NULL;
err_sta_init:
err_set_bss_mode:
err_alloc_netdev:
memset(&priv->wdev, 0, sizeof(priv->wdev));
priv->wdev.iftype = NL80211_IFTYPE_UNSPECIFIED;
priv->bss_mode = NL80211_IFTYPE_UNSPECIFIED;
return ERR_PTR(ret);
} }
EXPORT_SYMBOL_GPL(mwifiex_add_virtual_intf); EXPORT_SYMBOL_GPL(mwifiex_add_virtual_intf);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment