Skip to content

Commit

Permalink
mac80211: make key locking clearer
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Johannes Berg authored and John W. Linville committed May 12, 2011
1 parent dea4096 commit 5c0c364
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 27 deletions.
38 changes: 15 additions & 23 deletions net/mac80211/cfg.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
}
Expand Down
8 changes: 4 additions & 4 deletions net/mac80211/key.c
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand All @@ -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);
Expand Down
1 change: 1 addition & 0 deletions net/mac80211/key.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 5c0c364

Please sign in to comment.