[RFC] Remove VLAN interface on STA free

Michael Braun michael-dev
Wed Mar 25 09:15:11 PDT 2015


Currently, vlan_remove_dynamic is only called when the station vlan id is
changed (ap_sta_bind_vlan), but not when the station is freed. So dynamic
vlan interfaces are not removed actually except within 1x reauthentification
vlan id change, although most of the code is already there.

This patch fixes this by calling vlan_remove_dynamic in ap_free_sta.

It cannot just use sta->vlan_id for this, as this might have been changed
without calling ap_sta_bind_vlan (ap/ieee802_11.c:handle_auth fetches from
RADIUS cache for WPA-PSK), thus ref counting might not have been updated.
Additionally, ref counting might get wrong due to old_vlanid = 0 being passed
unconditionally, thus increasing the ref counter multiple times.

So tracking the currently assigned (i.e. dynamic_vlan counter increased) vlan
is done in a new variable sta->vlan_id_bound. Therefore, the old_vlan_id
argument of ap_sta_bind_vlan is no longer needed and setting the vlan for the
sta in driver happens unconditionally.

Additionally, vlan->dynamic_vlan is only incremented when it actually
is a dynamic vlan.

Signed-off-by: Michael Braun <michael-dev at fami-braun.de>

To: j at w1.fi
Cc: hostap at lists.shmoo.com
Cc: projekt-wlan at fem.tu-ilmenau.de
---
 src/ap/ieee802_11.c |  4 ++--
 src/ap/ieee802_1x.c | 10 +++-------
 src/ap/sta_info.c   | 34 ++++++++++++++++++++++------------
 src/ap/sta_info.h   |  4 ++--
 4 files changed, 29 insertions(+), 23 deletions(-)

diff --git a/src/ap/ieee802_11.c b/src/ap/ieee802_11.c
index 89911b1..3601dfe 100644
--- a/src/ap/ieee802_11.c
+++ b/src/ap/ieee802_11.c
@@ -2473,11 +2473,11 @@ static void handle_assoc_cb(struct hostapd_data *hapd,
 		 * so bind it to the selected VLAN interface now, since the
 		 * interface selection is not going to change anymore.
 		 */
-		if (ap_sta_bind_vlan(hapd, sta, 0) < 0)
+		if (ap_sta_bind_vlan(hapd, sta) < 0)
 			return;
 	} else if (sta->vlan_id) {
 		/* VLAN ID already set (e.g., by PMKSA caching), so bind STA */
-		if (ap_sta_bind_vlan(hapd, sta, 0) < 0)
+		if (ap_sta_bind_vlan(hapd, sta) < 0)
 			return;
 	}
 
diff --git a/src/ap/ieee802_1x.c b/src/ap/ieee802_1x.c
index 79dc0f9..3dcd8fc 100644
--- a/src/ap/ieee802_1x.c
+++ b/src/ap/ieee802_1x.c
@@ -1108,8 +1108,6 @@ void ieee802_1x_new_station(struct hostapd_data *hapd, struct sta_info *sta)
 
 	pmksa = wpa_auth_sta_get_pmksa(sta->wpa_sm);
 	if (pmksa) {
-		int old_vlanid;
-
 		hostapd_logger(hapd, sta->addr, HOSTAPD_MODULE_IEEE8021X,
 			       HOSTAPD_LEVEL_DEBUG,
 			       "PMK from PMKSA cache - skip IEEE 802.1X/EAP");
@@ -1123,11 +1121,10 @@ void ieee802_1x_new_station(struct hostapd_data *hapd, struct sta_info *sta)
 		sta->eapol_sm->authFail = FALSE;
 		if (sta->eapol_sm->eap)
 			eap_sm_notify_cached(sta->eapol_sm->eap);
-		old_vlanid = sta->vlan_id;
 		pmksa_cache_to_eapol_data(pmksa, sta->eapol_sm);
 		if (sta->ssid->dynamic_vlan == DYNAMIC_VLAN_DISABLED)
 			sta->vlan_id = 0;
-		ap_sta_bind_vlan(hapd, sta, old_vlanid);
+		ap_sta_bind_vlan(hapd, sta);
 	} else {
 		if (reassoc) {
 			/*
@@ -1590,7 +1587,7 @@ ieee802_1x_receive_auth(struct radius_msg *msg, struct radius_msg *req,
 	struct hostapd_data *hapd = data;
 	struct sta_info *sta;
 	u32 session_timeout = 0, termination_action, acct_interim_interval;
-	int session_timeout_set, old_vlanid = 0;
+	int session_timeout_set;
 	struct eapol_state_machine *sm;
 	int override_eapReq = 0;
 	struct radius_hdr *hdr = radius_msg_get_hdr(msg);
@@ -1661,7 +1658,6 @@ ieee802_1x_receive_auth(struct radius_msg *msg, struct radius_msg *req,
 			sta->vlan_id = 0;
 #ifndef CONFIG_NO_VLAN
 		else {
-			old_vlanid = sta->vlan_id;
 			sta->vlan_id = radius_msg_get_vlanid(msg);
 		}
 		if (sta->vlan_id > 0 &&
@@ -1681,7 +1677,7 @@ ieee802_1x_receive_auth(struct radius_msg *msg, struct radius_msg *req,
 		}
 #endif /* CONFIG_NO_VLAN */
 
-		if (ap_sta_bind_vlan(hapd, sta, old_vlanid) < 0)
+		if (ap_sta_bind_vlan(hapd, sta) < 0)
 			break;
 
 		sta->session_timeout_set = !!session_timeout_set;
diff --git a/src/ap/sta_info.c b/src/ap/sta_info.c
index 7e75e1a..40f8b8e 100644
--- a/src/ap/sta_info.c
+++ b/src/ap/sta_info.c
@@ -171,6 +171,11 @@ void ap_free_sta(struct hostapd_data *hapd, struct sta_info *sta)
 	    !(sta->flags & WLAN_STA_PREAUTH))
 		hostapd_drv_sta_remove(hapd, sta->addr);
 
+#ifndef CONFIG_NO_VLAN
+	if (sta->vlan_id_bound)
+		vlan_remove_dynamic(hapd, sta->vlan_id_bound);
+#endif
+
 	ap_sta_hash_del(hapd, sta);
 	ap_sta_list_del(hapd, sta);
 
@@ -768,20 +773,13 @@ int ap_sta_wps_cancel(struct hostapd_data *hapd,
 #endif /* CONFIG_WPS */
 
 
-int ap_sta_bind_vlan(struct hostapd_data *hapd, struct sta_info *sta,
-		     int old_vlanid)
+int ap_sta_bind_vlan(struct hostapd_data *hapd, struct sta_info *sta)
 {
 #ifndef CONFIG_NO_VLAN
 	const char *iface;
 	struct hostapd_vlan *vlan = NULL;
 	int ret;
-
-	/*
-	 * Do not proceed furthur if the vlan id remains same. We do not want
-	 * duplicate dynamic vlan entries.
-	 */
-	if (sta->vlan_id == old_vlanid)
-		return 0;
+	int old_vlanid = sta->vlan_id_bound;
 
 	iface = hapd->conf->iface;
 	if (sta->ssid->vlan[0])
@@ -805,6 +803,14 @@ int ap_sta_bind_vlan(struct hostapd_data *hapd, struct sta_info *sta,
 			iface = vlan->ifname;
 	}
 
+	/*
+	 * Do not increment ref counters if the vlan id remains same, but do
+	 * not skip hostapd_drv_set_sta_vlan as hostapd_drv_sta_remove might
+	 * have been called before.
+	 */
+	if (sta->vlan_id == old_vlanid)
+		goto skip_counting;
+
 	if (sta->vlan_id > 0 && vlan == NULL) {
 		hostapd_logger(hapd, sta->addr, HOSTAPD_MODULE_IEEE80211,
 			       HOSTAPD_LEVEL_DEBUG, "could not find VLAN for "
@@ -838,7 +844,7 @@ int ap_sta_bind_vlan(struct hostapd_data *hapd, struct sta_info *sta,
 			       HOSTAPD_LEVEL_DEBUG, "added new dynamic VLAN "
 			       "interface '%s'", iface);
 	} else if (vlan && vlan->vlan_id == sta->vlan_id) {
-		if (sta->vlan_id > 0) {
+		if (vlan->dynamic_vlan > 0) {
 			vlan->dynamic_vlan++;
 			hostapd_logger(hapd, sta->addr,
 				       HOSTAPD_MODULE_IEEE80211,
@@ -862,6 +868,10 @@ int ap_sta_bind_vlan(struct hostapd_data *hapd, struct sta_info *sta,
 		}
 	}
 
+	/* ref counters have been encreased, so mark the station */
+	sta->vlan_id_bound = sta->vlan_id;
+
+skip_counting:
 	hostapd_logger(hapd, sta->addr, HOSTAPD_MODULE_IEEE80211,
 		       HOSTAPD_LEVEL_DEBUG, "binding station to interface "
 		       "'%s'", iface);
@@ -876,10 +886,10 @@ int ap_sta_bind_vlan(struct hostapd_data *hapd, struct sta_info *sta,
 			       "entry to vlan_id=%d", sta->vlan_id);
 	}
 
-done:
 	/* During 1x reauth, if the vlan id changes, then remove the old id. */
-	if (old_vlanid > 0)
+	if (old_vlanid > 0 && old_vlanid != sta->vlan_id)
 		vlan_remove_dynamic(hapd, old_vlanid);
+done:
 
 	return ret;
 #else /* CONFIG_NO_VLAN */
diff --git a/src/ap/sta_info.h b/src/ap/sta_info.h
index 57551ab..9b6b161 100644
--- a/src/ap/sta_info.h
+++ b/src/ap/sta_info.h
@@ -121,6 +121,7 @@ struct sta_info {
 	struct hostapd_ssid *ssid_probe; /* SSID selection based on ProbeReq */
 
 	int vlan_id;
+	int vlan_id_bound; /* updated by ap_sta_bind_vlan */
 	 /* PSKs from RADIUS authentication server */
 	struct hostapd_sta_wpa_psk_short *psk;
 
@@ -218,8 +219,7 @@ void ap_sta_deauthenticate(struct hostapd_data *hapd, struct sta_info *sta,
 int ap_sta_wps_cancel(struct hostapd_data *hapd,
 		      struct sta_info *sta, void *ctx);
 #endif /* CONFIG_WPS */
-int ap_sta_bind_vlan(struct hostapd_data *hapd, struct sta_info *sta,
-		     int old_vlanid);
+int ap_sta_bind_vlan(struct hostapd_data *hapd, struct sta_info *sta);
 void ap_sta_start_sa_query(struct hostapd_data *hapd, struct sta_info *sta);
 void ap_sta_stop_sa_query(struct hostapd_data *hapd, struct sta_info *sta);
 int ap_check_sa_query_timeout(struct hostapd_data *hapd, struct sta_info *sta);
-- 
1.9.1




More information about the Hostap mailing list