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 <johannes@sipsolutions.net>
Cc: Luis Carlos Cobo <luisca@cozybit.com>
Signed-off-by: John W. Linville <linville@tuxdriver.com>

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index a083cc78855edc39b51eb011bf1cd6c430a1c4f2..b0c41a0cee79f11e5189023fac2ee53ad455068b 100644 (file)
--- 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 a00858dbab187c36ebcd117b17980cc652e390fb..f82ebdd53d485e5e8c39dab5d2f5481627a0b7df 100644 (file)
--- 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 8d620baba4fdda4ed3a753aeab2efd18243eff02..64476d9e8d778e0cb1d9ca8086beef963b8d7075 100644 (file)
--- 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 b31a627ff97fecc4f0390e94c6a458d29819d8e8..c6c0df4bbd2c2ca5f0350289ecff3901f856630e 100644 (file)
--- 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 f7e65fa3f9ed64e13f28712fae274a9d4ff0c97a..7b5be309fe0ad13035d2e3dbf2351a1f98e469d4 100644 (file)
--- 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);