Skip to contentSkip to conversation

mac80211: simplify key locking

PATCH In review Johannes Berg 2 participants 10 messages Jun 1, 2010 – Jul 27, 2010 mac80211

lore
Share & tools
lore

Johannes Berg

author maintainer
#0 lore raw

PATCH 1/1 mac80211: simplify key locking

From: Johannes Berg <[email protected]>

Since I recently made station management able to sleep, I can now rework key management as well; since it will no longer need a spinlock and can also use a mutex instead, a bunch of code to allow drivers' set_key to sleep while key management is protected by a spinlock can now be removed.

Signed-off-by Johannes Berg <[email protected]>
---
 net/mac80211/cfg.c         |   18 +-
 net/mac80211/ieee80211_i.h |    4 
 net/mac80211/iface.c       |    5 
 net/mac80211/key.c         |  288 ++++++++++-----------------------------------
 net/mac80211/key.h         |   22 ---
 net/mac80211/main.c        |    2 
 net/mac80211/sta_info.c    |    8 -
 7 files changed, 80 insertions(+), 267 deletions(-)
net/mac80211/cfg.c +8 -10
@@ -120,6 +120,9 @@ static int ieee80211_add_key(struct wiph
 	struct ieee80211_key *key;
 	int err;
 
+	if (!netif_running(dev))
+		return -ENETDOWN;
+
 	sdata = IEEE80211_DEV_TO_SUB_IF(dev);
 
 	switch (params->cipher) {

@@ -145,7 +148,7 @@ static int ieee80211_add_key(struct wiph
 	if (!key)
 		return -ENOMEM;
 
-	rcu_read_lock();
+	mutex_lock(&sdata->local->sta_mtx);
 
 	if (mac_addr) {
 		sta = sta_info_get_bss(sdata, mac_addr);

@@ -160,7 +163,7 @@ static int ieee80211_add_key(struct wiph
 
 	err = 0;
  out_unlock:
-	rcu_read_unlock();
+	mutex_unlock(&sdata->local->sta_mtx);
 
 	return err;
 }

@@ -174,7 +177,7 @@ static int ieee80211_del_key(struct wiph
 
 	sdata = IEEE80211_DEV_TO_SUB_IF(dev);
 
-	rcu_read_lock();
+	mutex_lock(&sdata->local->sta_mtx);
 
 	if (mac_addr) {
 		ret = -ENOENT;

@@ -202,7 +205,7 @@ static int ieee80211_del_key(struct wiph
 
 	ret = 0;
  out_unlock:
-	rcu_read_unlock();
+	mutex_unlock(&sdata->local->sta_mtx);
 
 	return ret;
 }

@@ -305,15 +308,10 @@ static int ieee80211_config_default_key(
 					struct net_device *dev,
 					u8 key_idx)
 {
-	struct ieee80211_sub_if_data *sdata;
+	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
 
-	rcu_read_lock();
-
-	sdata = IEEE80211_DEV_TO_SUB_IF(dev);
 	ieee80211_set_default_key(sdata, key_idx);
 
-	rcu_read_unlock();
-
 	return 0;
 }
net/mac80211/ieee80211_i.h +2 -2
@@ -746,10 +746,10 @@ struct ieee80211_local {
 	struct mutex iflist_mtx;
 
 	/*
-	 * Key lock, protects sdata's key_list and sta_info's
+	 * Key mutex, protects sdata's key_list and sta_info's
 	 * key pointers (write access, they're RCU.)
 	 */
-	spinlock_t key_lock;
+	struct mutex key_mtx;
 
 
 	/* Scanning and BSS list */
net/mac80211/key.c +66 -222

453 lines not on this page · raw message

net/mac80211/main.c +1 -1
@@ -396,7 +396,7 @@ struct ieee80211_hw *ieee80211_alloc_hw(
 	mutex_init(&local->iflist_mtx);
 	mutex_init(&local->scan_mtx);
 
-	spin_lock_init(&local->key_lock);
+	mutex_init(&local->key_mtx);
 	spin_lock_init(&local->filter_lock);
 	spin_lock_init(&local->queue_stop_reason_lock);
net/mac80211/key.h +1 -21
@@ -38,25 +38,9 @@ struct sta_info;
  *
  * @KEY_FLAG_UPLOADED_TO_HARDWARE: Indicates that this key is present
  *	in the hardware for TX crypto hardware acceleration.
- * @KEY_FLAG_TODO_DELETE: Key is marked for deletion and will, after an
- *	RCU grace period, no longer be reachable other than from the
- *	todo list.
- * @KEY_FLAG_TODO_HWACCEL_ADD: Key needs to be added to hardware acceleration.
- * @KEY_FLAG_TODO_HWACCEL_REMOVE: Key needs to be removed from hardware
- *	acceleration.
- * @KEY_FLAG_TODO_DEFKEY: Key is default key and debugfs needs to be updated.
- * @KEY_FLAG_TODO_ADD_DEBUGFS: Key needs to be added to debugfs.
- * @KEY_FLAG_TODO_DEFMGMTKEY: Key is default management key and debugfs needs
- *	to be updated.
  */
 enum ieee80211_internal_key_flags {
 	KEY_FLAG_UPLOADED_TO_HARDWARE	= BIT(0),
-	KEY_FLAG_TODO_DELETE		= BIT(1),
-	KEY_FLAG_TODO_HWACCEL_ADD	= BIT(2),
-	KEY_FLAG_TODO_HWACCEL_REMOVE	= BIT(3),
-	KEY_FLAG_TODO_DEFKEY		= BIT(4),
-	KEY_FLAG_TODO_ADD_DEBUGFS	= BIT(5),
-	KEY_FLAG_TODO_DEFMGMTKEY	= BIT(6),
 };
 
 enum ieee80211_internal_tkip_state {

@@ -79,10 +63,8 @@ struct ieee80211_key {
 
 	/* for sdata list */
 	struct list_head list;
-	/* for todo list */
-	struct list_head todo;
 
-	/* protected by todo lock! */
+	/* protected by key mutex */
 	unsigned int flags;
 
 	union {

@@ -155,6 +137,4 @@ void ieee80211_free_keys(struct ieee8021
 void ieee80211_enable_keys(struct ieee80211_sub_if_data *sdata);
 void ieee80211_disable_keys(struct ieee80211_sub_if_data *sdata);
 
-void ieee80211_key_todo(void);
-
 #endif /* IEEE80211_KEY_H */
net/mac80211/sta_info.c +0 -8
@@ -648,14 +648,6 @@ static int __must_check __sta_info_destr
 
 	if (sta->key) {
 		ieee80211_key_free(sta->key);
-		/*
-		 * We have only unlinked the key, and actually destroying it
-		 * may mean it is removed from hardware which requires that
-		 * the key->sta pointer is still valid, so flush the key todo
-		 * list here.
-		 */
-		ieee80211_key_todo();
-
 		WARN_ON(sta->key);
 	}
net/mac80211/iface.c +2 -3
@@ -268,7 +268,6 @@ static int ieee80211_open(struct net_dev
 
 		changed |= ieee80211_reset_erp_info(sdata);
 		ieee80211_bss_info_change_notify(sdata, changed);
-		ieee80211_enable_keys(sdata);
 
 		if (sdata->vif.type == NL80211_IFTYPE_STATION)
 			netif_carrier_off(dev);

@@ -522,8 +521,8 @@ static int ieee80211_stop(struct net_dev
 				BSS_CHANGED_BEACON_ENABLED);
 		}
 
-		/* disable all keys for as long as this netdev is down */
-		ieee80211_disable_keys(sdata);
+		/* free all remaining keys, there shouldn't be any */
+		ieee80211_free_keys(sdata);
 		drv_remove_interface(local, &sdata->vif);
 	}
#1 lore raw
Quoted 1 line · Johannes Berg

net/mac80211/key.c

commit ad0e2b5a00dbec303e4682b403bb6703d11dcdb2

net/mac80211/key.c static void __ieee80211_key_free(struct · line 474

Quoted 5 lines

void ieee80211_key_free(struct ieee80211_key *key)

net/mac80211/key.c +0 -3
-	if (!key->sdata) {
-		/* The key has not been linked yet, simply free it
-		 * and don't Oops */

It looks like this function can still be called with key->sdata == NULL in some cases and that does indeed result in an oops below:

net/mac80211/key.c static void __ieee80211_key_free(struct · line 420

Quoted 1 line
net/mac80211/key.c +1 -0
+	local = key->sdata->local;

I've seen this issue pop up in FT testing and based on a quick code review, at least ieee80211_add_key() calls ieee80211_key_free() in an error case (sta not found) without having first called ieee80211_key_link(). Which one is wrong - the caller or the freeing function? Should we just restore the previous key->sdata == NULL handler here?

I'd guess that the sta-not-found part is caused by FT trying to configure PTK before association in the FT protocol roaming case (wpa_supplicant tries to do this as soon as it knows the key because that works with some drivers and is needed to avoid race condition with encrypted frames; as a workaround, it will do this again after association if the previous attempt failed).

Signature · 1 line
Jouni Malinen                                            PGP id EFC895FA
author maintainer
#2 lore raw
Quoted 14 lines · Jouni Malinen
Quoted 1 line · Johannes Berg

net/mac80211/key.c

commit ad0e2b5a00dbec303e4682b403bb6703d11dcdb2

net/mac80211/key.c static void __ieee80211_key_free(struct · line 474

Quoted 5 lines

void ieee80211_key_free(struct ieee80211_key *key)

net/mac80211/key.c +0 -3
-	if (!key->sdata) {
-		/* The key has not been linked yet, simply free it
-		 * and don't Oops */

It looks like this function can still be called with key->sdata == NULL in some cases and that does indeed result in an oops below:

Quoted 9 lines

net/mac80211/key.c static void __ieee80211_key_free(struct · line 420

Quoted 1 line
net/mac80211/key.c +1 -0
+	local = key->sdata->local;

I've seen this issue pop up in FT testing and based on a quick code review, at least ieee80211_add_key() calls ieee80211_key_free() in an error case (sta not found) without having first called ieee80211_key_link(). Which one is wrong - the caller or the freeing function? Should we just restore the previous key->sdata == NULL handler here?

I think it's just another stupid bug.

I removed the comment because the linked vs. not linked handling is a bit different now I think ... I don't think we should restore the NULL handling as it was before, since __ieee80211_key_free() should be able to handle this now.

The fix should be passing in the local pointer to ieee80211_key_free() I guess. Can you try that?

johannes

#3 lore raw
Quoted 7 lines · Johannes Berg

I removed the comment because the linked vs. not linked handling is a bit different now I think ... I don't think we should restore the NULL handling as it was before, since __ieee80211_key_free() should be able to handle this now.

The fix should be passing in the local pointer to ieee80211_key_free() I guess. Can you try that?

__ieee80211_key_free() handles this, but the function it calls does not.. ieee80211_key_disable_hw_accel() dereferences key->local before checking whether the key has been uploaded to hardware (which this key would not be).. Would you like the local pointer to be passed to ieee80211_key_disable_hw_accel(), too? Or make __ieee80211_key_destroy() skip that call if key->local == NULL? Actually, calling __ieee80211_key_destroy() with not-yet-linked key is somewhat odd from the debugfs view point, too (ieee80211_debugfs_key_remove() gets called even when matching _key_add has not happened).

Signature · 1 line
Jouni Malinen                                            PGP id EFC895FA
#4 lore raw
In reply to Johannes Berg
Quoted 2 lines · Johannes Berg

The fix should be passing in the local pointer to ieee80211_key_free() I guess. Can you try that?

The following seems to be more or less minimal patch with that approach to get rid of this oops with FT protocol.. Is this what you were looking for?

---
 net/mac80211/cfg.c      |    6 +++---
 net/mac80211/key.c      |   13 ++++++-------
 net/mac80211/key.h      |    3 ++-
 net/mac80211/sta_info.c |    2 +-
 4 files changed, 12 insertions(+), 12 deletions(-)
net/mac80211/cfg.c +3 -3
@@ -158,7 +158,7 @@ static int ieee80211_add_key(struct wiph
 	if (mac_addr) {
 		sta = sta_info_get_bss(sdata, mac_addr);
 		if (!sta) {
-			ieee80211_key_free(key);
+			ieee80211_key_free(sdata->local, key);
 			err = -ENOENT;
 			goto out_unlock;
 		}

@@ -192,7 +192,7 @@ static int ieee80211_del_key(struct wiph
 			goto out_unlock;
 
 		if (sta->key) {
-			ieee80211_key_free(sta->key);
+			ieee80211_key_free(sdata->local, sta->key);
 			WARN_ON(sta->key);
 			ret = 0;
 		}

@@ -205,7 +205,7 @@ static int ieee80211_del_key(struct wiph
 		goto out_unlock;
 	}
 
-	ieee80211_key_free(sdata->keys[key_idx]);
+	ieee80211_key_free(sdata->local, sdata->keys[key_idx]);
 	WARN_ON(sdata->keys[key_idx]);
 
 	ret = 0;
net/mac80211/key.c +6 -7
@@ -323,13 +323,15 @@ static void __ieee80211_key_destroy(stru
 	if (!key)
 		return;
 
-	ieee80211_key_disable_hw_accel(key);
+	if (key->local)
+		ieee80211_key_disable_hw_accel(key);
 
 	if (key->conf.alg == ALG_CCMP)
 		ieee80211_aes_key_free(key->u.ccmp.tfm);
 	if (key->conf.alg == ALG_AES_CMAC)
 		ieee80211_aes_cmac_key_free(key->u.aes_cmac.tfm);
-	ieee80211_debugfs_key_remove(key);
+	if (key->local)
+		ieee80211_debugfs_key_remove(key);
 
 	kfree(key);
 }

@@ -410,15 +412,12 @@ static void __ieee80211_key_free(struct 
 	__ieee80211_key_destroy(key);
 }
 
-void ieee80211_key_free(struct ieee80211_key *key)
+void ieee80211_key_free(struct ieee80211_local *local,
+			struct ieee80211_key *key)
 {
-	struct ieee80211_local *local;
-
 	if (!key)
 		return;
 
-	local = key->sdata->local;
-
 	mutex_lock(&local->key_mtx);
 	__ieee80211_key_free(key);
 	mutex_unlock(&local->key_mtx);
net/mac80211/key.h +2 -1
@@ -135,7 +135,8 @@ struct ieee80211_key *ieee80211_key_allo
 void 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);
 void ieee80211_set_default_mgmt_key(struct ieee80211_sub_if_data *sdata,
 				    int idx);
net/mac80211/sta_info.c +1 -1
@@ -647,7 +647,7 @@ static int __must_check __sta_info_destr
 		return ret;
 
 	if (sta->key) {
-		ieee80211_key_free(sta->key);
+		ieee80211_key_free(local, sta->key);
 		WARN_ON(sta->key);
 	}
Signature · 1 line
Jouni Malinen                                            PGP id EFC895FA
author maintainer
#5 lore raw
Quoted 3 lines · Jouni Malinen

The following seems to be more or less minimal patch with that approach to get rid of this oops with FT protocol.. Is this what you were looking for?

Well so to answer both your emails :-)

net/mac80211/key.c static void __ieee80211_key_destroy(stru

Quoted 8 lines
net/mac80211/key.c +4 -2
-	ieee80211_key_disable_hw_accel(key);
+	if (key->local)
+		ieee80211_key_disable_hw_accel(key);
 
 
-	ieee80211_debugfs_key_remove(key);
+	if (key->local)
+		ieee80211_debugfs_key_remove(key);

These might look odd, but they're fine on a key that hasn't been used since it couldn't have been uploaded to hardware, or put into debugfs, so they'll just exit right away.

The rest of the patch is exactly what I was thinking of.

johannes

#6 lore raw
Quoted 6 lines · Johannes Berg
Quoted 3 lines · Jouni Malinen
net/mac80211/key.c +2 -1
-	ieee80211_key_disable_hw_accel(key);
+	if (key->local)
+		ieee80211_key_disable_hw_accel(key);
Quoted 7 lines

net/mac80211/key.c static void __ieee80211_key_destroy(stru · line 334

Quoted 3 lines
net/mac80211/key.c +2 -1
-	ieee80211_debugfs_key_remove(key);
+	if (key->local)
+		ieee80211_debugfs_key_remove(key);

These might look odd, but they're fine on a key that hasn't been used since it couldn't have been uploaded to hardware, or put into debugfs, so they'll just exit right away.

Well, the latter one may indeed do that since debugfs_remove_recursive() survives NULL pointer. However, the former one does not. ieee80211_key_disable_hw_accel() has a key->local dereference and it oopses without the "if (key->local)" part here (before checking whether the key is uploaded to hardware). I started first making that handle unlinked keys, but since this gets called before ieee80211_key_enable_hw_accel() in the problem case, it looked more logical to fix the caller not to get to the disable function in the first place.

Signature · 1 line
Jouni Malinen                                            PGP id EFC895FA
author maintainer
#7 lore raw
Quoted 9 lines · Jouni Malinen

Well, the latter one may indeed do that since debugfs_remove_recursive() survives NULL pointer. However, the former one does not. ieee80211_key_disable_hw_accel() has a key->local dereference and it oopses without the "if (key->local)" part here (before checking whether the key is uploaded to hardware). I started first making that handle unlinked keys, but since this gets called before ieee80211_key_enable_hw_accel() in the problem case, it looked more logical to fix the caller not to get to the disable function in the first place.

Oh, indeed, that comes before the check. I agree then, this patch seems like the best fix.

johannes

#8 lore raw

PATCH 1/1 mac80211: Fix key freeing to handle unlinked keys

Key locking simplification removed key->sdata != NULL verification from ieee80211_key_free(). While that is fine for most use cases, there is one path where this function can be called with an unlinked key (i.e., key->sdata == NULL && key->local == NULL). This results in a NULL pointer dereference with the current implementation. This is known to happen at least with FT protocol when wpa_supplicant tries to configure the key before association.

Avoid the issue by passing in the local pointer to ieee80211_key_free(). In addition, do not clear the key from hw_accel or debugfs if it has not yet been added. At least the hw_accel one could trigger another NULL pointer dereference.

Signed-off-by Jouni Malinen <[email protected]>
---
 net/mac80211/cfg.c      |    6 +++---
 net/mac80211/key.c      |   13 ++++++-------
 net/mac80211/key.h      |    3 ++-
 net/mac80211/sta_info.c |    2 +-
 4 files changed, 12 insertions(+), 12 deletions(-)

(Note: The reference to key locking simplification is to the commit ad0e2b5a00dbec303e4682b403bb6703d11dcdb2 which is not yet in linux-2.6.)

net/mac80211/cfg.c +3 -3
@@ -158,7 +158,7 @@ static int ieee80211_add_key(struct wiph
 	if (mac_addr) {
 		sta = sta_info_get_bss(sdata, mac_addr);
 		if (!sta) {
-			ieee80211_key_free(key);
+			ieee80211_key_free(sdata->local, key);
 			err = -ENOENT;
 			goto out_unlock;
 		}

@@ -192,7 +192,7 @@ static int ieee80211_del_key(struct wiph
 			goto out_unlock;
 
 		if (sta->key) {
-			ieee80211_key_free(sta->key);
+			ieee80211_key_free(sdata->local, sta->key);
 			WARN_ON(sta->key);
 			ret = 0;
 		}

@@ -205,7 +205,7 @@ static int ieee80211_del_key(struct wiph
 		goto out_unlock;
 	}
 
-	ieee80211_key_free(sdata->keys[key_idx]);
+	ieee80211_key_free(sdata->local, sdata->keys[key_idx]);
 	WARN_ON(sdata->keys[key_idx]);
 
 	ret = 0;
net/mac80211/key.c +6 -7
@@ -323,13 +323,15 @@ static void __ieee80211_key_destroy(stru
 	if (!key)
 		return;
 
-	ieee80211_key_disable_hw_accel(key);
+	if (key->local)
+		ieee80211_key_disable_hw_accel(key);
 
 	if (key->conf.alg == ALG_CCMP)
 		ieee80211_aes_key_free(key->u.ccmp.tfm);
 	if (key->conf.alg == ALG_AES_CMAC)
 		ieee80211_aes_cmac_key_free(key->u.aes_cmac.tfm);
-	ieee80211_debugfs_key_remove(key);
+	if (key->local)
+		ieee80211_debugfs_key_remove(key);
 
 	kfree(key);
 }

@@ -410,15 +412,12 @@ static void __ieee80211_key_free(struct 
 	__ieee80211_key_destroy(key);
 }
 
-void ieee80211_key_free(struct ieee80211_key *key)
+void ieee80211_key_free(struct ieee80211_local *local,
+			struct ieee80211_key *key)
 {
-	struct ieee80211_local *local;
-
 	if (!key)
 		return;
 
-	local = key->sdata->local;
-
 	mutex_lock(&local->key_mtx);
 	__ieee80211_key_free(key);
 	mutex_unlock(&local->key_mtx);
net/mac80211/key.h +2 -1
@@ -135,7 +135,8 @@ struct ieee80211_key *ieee80211_key_allo
 void 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);
 void ieee80211_set_default_mgmt_key(struct ieee80211_sub_if_data *sdata,
 				    int idx);
net/mac80211/sta_info.c +1 -1
@@ -647,7 +647,7 @@ static int __must_check __sta_info_destr
 		return ret;
 
 	if (sta->key) {
-		ieee80211_key_free(sta->key);
+		ieee80211_key_free(local, sta->key);
 		WARN_ON(sta->key);
 	}
Signature · 1 line
Jouni Malinen                                            PGP id EFC895FA
author maintainer Reviewed-by on v1 1/1
#9 lore raw

Re: [PATCH] mac80211: Fix key freeing to handle unlinked keys

Quoted 8 lines · Jouni Malinen

Key locking simplification removed key->sdata != NULL verification from ieee80211_key_free(). While that is fine for most use cases, there is one path where this function can be called with an unlinked key (i.e., key->sdata == NULL && key->local == NULL). This results in a NULL pointer dereference with the current implementation. This is known to happen at least with FT protocol when wpa_supplicant tries to configure the key before association.

Quoted 6 lines

Avoid the issue by passing in the local pointer to ieee80211_key_free(). In addition, do not clear the key from hw_accel or debugfs if it has not yet been added. At least the hw_accel one could trigger another NULL pointer dereference.

Signed-off-by Jouni Malinen <[email protected]>
Reviewed-by Johannes Berg <[email protected]>
Quoted 9 lines
---
 net/mac80211/cfg.c      |    6 +++---
 net/mac80211/key.c      |   13 ++++++-------
 net/mac80211/key.h      |    3 ++-
 net/mac80211/sta_info.c |    2 +-
 4 files changed, 12 insertions(+), 12 deletions(-)

(Note: The reference to key locking simplification is to the commit ad0e2b5a00dbec303e4682b403bb6703d11dcdb2 which is not yet in linux-2.6.)

Yes, this is because it's in wireless-next-2.6, the commit ID should still be stable.

johannes

Quoted 90 lines
net/mac80211/cfg.c +3 -3
@@ -158,7 +158,7 @@ static int ieee80211_add_key(struct wiph
 	if (mac_addr) {
 		sta = sta_info_get_bss(sdata, mac_addr);
 		if (!sta) {
-			ieee80211_key_free(key);
+			ieee80211_key_free(sdata->local, key);
 			err = -ENOENT;
 			goto out_unlock;
 		}

@@ -192,7 +192,7 @@ static int ieee80211_del_key(struct wiph
 			goto out_unlock;
 
 		if (sta->key) {
-			ieee80211_key_free(sta->key);
+			ieee80211_key_free(sdata->local, sta->key);
 			WARN_ON(sta->key);
 			ret = 0;
 		}

@@ -205,7 +205,7 @@ static int ieee80211_del_key(struct wiph
 		goto out_unlock;
 	}
 
-	ieee80211_key_free(sdata->keys[key_idx]);
+	ieee80211_key_free(sdata->local, sdata->keys[key_idx]);
 	WARN_ON(sdata->keys[key_idx]);
 
 	ret = 0;
net/mac80211/key.c +6 -7
@@ -323,13 +323,15 @@ static void __ieee80211_key_destroy(stru
 	if (!key)
 		return;
 
-	ieee80211_key_disable_hw_accel(key);
+	if (key->local)
+		ieee80211_key_disable_hw_accel(key);
 
 	if (key->conf.alg == ALG_CCMP)
 		ieee80211_aes_key_free(key->u.ccmp.tfm);
 	if (key->conf.alg == ALG_AES_CMAC)
 		ieee80211_aes_cmac_key_free(key->u.aes_cmac.tfm);
-	ieee80211_debugfs_key_remove(key);
+	if (key->local)
+		ieee80211_debugfs_key_remove(key);
 
 	kfree(key);
 }

@@ -410,15 +412,12 @@ static void __ieee80211_key_free(struct 
 	__ieee80211_key_destroy(key);
 }
 
-void ieee80211_key_free(struct ieee80211_key *key)
+void ieee80211_key_free(struct ieee80211_local *local,
+			struct ieee80211_key *key)
 {
-	struct ieee80211_local *local;
-
 	if (!key)
 		return;
 
-	local = key->sdata->local;
-
 	mutex_lock(&local->key_mtx);
 	__ieee80211_key_free(key);
 	mutex_unlock(&local->key_mtx);
net/mac80211/key.h +2 -1
@@ -135,7 +135,8 @@ struct ieee80211_key *ieee80211_key_allo
 void 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);
 void ieee80211_set_default_mgmt_key(struct ieee80211_sub_if_data *sdata,
 				    int idx);
net/mac80211/sta_info.c +1 -1
@@ -647,7 +647,7 @@ static int __must_check __sta_info_destr
 		return ret;
 
 	if (sta->key) {
-		ieee80211_key_free(sta->key);
+		ieee80211_key_free(local, sta->key);
 		WARN_ON(sta->key);
 	}