diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c index 6b183a3526b0ae2f4f0b0c703d1686a2d2bc5222..fbd462c78e185dc70be6cecfaffc9e0e535e371b 100644 --- a/net/mac80211/cfg.c +++ b/net/mac80211/cfg.c @@ -672,7 +672,7 @@ static int ieee80211_add_station(struct wiphy *wiphy, struct net_device *dev, err = sta_info_insert(sta); if (err) { - sta_info_destroy(sta); + /* STA has been freed */ rcu_read_unlock(); return err; } diff --git a/net/mac80211/ieee80211.c b/net/mac80211/ieee80211.c index 8c0f782d21e361a20fe1b828b327d4fc481125c3..5ee431b6256c6101bceb4515855b8d5b1fc2790f 100644 --- a/net/mac80211/ieee80211.c +++ b/net/mac80211/ieee80211.c @@ -268,7 +268,7 @@ static int ieee80211_open(struct net_device *dev) res = sta_info_insert(sta); if (res) { - sta_info_destroy(sta); + /* STA has been freed */ return res; } break; diff --git a/net/mac80211/ieee80211_sta.c b/net/mac80211/ieee80211_sta.c index baa68575b98a3b534c622fc341a6ab0f60fb414f..00fde111c268daf60149a865151dcde62f4ad2eb 100644 --- a/net/mac80211/ieee80211_sta.c +++ b/net/mac80211/ieee80211_sta.c @@ -1942,7 +1942,6 @@ static void ieee80211_rx_mgmt_assoc_resp(struct ieee80211_sub_if_data *sdata, if (err) { printk(KERN_DEBUG "%s: failed to insert STA entry for" " the AP (error %d)\n", dev->name, err); - sta_info_destroy(sta); rcu_read_unlock(); return; } @@ -4172,10 +4171,8 @@ struct sta_info * ieee80211_ibss_add_sta(struct net_device *dev, rate_control_rate_init(sta, local); - if (sta_info_insert(sta)) { - sta_info_destroy(sta); + if (sta_info_insert(sta)) return NULL; - } return sta; } diff --git a/net/mac80211/mesh_plink.c b/net/mac80211/mesh_plink.c index 18fe52436c47b7d6aa1437ad22f896a8c09983cd..56c54e321b387a9ffd1c8423a9048246c210caf7 100644 --- a/net/mac80211/mesh_plink.c +++ b/net/mac80211/mesh_plink.c @@ -89,6 +89,10 @@ static inline void mesh_plink_fsm_restart(struct sta_info *sta) sta->plink_retries = 0; } +/* + * NOTE: This is just an alias for sta_info_alloc(), see notes + * on it in the lifecycle management section! + */ static struct sta_info *mesh_plink_alloc(struct ieee80211_sub_if_data *sdata, u8 *hw_addr, u64 rates) { @@ -235,7 +239,6 @@ void mesh_neighbour_update(u8 *hw_addr, u64 rates, struct net_device *dev, return; } if (sta_info_insert(sta)) { - sta_info_destroy(sta); rcu_read_unlock(); return; } @@ -506,7 +509,6 @@ void mesh_rx_plink_frame(struct net_device *dev, struct ieee80211_mgmt *mgmt, return; } if (sta_info_insert(sta)) { - sta_info_destroy(sta); rcu_read_unlock(); return; } diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c index 2a5a2f067bae044815cd5c1e91152b7b516261fe..5497ca1843fea1ff6172cbd9a4c1aaebfc82ae1e 100644 --- a/net/mac80211/sta_info.c +++ b/net/mac80211/sta_info.c @@ -36,16 +36,23 @@ * (which is pretty useless) or insert it into the hash table using * sta_info_insert() which demotes the reference from ownership to a regular * RCU-protected reference; if the function is called without protection by an - * RCU critical section the reference is instantly invalidated. + * RCU critical section the reference is instantly invalidated. Note that the + * caller may not do much with the STA info before inserting it, in particular, + * it may not start any mesh peer link management or add encryption keys. + * + * When the insertion fails (sta_info_insert()) returns non-zero), the + * structure will have been freed by sta_info_insert()! * * Because there are debugfs entries for each station, and adding those * must be able to sleep, it is also possible to "pin" a station entry, * that means it can be removed from the hash table but not be freed. - * See the comment in __sta_info_unlink() for more information. + * See the comment in __sta_info_unlink() for more information, this is + * an internal capability only. * * In order to remove a STA info structure, the caller needs to first * unlink it (sta_info_unlink()) from the list and hash tables and - * then wait for an RCU synchronisation before it can be freed. Due to + * then destroy it while holding the RTNL; sta_info_destroy() will wait + * for an RCU grace period to elapse before actually freeing it. Due to * the pinning and the possibility of multiple callers trying to remove * the same STA info at the same time, sta_info_unlink() can clear the * STA info pointer it is passed to indicate that the STA info is owned @@ -127,12 +134,35 @@ struct sta_info *sta_info_get_by_idx(struct ieee80211_local *local, int idx, return NULL; } +/** + * __sta_info_free - internal STA free helper + * + * @sta: STA info to free + * + * This function must undo everything done by sta_info_alloc() + * that may happen before sta_info_insert(). + */ +static void __sta_info_free(struct ieee80211_local *local, + struct sta_info *sta) +{ + DECLARE_MAC_BUF(mbuf); + + rate_control_free_sta(sta->rate_ctrl, sta->rate_ctrl_priv); + rate_control_put(sta->rate_ctrl); + +#ifdef CONFIG_MAC80211_VERBOSE_DEBUG + printk(KERN_DEBUG "%s: Destroyed STA %s\n", + wiphy_name(local->hw.wiphy), print_mac(mbuf, sta->addr)); +#endif /* CONFIG_MAC80211_VERBOSE_DEBUG */ + + kfree(sta); +} + void sta_info_destroy(struct sta_info *sta) { struct ieee80211_local *local; struct sk_buff *skb; int i; - DECLARE_MAC_BUF(mbuf); ASSERT_RTNL(); might_sleep(); @@ -182,15 +212,7 @@ void sta_info_destroy(struct sta_info *sta) spin_unlock_bh(&sta->ampdu_mlme.ampdu_tx); } - rate_control_free_sta(sta->rate_ctrl, sta->rate_ctrl_priv); - rate_control_put(sta->rate_ctrl); - -#ifdef CONFIG_MAC80211_VERBOSE_DEBUG - printk(KERN_DEBUG "%s: Destroyed STA %s\n", - wiphy_name(local->hw.wiphy), print_mac(mbuf, sta->addr)); -#endif /* CONFIG_MAC80211_VERBOSE_DEBUG */ - - kfree(sta); + __sta_info_free(local, sta); } @@ -266,6 +288,7 @@ int sta_info_insert(struct sta_info *sta) struct ieee80211_local *local = sta->local; struct ieee80211_sub_if_data *sdata = sta->sdata; unsigned long flags; + int err = 0; DECLARE_MAC_BUF(mac); /* @@ -273,20 +296,23 @@ int sta_info_insert(struct sta_info *sta) * something inserts a STA (on one CPU) without holding the RTNL * and another CPU turns off the net device. */ - if (unlikely(!netif_running(sdata->dev))) - return -ENETDOWN; - - if (WARN_ON(compare_ether_addr(sta->addr, sdata->dev->dev_addr) == 0)) - return -EINVAL; + if (unlikely(!netif_running(sdata->dev))) { + err = -ENETDOWN; + goto out_free; + } - if (WARN_ON(is_multicast_ether_addr(sta->addr))) - return -EINVAL; + if (WARN_ON(compare_ether_addr(sta->addr, sdata->dev->dev_addr) == 0 || + is_multicast_ether_addr(sta->addr))) { + err = -EINVAL; + goto out_free; + } spin_lock_irqsave(&local->sta_lock, flags); /* check if STA exists already */ if (__sta_info_find(local, sta->addr)) { spin_unlock_irqrestore(&local->sta_lock, flags); - return -EEXIST; + err = -EEXIST; + goto out_free; } list_add(&sta->list, &local->sta_list); local->num_sta++; @@ -309,9 +335,13 @@ int sta_info_insert(struct sta_info *sta) spin_unlock_irqrestore(&local->sta_lock, flags); #ifdef CONFIG_MAC80211_DEBUGFS - /* debugfs entry adding might sleep, so schedule process + /* + * Debugfs entry adding might sleep, so schedule process * context task for adding entry for STAs that do not yet - * have one. */ + * have one. + * NOTE: due to auto-freeing semantics this may only be done + * if the insertion is successful! + */ queue_work(local->hw.workqueue, &local->sta_debugfs_add); #endif @@ -319,6 +349,10 @@ int sta_info_insert(struct sta_info *sta) mesh_accept_plinks_update(sdata); return 0; + out_free: + BUG_ON(!err); + __sta_info_free(local, sta); + return err; } static inline void __bss_tim_set(struct ieee80211_if_ap *bss, u16 aid)