blob: 57e3f42bf61b6a31f4d22047f4d9f73c0c116456 [file] [log] [blame]
From fbe1a7b8e20af7035a611242f14f15b805b552eb Mon Sep 17 00:00:00 2001
From: Avery Pennarun <apenwarr@gmail.com>
Date: Thu, 25 Sep 2014 01:31:56 -0400
Subject: [PATCH] mwifiex: eliminate a crash on hostapd shutdown.
The way sta_list_spinlock is used is highly suspect and there are definitely
race conditions. mwifiex_cfg80211_del_station is called from a netlink
callback at the same time as mwifiex_del_sta_entry is called from the SDIO
thread, and they both try to delete all stations, but in different ways.
The net result is they could end up both modifying sta_list at the same time
and causing a crash. This patch fixes the one particular crash case we see,
by referring to a station by its MAC address instead of a pointer to its
station structure. That way, when a station is deleted by surprise in
another thread, we just have a use-after-free bug on the MAC address array,
which quietly fails a subsequent lookup of the node structure but at least
doesn't crash.
The remaining symptom of the race condition is that stations are not
actually deauthorized at hostapd shutdown (because one of the code paths
does not send the firmware a deauth request), which is surprising. They
are, however, deauthorized when hostapd restarts, so the problem isn't too
serious.
A better fix would probably involve totally rethinking the locking in
mwifiex. There is little need for it to use such fine-grained locking,
especially around administrative structures like the station list.
---
drivers/net/wireless/mwifiex/cfg80211.c | 16 +++++++++++-----
drivers/net/wireless/mwifiex/main.h | 3 +--
drivers/net/wireless/mwifiex/uap_event.c | 10 +++++-----
3 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/drivers/net/wireless/mwifiex/cfg80211.c b/drivers/net/wireless/mwifiex/cfg80211.c
index 5013a69..7e45ada 100644
--- a/drivers/net/wireless/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/mwifiex/cfg80211.c
@@ -1289,13 +1289,19 @@ mwifiex_cfg80211_del_station(struct wiphy *wiphy, struct net_device *dev,
if (!mac || is_broadcast_ether_addr(mac)) {
wiphy_dbg(wiphy, "%s: NULL/broadcast mac address\n", __func__);
- list_for_each_entry(sta_node, &priv->sta_list, list) {
+ do {
+ spin_lock_irqsave(&priv->sta_list_spinlock, flags);
+ sta_node = list_first_entry_or_null(&priv->sta_list, struct mwifiex_sta_node, list);
+ mac = sta_node ? sta_node->mac_addr : NULL;
+ spin_unlock_irqrestore(&priv->sta_list_spinlock, flags);
+ if (!mac) break;
if (mwifiex_send_cmd(priv, HostCmd_CMD_UAP_STA_DEAUTH,
HostCmd_ACT_GEN_SET, 0,
- sta_node->mac_addr, true))
+ mac, true)) {
return -1;
- mwifiex_uap_del_sta_data(priv, sta_node);
- }
+ }
+ mwifiex_uap_del_sta_data(priv, mac);
+ } while (1);
} else {
wiphy_dbg(wiphy, "%s: mac address %pM\n", __func__, mac);
spin_lock_irqsave(&priv->sta_list_spinlock, flags);
@@ -1306,7 +1312,7 @@ mwifiex_cfg80211_del_station(struct wiphy *wiphy, struct net_device *dev,
HostCmd_ACT_GEN_SET, 0,
sta_node->mac_addr, true))
return -1;
- mwifiex_uap_del_sta_data(priv, sta_node);
+ mwifiex_uap_del_sta_data(priv, mac);
}
}
diff --git a/drivers/net/wireless/mwifiex/main.h b/drivers/net/wireless/mwifiex/main.h
index 58261f9..fab2c58 100644
--- a/drivers/net/wireless/mwifiex/main.h
+++ b/drivers/net/wireless/mwifiex/main.h
@@ -1209,8 +1209,7 @@ int mwifiex_set_mgmt_ies(struct mwifiex_private *priv,
struct cfg80211_beacon_data *data);
int mwifiex_del_mgmt_ies(struct mwifiex_private *priv);
u8 *mwifiex_11d_code_2_region(u8 code);
-void mwifiex_uap_del_sta_data(struct mwifiex_private *priv,
- struct mwifiex_sta_node *node);
+void mwifiex_uap_del_sta_data(struct mwifiex_private *priv, u8 *mac);
void mwifiex_11h_process_join(struct mwifiex_private *priv, u8 **buffer,
struct mwifiex_bssdescriptor *bss_desc);
diff --git a/drivers/net/wireless/mwifiex/uap_event.c b/drivers/net/wireless/mwifiex/uap_event.c
index 92e77a3..07e1689 100644
--- a/drivers/net/wireless/mwifiex/uap_event.c
+++ b/drivers/net/wireless/mwifiex/uap_event.c
@@ -186,13 +186,13 @@ int mwifiex_process_uap_event(struct mwifiex_private *priv)
* tables created for this station are deleted.
*/
void mwifiex_uap_del_sta_data(struct mwifiex_private *priv,
- struct mwifiex_sta_node *node)
+ u8 *mac)
{
- if (priv->ap_11n_enabled && node->is_11n_enabled) {
- mwifiex_11n_del_rx_reorder_tbl_by_ta(priv, node->mac_addr);
- mwifiex_del_tx_ba_stream_tbl_by_ra(priv, node->mac_addr);
+ if (priv->ap_11n_enabled) {
+ mwifiex_11n_del_rx_reorder_tbl_by_ta(priv, mac);
+ mwifiex_del_tx_ba_stream_tbl_by_ra(priv, mac);
}
- mwifiex_del_sta_entry(priv, node->mac_addr);
+ mwifiex_del_sta_entry(priv, mac);
return;
}
--
2.1.0.rc2.206.gedb03e5