mac80211: make key locking clearer

The code in ieee80211_del_key() doesn't acquire the
key_mtx properly when it dereferences the keys. It
turns out that isn't actually necessary since the
key_mtx itself seems to be redundant since all key
manipulations are done under the RTNL, but as long
as we have the key_mtx we should use it the right
way too.

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

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index ed3400cd7a5af66a0456c360368713ffcd80a47e..94690366321cce6bf8ceb44433be191315e7cbf2 100644 (file)
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -160,13 +160,14 @@ static int ieee80211_add_key(struct wiphy *wiphy, struct net_device *dev,
 static int ieee80211_del_key(struct wiphy *wiphy, struct net_device *dev,
                             u8 key_idx, bool pairwise, const u8 *mac_addr)
 {
-       struct ieee80211_sub_if_data *sdata;
+       struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+       struct ieee80211_local *local = sdata->local;
        struct sta_info *sta;
+       struct ieee80211_key *key = NULL;
        int ret;
 
-       sdata = IEEE80211_DEV_TO_SUB_IF(dev);
-
-       mutex_lock(&sdata->local->sta_mtx);
+       mutex_lock(&local->sta_mtx);
+       mutex_lock(&local->key_mtx);
 
        if (mac_addr) {
                ret = -ENOENT;
@@ -175,33 +176,24 @@ static int ieee80211_del_key(struct wiphy *wiphy, struct net_device *dev,
                if (!sta)
                        goto out_unlock;
 
-               if (pairwise) {
-                       if (sta->ptk) {
-                               ieee80211_key_free(sdata->local, sta->ptk);
-                               ret = 0;
-                       }
-               } else {
-                       if (sta->gtk[key_idx]) {
-                               ieee80211_key_free(sdata->local,
-                                                  sta->gtk[key_idx]);
-                               ret = 0;
-                       }
-               }
-
-               goto out_unlock;
-       }
+               if (pairwise)
+                       key = sta->ptk;
+               else
+                       key = sta->gtk[key_idx];
+       } else
+               key = sdata->keys[key_idx];
 
-       if (!sdata->keys[key_idx]) {
+       if (!key) {
                ret = -ENOENT;
                goto out_unlock;
        }
 
-       ieee80211_key_free(sdata->local, sdata->keys[key_idx]);
-       WARN_ON(sdata->keys[key_idx]);
+       __ieee80211_key_free(key);
 
        ret = 0;
  out_unlock:
-       mutex_unlock(&sdata->local->sta_mtx);
+       mutex_unlock(&local->key_mtx);
+       mutex_unlock(&local->sta_mtx);
 
        return ret;
 }

diff --git a/net/mac80211/key.c b/net/mac80211/key.c
index b510721e3b3d78d55153d8cb217693403d59054b..958832dd4f0a43470400d774ae63051eda537f79 100644 (file)
--- a/net/mac80211/key.c
+++ b/net/mac80211/key.c
@@ -471,8 +471,11 @@ int ieee80211_key_link(struct ieee80211_key *key,
        return ret;
 }
 
-static void __ieee80211_key_free(struct ieee80211_key *key)
+void __ieee80211_key_free(struct ieee80211_key *key)
 {
+       if (!key)
+               return;
+
        /*
         * Replace key with nothingness if it was ever used.
         */
@@ -486,9 +489,6 @@ static void __ieee80211_key_free(struct ieee80211_key *key)
 void ieee80211_key_free(struct ieee80211_local *local,
                        struct ieee80211_key *key)
 {
-       if (!key)
-               return;
-
        mutex_lock(&local->key_mtx);
        __ieee80211_key_free(key);
        mutex_unlock(&local->key_mtx);

diff --git a/net/mac80211/key.h b/net/mac80211/key.h
index 4ddbe27eb570c8606a4f6a3aaeece9970324905d..e5432ef8b203ce16134d37f3408c73923559d0b2 100644 (file)
--- a/net/mac80211/key.h
+++ b/net/mac80211/key.h
@@ -135,6 +135,7 @@ struct ieee80211_key *ieee80211_key_alloc(u32 cipher, int idx, size_t key_len,
 int __must_check ieee80211_key_link(struct ieee80211_key *key,
                                    struct ieee80211_sub_if_data *sdata,
                                    struct sta_info *sta);
+void __ieee80211_key_free(struct ieee80211_key *key);
 void ieee80211_key_free(struct ieee80211_local *local,
                        struct ieee80211_key *key);
 void ieee80211_set_default_key(struct ieee80211_sub_if_data *sdata, int idx,