Commit 32162a4dab0e6a4ca7f886a01173b5f9b80843be
Committed by
John W. Linville
John W. Linville
1 parent
1b2fb7dc71
Exists in
master
and in
7 other branches
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 <j@w1.fi> Reviewed-by: Johannes Berg <johannes@sipsolutions.net> Signed-off-by: John W. Linville <linville@tuxdriver.com>
Showing 4 changed files with 12 additions and 12 deletions Side-by-side Diff
net/mac80211/cfg.c
| ... | ... | @@ -158,7 +158,7 @@ |
| 158 | 158 | if (mac_addr) { |
| 159 | 159 | sta = sta_info_get_bss(sdata, mac_addr); |
| 160 | 160 | if (!sta) { |
| 161 | - ieee80211_key_free(key); | |
| 161 | + ieee80211_key_free(sdata->local, key); | |
| 162 | 162 | err = -ENOENT; |
| 163 | 163 | goto out_unlock; |
| 164 | 164 | } |
| ... | ... | @@ -192,7 +192,7 @@ |
| 192 | 192 | goto out_unlock; |
| 193 | 193 | |
| 194 | 194 | if (sta->key) { |
| 195 | - ieee80211_key_free(sta->key); | |
| 195 | + ieee80211_key_free(sdata->local, sta->key); | |
| 196 | 196 | WARN_ON(sta->key); |
| 197 | 197 | ret = 0; |
| 198 | 198 | } |
| ... | ... | @@ -205,7 +205,7 @@ |
| 205 | 205 | goto out_unlock; |
| 206 | 206 | } |
| 207 | 207 | |
| 208 | - ieee80211_key_free(sdata->keys[key_idx]); | |
| 208 | + ieee80211_key_free(sdata->local, sdata->keys[key_idx]); | |
| 209 | 209 | WARN_ON(sdata->keys[key_idx]); |
| 210 | 210 | |
| 211 | 211 | ret = 0; |
net/mac80211/key.c
| ... | ... | @@ -323,13 +323,15 @@ |
| 323 | 323 | if (!key) |
| 324 | 324 | return; |
| 325 | 325 | |
| 326 | - ieee80211_key_disable_hw_accel(key); | |
| 326 | + if (key->local) | |
| 327 | + ieee80211_key_disable_hw_accel(key); | |
| 327 | 328 | |
| 328 | 329 | if (key->conf.alg == ALG_CCMP) |
| 329 | 330 | ieee80211_aes_key_free(key->u.ccmp.tfm); |
| 330 | 331 | if (key->conf.alg == ALG_AES_CMAC) |
| 331 | 332 | ieee80211_aes_cmac_key_free(key->u.aes_cmac.tfm); |
| 332 | - ieee80211_debugfs_key_remove(key); | |
| 333 | + if (key->local) | |
| 334 | + ieee80211_debugfs_key_remove(key); | |
| 333 | 335 | |
| 334 | 336 | kfree(key); |
| 335 | 337 | } |
| 336 | 338 | |
| 337 | 339 | |
| ... | ... | @@ -410,14 +412,11 @@ |
| 410 | 412 | __ieee80211_key_destroy(key); |
| 411 | 413 | } |
| 412 | 414 | |
| 413 | -void ieee80211_key_free(struct ieee80211_key *key) | |
| 415 | +void ieee80211_key_free(struct ieee80211_local *local, | |
| 416 | + struct ieee80211_key *key) | |
| 414 | 417 | { |
| 415 | - struct ieee80211_local *local; | |
| 416 | - | |
| 417 | 418 | if (!key) |
| 418 | 419 | return; |
| 419 | - | |
| 420 | - local = key->sdata->local; | |
| 421 | 420 | |
| 422 | 421 | mutex_lock(&local->key_mtx); |
| 423 | 422 | __ieee80211_key_free(key); |
net/mac80211/key.h
| ... | ... | @@ -135,7 +135,8 @@ |
| 135 | 135 | void ieee80211_key_link(struct ieee80211_key *key, |
| 136 | 136 | struct ieee80211_sub_if_data *sdata, |
| 137 | 137 | struct sta_info *sta); |
| 138 | -void ieee80211_key_free(struct ieee80211_key *key); | |
| 138 | +void ieee80211_key_free(struct ieee80211_local *local, | |
| 139 | + struct ieee80211_key *key); | |
| 139 | 140 | void ieee80211_set_default_key(struct ieee80211_sub_if_data *sdata, int idx); |
| 140 | 141 | void ieee80211_set_default_mgmt_key(struct ieee80211_sub_if_data *sdata, |
| 141 | 142 | int idx); |