mac80211: add interface list lock

Using only the RTNL has a number of problems, most notably that
ieee80211_iterate_active_interfaces() and other interface list
traversals cannot be done from the internal workqueue because it
needs to be flushed under the RTNL.

This patch introduces a new mutex that protects the interface list
against modifications. A more detailed explanation is part of the
code change.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: John W. Linville <linville@tuxdriver.com>

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index c1e8261..8e65adf 100644 (file)
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -928,9 +928,8 @@ enum ieee80211_hw_flags {
  * @workqueue: single threaded workqueue available for driver use,
  *     allocated by mac80211 on registration and flushed when an
  *     interface is removed.
- *     NOTICE: All work performed on this workqueue should NEVER
- *     acquire the RTNL lock (i.e. Don't use the function
- *     ieee80211_iterate_active_interfaces())
+ *     NOTICE: All work performed on this workqueue must not
+ *     acquire the RTNL lock.
  *
  * @priv: pointer to private area that was allocated for driver use
  *     along with this structure.

diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 927cbde..eaf3603 100644 (file)
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -643,7 +643,9 @@ struct ieee80211_local {
        struct crypto_blkcipher *wep_rx_tfm;
        u32 wep_iv;
 
+       /* see iface.c */
        struct list_head interfaces;
+       struct mutex iflist_mtx;
 
        /*
         * Key lock, protects sdata's key_list and sta_info's

diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 8dc2c21..00562a8 100644 (file)
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -21,6 +21,23 @@
 #include "mesh.h"
 #include "led.h"
 
+/**
+ * DOC: Interface list locking
+ *
+ * The interface list in each struct ieee80211_local is protected
+ * three-fold:
+ *
+ * (1) modifications may only be done under the RTNL
+ * (2) modifications and readers are protected against each other by
+ *     the iflist_mtx.
+ * (3) modifications are done in an RCU manner so atomic readers
+ *     can traverse the list in RCU-safe blocks.
+ *
+ * As a consequence, reads (traversals) of the list can be protected
+ * by either the RTNL, the iflist_mtx or RCU.
+ */
+
+
 static int ieee80211_change_mtu(struct net_device *dev, int new_mtu)
 {
        int meshhdrlen;
@@ -800,7 +817,9 @@ int ieee80211_if_add(struct ieee80211_local *local, const char *name,
                                            params->mesh_id_len,
                                            params->mesh_id);
 
+       mutex_lock(&local->iflist_mtx);
        list_add_tail_rcu(&sdata->list, &local->interfaces);
+       mutex_unlock(&local->iflist_mtx);
 
        if (new_dev)
                *new_dev = ndev;
@@ -816,7 +835,10 @@ void ieee80211_if_remove(struct ieee80211_sub_if_data *sdata)
 {
        ASSERT_RTNL();
 
+       mutex_lock(&sdata->local->iflist_mtx);
        list_del_rcu(&sdata->list);
+       mutex_unlock(&sdata->local->iflist_mtx);
+
        synchronize_rcu();
        unregister_netdevice(sdata->dev);
 }
@@ -832,7 +854,16 @@ void ieee80211_remove_interfaces(struct ieee80211_local *local)
        ASSERT_RTNL();
 
        list_for_each_entry_safe(sdata, tmp, &local->interfaces, list) {
+               /*
+                * we cannot hold the iflist_mtx across unregister_netdevice,
+                * but we only need to hold it for list modifications to lock
+                * out readers since we're under the RTNL here as all other
+                * writers.
+                */
+               mutex_lock(&local->iflist_mtx);
                list_del(&sdata->list);
+               mutex_unlock(&local->iflist_mtx);
+
                unregister_netdevice(sdata->dev);
        }
 }

diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index 210dfe3..a109c06 100644 (file)
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -758,6 +758,7 @@ struct ieee80211_hw *ieee80211_alloc_hw(size_t priv_data_len,
        local->hw.conf.radio_enabled = true;
 
        INIT_LIST_HEAD(&local->interfaces);
+       mutex_init(&local->iflist_mtx);
 
        spin_lock_init(&local->key_lock);
 
@@ -1008,6 +1009,8 @@ void ieee80211_free_hw(struct ieee80211_hw *hw)
 {
        struct ieee80211_local *local = hw_to_local(hw);
 
+       mutex_destroy(&local->iflist_mtx);
+
        wiphy_free(local->hw.wiphy);
 }
 EXPORT_SYMBOL(ieee80211_free_hw);

diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index fc30f29..73c7d73 100644 (file)
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -468,7 +468,7 @@ void ieee80211_iterate_active_interfaces(
        struct ieee80211_local *local = hw_to_local(hw);
        struct ieee80211_sub_if_data *sdata;
 
-       rtnl_lock();
+       mutex_lock(&local->iflist_mtx);
 
        list_for_each_entry(sdata, &local->interfaces, list) {
                switch (sdata->vif.type) {
@@ -489,7 +489,7 @@ void ieee80211_iterate_active_interfaces(
                                 &sdata->vif);
        }
 
-       rtnl_unlock();
+       mutex_unlock(&local->iflist_mtx);
 }
 EXPORT_SYMBOL_GPL(ieee80211_iterate_active_interfaces);