mwifiex: fixup error cases in mwifiex_add_virtual_intf()
authorBrian Norris <briannorris@chromium.org>
Fri, 12 May 2017 16:41:58 +0000 (09:41 -0700)
committerBen Hutchings <ben@decadent.org.uk>
Thu, 12 Oct 2017 14:27:08 +0000 (15:27 +0100)
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>
drivers/net/wireless/mwifiex/cfg80211.c

index 4d7e334..68e9583 100644 (file)
@@ -1176,6 +1176,7 @@ struct net_device *mwifiex_add_virtual_intf(struct wiphy *wiphy,
        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_intf(struct wiphy *wiphy,
                              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_intf(struct wiphy *wiphy,
 
        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);