Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 73651ee6 authored by Johannes Berg's avatar Johannes Berg Committed by John W. Linville
Browse files

mac80211: split sta_info_add 



sta_info_add() has two functions: allocating a station info
structure and inserting it into the hash table/list. Splitting
these two functions allows allocating with GFP_KERNEL in many
places instead of GFP_ATOMIC which is now required by the RCU
protection. Additionally, in many places RCU protection is now
no longer needed at all because between sta_info_alloc() and
sta_info_insert() the caller owns the structure.

This fixes a few race conditions with setting initial flags
and similar, but not all (see comments in ieee80211_sta.c and
cfg.c). More documentation on the existing races will be in
a follow-up patch.

Signed-off-by: default avatarJohannes Berg <johannes@sipsolutions.net>
Signed-off-by: default avatarJohn W. Linville <linville@tuxdriver.com>
parent d0709a65

net/mac80211/cfg.c

+33 −9
Original line number Diff line number Diff line
@@ -571,6 +571,12 @@ static void sta_apply_parameters(struct ieee80211_local *local, 
	struct ieee80211_supported_band *sband;

	struct ieee80211_sub_if_data *sdata = sta->sdata;



	/*

	 * FIXME: updating the flags is racy when this function is

	 *	  called from ieee80211_change_station(), this will

	 *	  be resolved in a future patch.

	 */



	if (params->station_flags & STATION_FLAG_CHANGED) {

		sta->flags &= ~WLAN_STA_AUTHORIZED;

		if (params->station_flags & STATION_FLAG_AUTHORIZED)
@@ -585,6 +591,13 @@ static void sta_apply_parameters(struct ieee80211_local *local, 
			sta->flags |= WLAN_STA_WME;

	}



	/*

	 * FIXME: updating the following information is racy when this

	 *	  function is called from ieee80211_change_station().

	 *	  However, all this information should be static so

	 *	  maybe we should just reject attemps to change it.

	 */



	if (params->aid) {

		sta->aid = params->aid;

		if (sta->aid > IEEE80211_MAX_AID)
@@ -626,6 +639,7 @@ static int ieee80211_add_station(struct wiphy *wiphy, struct net_device *dev, 
	struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);

	struct sta_info *sta;

	struct ieee80211_sub_if_data *sdata;

	int err;



	/* Prevent a race with changing the rate control algorithm */

	if (!netif_running(dev))
@@ -641,16 +655,11 @@ static int ieee80211_add_station(struct wiphy *wiphy, struct net_device *dev, 
		sdata = IEEE80211_DEV_TO_SUB_IF(dev);



	if (ieee80211_vif_is_mesh(&sdata->vif))

		sta = mesh_plink_add(mac, DEFAULT_RATES, sdata);

		sta = mesh_plink_alloc(sdata, mac, DEFAULT_RATES, GFP_KERNEL);

	else

		sta = sta_info_add(sdata, mac);



	if (IS_ERR(sta))

		return PTR_ERR(sta);



	if (sdata->vif.type == IEEE80211_IF_TYPE_VLAN ||

	    sdata->vif.type == IEEE80211_IF_TYPE_AP)

		ieee80211_send_layer2_update(sta);

		sta = sta_info_alloc(sdata, mac, GFP_KERNEL);

	if (!sta)

		return -ENOMEM;



	sta->flags = WLAN_STA_AUTH | WLAN_STA_ASSOC;
@@ -658,6 +667,21 @@ static int ieee80211_add_station(struct wiphy *wiphy, struct net_device *dev, 


	rate_control_rate_init(sta, local);



	rcu_read_lock();



	err = sta_info_insert(sta);

	if (err) {

		sta_info_destroy(sta);

		rcu_read_unlock();

		return err;

	}



	if (sdata->vif.type == IEEE80211_IF_TYPE_VLAN ||

	    sdata->vif.type == IEEE80211_IF_TYPE_AP)

		ieee80211_send_layer2_update(sta);



	rcu_read_unlock();



	return 0;

}

net/mac80211/ieee80211.c

+11 −7
Original line number Diff line number Diff line
@@ -899,6 +899,7 @@ int ieee80211_if_update_wds(struct net_device *dev, u8 *remote_addr) 
	struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);

	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);

	struct sta_info *sta;

	int err;

	DECLARE_MAC_BUF(mac);



	might_sleep();
@@ -906,16 +907,19 @@ int ieee80211_if_update_wds(struct net_device *dev, u8 *remote_addr) 
	if (compare_ether_addr(remote_addr, sdata->u.wds.remote_addr) == 0)

		return 0;



	rcu_read_lock();



	/* Create STA entry for the new peer */

	sta = sta_info_add(sdata, remote_addr);

	if (IS_ERR(sta)) {

		rcu_read_unlock();

		return PTR_ERR(sta);

	}

	sta = sta_info_alloc(sdata, remote_addr, GFP_KERNEL);

	if (!sta)

		return -ENOMEM;



	sta->flags |= WLAN_STA_AUTHORIZED;

	err = sta_info_insert(sta);

	if (err) {

		sta_info_destroy(sta);

		return err;

	}



	rcu_read_lock();



	/* Remove STA entry for the old peer */

	sta = sta_info_get(local, sdata->u.wds.remote_addr);

net/mac80211/ieee80211_sta.c

+33 −11
Original line number Diff line number Diff line
@@ -1454,7 +1454,7 @@ void sta_addba_resp_timer_expired(unsigned long data) 
{

	/* not an elegant detour, but there is no choice as the timer passes

	 * only one argument, and both sta_info and TID are needed, so init

	 * flow in sta_info_add gives the TID as data, while the timer_to_id

	 * flow in sta_info_create gives the TID as data, while the timer_to_id

	 * array gives the sta through container_of */

	u16 tid = *(int *)data;

	struct sta_info *temp_sta = container_of((void *)data,
@@ -1505,7 +1505,7 @@ void sta_rx_agg_session_timer_expired(unsigned long data) 
{

	/* not an elegant detour, but there is no choice as the timer passes

	 * only one argument, and verious sta_info are needed here, so init

	 * flow in sta_info_add gives the TID as data, while the timer_to_id

	 * flow in sta_info_create gives the TID as data, while the timer_to_id

	 * array gives the sta through container_of */

	u8 *ptid = (u8 *)data;

	u8 *timer_to_id = ptid - *ptid;
@@ -1829,11 +1829,12 @@ static void ieee80211_rx_mgmt_assoc_resp(struct ieee80211_sub_if_data *sdata, 
	sta = sta_info_get(local, ifsta->bssid);

	if (!sta) {

		struct ieee80211_sta_bss *bss;

		int err;



		sta = sta_info_add(sdata, ifsta->bssid);

		if (IS_ERR(sta)) {

			printk(KERN_DEBUG "%s: failed to add STA entry for the"

			       " AP (error %ld)\n", dev->name, PTR_ERR(sta));

		sta = sta_info_alloc(sdata, ifsta->bssid, GFP_ATOMIC);

		if (!sta) {

			printk(KERN_DEBUG "%s: failed to alloc STA entry for"

			       " the AP\n", dev->name);

			rcu_read_unlock();

			return;

		}
@@ -1846,8 +1847,27 @@ static void ieee80211_rx_mgmt_assoc_resp(struct ieee80211_sub_if_data *sdata, 
			sta->last_noise = bss->noise;

			ieee80211_rx_bss_put(dev, bss);

		}



		err = sta_info_insert(sta);

		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;

		}

	}



	/*

	 * FIXME: Do we really need to update the sta_info's information here?

	 *	  We already know about the AP (we found it in our list) so it

	 *	  should already be filled with the right info, no?

	 *	  As is stands, all this is racy because typically we assume

	 *	  the information that is filled in here (except flags) doesn't

	 *	  change while a STA structure is alive. As such, it should move

	 *	  to between the sta_info_alloc() and sta_info_insert() above.

	 */



	sta->flags |= WLAN_STA_AUTH | WLAN_STA_ASSOC | WLAN_STA_ASSOC_AP |

		      WLAN_STA_AUTHORIZED;
@@ -2588,10 +2608,8 @@ static void ieee80211_rx_bss_info(struct net_device *dev, 
				       "local TSF - IBSS merge with BSSID %s\n",

				       dev->name, print_mac(mac, mgmt->bssid));

			ieee80211_sta_join_ibss(dev, &sdata->u.sta, bss);

			rcu_read_lock();

			ieee80211_ibss_add_sta(dev, NULL,

					       mgmt->bssid, mgmt->sa);

			rcu_read_unlock();

		}

	}
@@ -4023,7 +4041,6 @@ int ieee80211_sta_set_extra_ie(struct net_device *dev, char *ie, size_t len) 
}





/* must be called under RCU read lock */

struct sta_info * ieee80211_ibss_add_sta(struct net_device *dev,

					 struct sk_buff *skb, u8 *bssid,

					 u8 *addr)
@@ -4046,8 +4063,8 @@ struct sta_info * ieee80211_ibss_add_sta(struct net_device *dev, 
	printk(KERN_DEBUG "%s: Adding new IBSS station %s (dev=%s)\n",

	       wiphy_name(local->hw.wiphy), print_mac(mac, addr), dev->name);



	sta = sta_info_add(sdata, addr);

	if (IS_ERR(sta))

	sta = sta_info_alloc(sdata, addr, GFP_ATOMIC);

	if (!sta)

		return NULL;



	sta->flags |= WLAN_STA_AUTHORIZED;
@@ -4057,6 +4074,11 @@ 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);

		return NULL;

	}



	return sta;

}

net/mac80211/mesh.h

+2 −2
Original line number Diff line number Diff line
@@ -232,8 +232,8 @@ void mesh_neighbour_update(u8 *hw_addr, u64 rates, struct net_device *dev, 
bool mesh_peer_accepts_plinks(struct ieee802_11_elems *ie,

			      struct net_device *dev);

void mesh_accept_plinks_update(struct ieee80211_sub_if_data *sdata);

struct sta_info *mesh_plink_add(u8 *hw_addr, u64 rates,

				struct ieee80211_sub_if_data *sdata);

struct sta_info *mesh_plink_alloc(struct ieee80211_sub_if_data *sdata,

				  u8 *hw_addr, u64 rates, gfp_t gfp);

void mesh_plink_broken(struct sta_info *sta);

void mesh_plink_deactivate(struct sta_info *sta);

int mesh_plink_open(struct sta_info *sta);

net/mac80211/mesh_plink.c

+25 −18
Original line number Diff line number Diff line
@@ -89,44 +89,41 @@ static inline void mesh_plink_fsm_restart(struct sta_info *sta) 
}



/**

 * mesh_plink_add - allocate and add a new mesh peer link

 * mesh_plink_alloc - allocate a new mesh peer link

 *

 * @sdata: local mesh interface

 * @hw_addr: hardware address (ETH_ALEN length)

 * @rates: rates the mesh peer supports

 * @dev: local mesh interface

 *

 * The initial state of the new plink is set to LISTEN

 *

 * Returns: non-NULL on success, ERR_PTR() on error.

 * Returns: NULL on error.

 */

struct sta_info *mesh_plink_add(u8 *hw_addr, u64 rates,

				struct ieee80211_sub_if_data *sdata)

struct sta_info *mesh_plink_alloc(struct ieee80211_sub_if_data *sdata,

				  u8 *hw_addr, u64 rates, gfp_t gfp)

{

	struct ieee80211_local *local = sdata->local;

	struct sta_info *sta;



	if (compare_ether_addr(hw_addr, sdata->dev->dev_addr) == 0)

		/* never add ourselves as neighbours */

		return ERR_PTR(-EINVAL);

		return NULL;



	if (is_multicast_ether_addr(hw_addr))

		return ERR_PTR(-EINVAL);

		return NULL;



	if (local->num_sta >= MESH_MAX_PLINKS)

		return ERR_PTR(-ENOSPC);

		return NULL;



	sta = sta_info_add(sdata, hw_addr);

	if (IS_ERR(sta))

		return sta;

	sta = sta_info_alloc(sdata, hw_addr, gfp);

	if (!sta)

		return NULL;



	sta->plink_state = LISTEN;

	spin_lock_init(&sta->plink_lock);

	init_timer(&sta->plink_timer);

	sta->flags |= WLAN_STA_AUTHORIZED;

	sta->supp_rates[local->hw.conf.channel->band] = rates;

	rate_control_rate_init(sta, local);



	mesh_accept_plinks_update(sdata);



	return sta;

}
@@ -252,8 +249,13 @@ void mesh_neighbour_update(u8 *hw_addr, u64 rates, struct net_device *dev, 


	sta = sta_info_get(local, hw_addr);

	if (!sta) {

		sta = mesh_plink_add(hw_addr, rates, sdata);

		if (IS_ERR(sta)) {

		sta = mesh_plink_alloc(sdata, hw_addr, rates, GFP_ATOMIC);

		if (!sta) {

			rcu_read_unlock();

			return;

		}

		if (sta_info_insert(sta)) {

			sta_info_destroy(sta);

			rcu_read_unlock();

			return;

		}
@@ -516,12 +518,17 @@ void mesh_rx_plink_frame(struct net_device *dev, struct ieee80211_mgmt *mgmt, 
		}



		rates = ieee80211_sta_get_rates(local, &elems, rx_status->band);

		sta = mesh_plink_add(mgmt->sa, rates, sdata);

		if (IS_ERR(sta)) {

		sta = mesh_plink_alloc(sdata, mgmt->sa, rates, GFP_ATOMIC);

		if (!sta) {

			mpl_dbg("Mesh plink error: plink table full\n");

			rcu_read_unlock();

			return;

		}

		if (sta_info_insert(sta)) {

			sta_info_destroy(sta);

			rcu_read_unlock();

			return;

		}

		event = OPN_ACPT;

		spin_lock_bh(&sta->plink_lock);

	} else {