From 43ba7e958f2ca05e4e9171a15402288419289d71 Mon Sep 17 00:00:00 2001 From: Johannes Berg Date: Thu, 21 Feb 2008 14:09:30 +0100 Subject: [PATCH] mac80211: atomically check whether STA exists already When a STA structure is added, it is often checked whether it already exists before adding it. This, however, isn't done atomically so there is a race condition that could lead to two STA structures being added with the same MAC address. This patch changes sta_info_add() to return an ERR_PTR in case of failure and adds the failure mode -EEXIST when the STA already exists. Signed-off-by: Johannes Berg Cc: Luis Carlos Cobo Signed-off-by: John W. Linville --- net/mac80211/cfg.c | 11 ++--------- net/mac80211/ieee80211.c | 4 ++-- net/mac80211/ieee80211_sta.c | 6 +++--- net/mac80211/sta_info.c | 38 +++++++++++++++++++++++++----------- net/mac80211/sta_info.h | 4 ++-- 5 files changed, 36 insertions(+), 27 deletions(-) diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c index a083cc78855..b0c41a0cee7 100644 --- a/net/mac80211/cfg.c +++ b/net/mac80211/cfg.c @@ -562,13 +562,6 @@ static int ieee80211_add_station(struct wiphy *wiphy, struct net_device *dev, if (!netif_running(dev)) return -ENETDOWN; - /* XXX: get sta belonging to dev */ - sta = sta_info_get(local, mac); - if (sta) { - sta_info_put(sta); - return -EEXIST; - } - if (params->vlan) { sdata = IEEE80211_DEV_TO_SUB_IF(params->vlan); @@ -579,8 +572,8 @@ static int ieee80211_add_station(struct wiphy *wiphy, struct net_device *dev, sdata = IEEE80211_DEV_TO_SUB_IF(dev); sta = sta_info_add(local, dev, mac, GFP_KERNEL); - if (!sta) - return -ENOMEM; + if (IS_ERR(sta)) + return PTR_ERR(sta); sta->dev = sdata->dev; if (sdata->vif.type == IEEE80211_IF_TYPE_VLAN || diff --git a/net/mac80211/ieee80211.c b/net/mac80211/ieee80211.c index a00858dbab1..f82ebdd53d4 100644 --- a/net/mac80211/ieee80211.c +++ b/net/mac80211/ieee80211.c @@ -838,8 +838,8 @@ int ieee80211_if_update_wds(struct net_device *dev, u8 *remote_addr) /* Create STA entry for the new peer */ sta = sta_info_add(local, dev, remote_addr, GFP_KERNEL); - if (!sta) - return -ENOMEM; + if (IS_ERR(sta)) + return PTR_ERR(sta); sta->flags |= WLAN_STA_AUTHORIZED; diff --git a/net/mac80211/ieee80211_sta.c b/net/mac80211/ieee80211_sta.c index 8d620baba4f..64476d9e8d7 100644 --- a/net/mac80211/ieee80211_sta.c +++ b/net/mac80211/ieee80211_sta.c @@ -1807,9 +1807,9 @@ static void ieee80211_rx_mgmt_assoc_resp(struct ieee80211_sub_if_data *sdata, if (!sta) { struct ieee80211_sta_bss *bss; sta = sta_info_add(local, dev, ifsta->bssid, GFP_KERNEL); - if (!sta) { + if (IS_ERR(sta)) { printk(KERN_DEBUG "%s: failed to add STA entry for the" - " AP\n", dev->name); + " AP (error %ld)\n", dev->name, PTR_ERR(sta)); return; } bss = ieee80211_rx_bss_get(dev, ifsta->bssid, @@ -3820,7 +3820,7 @@ struct sta_info * ieee80211_ibss_add_sta(struct net_device *dev, wiphy_name(local->hw.wiphy), print_mac(mac, addr), dev->name); sta = sta_info_add(local, dev, addr, GFP_ATOMIC); - if (!sta) + if (IS_ERR(sta)) return NULL; sta->flags |= WLAN_STA_AUTHORIZED; diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c index b31a627ff97..c6c0df4bbd2 100644 --- a/net/mac80211/sta_info.c +++ b/net/mac80211/sta_info.c @@ -55,19 +55,29 @@ static int sta_info_hash_del(struct ieee80211_local *local, return -ENOENT; } -struct sta_info *sta_info_get(struct ieee80211_local *local, u8 *addr) +/* must hold local->sta_lock */ +static struct sta_info *__sta_info_find(struct ieee80211_local *local, + u8 *addr) { struct sta_info *sta; - read_lock_bh(&local->sta_lock); sta = local->sta_hash[STA_HASH(addr)]; while (sta) { - if (memcmp(sta->addr, addr, ETH_ALEN) == 0) { - __sta_info_get(sta); + if (compare_ether_addr(sta->addr, addr) == 0) break; - } sta = sta->hnext; } + return sta; +} + +struct sta_info *sta_info_get(struct ieee80211_local *local, u8 *addr) +{ + struct sta_info *sta; + + read_lock_bh(&local->sta_lock); + sta = __sta_info_find(local, addr); + if (sta) + __sta_info_get(sta); read_unlock_bh(&local->sta_lock); return sta; @@ -110,8 +120,8 @@ void sta_info_put(struct sta_info *sta) EXPORT_SYMBOL(sta_info_put); -struct sta_info * sta_info_add(struct ieee80211_local *local, - struct net_device *dev, u8 *addr, gfp_t gfp) +struct sta_info *sta_info_add(struct ieee80211_local *local, + struct net_device *dev, u8 *addr, gfp_t gfp) { struct sta_info *sta; int i; @@ -119,7 +129,7 @@ struct sta_info * sta_info_add(struct ieee80211_local *local, sta = kzalloc(sizeof(*sta), gfp); if (!sta) - return NULL; + return ERR_PTR(-ENOMEM); kref_init(&sta->kref); @@ -128,7 +138,7 @@ struct sta_info * sta_info_add(struct ieee80211_local *local, if (!sta->rate_ctrl_priv) { rate_control_put(sta->rate_ctrl); kfree(sta); - return NULL; + return ERR_PTR(-ENOMEM); } memcpy(sta->addr, addr, ETH_ALEN); @@ -158,9 +168,15 @@ struct sta_info * sta_info_add(struct ieee80211_local *local, } skb_queue_head_init(&sta->ps_tx_buf); skb_queue_head_init(&sta->tx_filtered); - __sta_info_get(sta); /* sta used by caller, decremented by - * sta_info_put() */ write_lock_bh(&local->sta_lock); + /* mark sta as used (by caller) */ + __sta_info_get(sta); + /* check if STA exists already */ + if (__sta_info_find(local, addr)) { + write_unlock_bh(&local->sta_lock); + sta_info_put(sta); + return ERR_PTR(-EEXIST); + } list_add(&sta->list, &local->sta_list); local->num_sta++; sta_info_hash_add(local, sta); diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h index f7e65fa3f9e..7b5be309fe0 100644 --- a/net/mac80211/sta_info.h +++ b/net/mac80211/sta_info.h @@ -239,8 +239,8 @@ static inline void __sta_info_get(struct sta_info *sta) struct sta_info * sta_info_get(struct ieee80211_local *local, u8 *addr); void sta_info_put(struct sta_info *sta); -struct sta_info * sta_info_add(struct ieee80211_local *local, - struct net_device *dev, u8 *addr, gfp_t gfp); +struct sta_info *sta_info_add(struct ieee80211_local *local, + struct net_device *dev, u8 *addr, gfp_t gfp); void sta_info_remove(struct sta_info *sta); void sta_info_free(struct sta_info *sta); void sta_info_init(struct ieee80211_local *local); -- 2.41.1