Remove VLAN interface on STA free

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 reference counting might
not have been updated. Additionally, reference counting might get wrong
due to old_vlanid = 0 being passed unconditionally, thus increasing the
reference 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@fami-braun.de>
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 @@
 		 * 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 d7feda1..f945efa 100644
--- a/src/ap/ieee802_1x.c
+++ b/src/ap/ieee802_1x.c
@@ -1108,8 +1108,6 @@
 
 	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,8 @@
 		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 +1585,7 @@
 	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, vlan_id = 0;
+	int session_timeout_set, vlan_id = 0;
 	struct eapol_state_machine *sm;
 	int override_eapReq = 0;
 	struct radius_hdr *hdr = radius_msg_get_hdr(msg);
@@ -1687,10 +1682,9 @@
 		}
 #endif /* CONFIG_NO_VLAN */
 
-		old_vlanid = sta->vlan_id;
 		sta->vlan_id = vlan_id;
 		if ((sta->flags & WLAN_STA_ASSOC) &&
-		    ap_sta_bind_vlan(hapd, sta, old_vlanid) < 0)
+		    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..1576db9 100644
--- a/src/ap/sta_info.c
+++ b/src/ap/sta_info.c
@@ -171,6 +171,19 @@
 	    !(sta->flags & WLAN_STA_PREAUTH))
 		hostapd_drv_sta_remove(hapd, sta->addr);
 
+#ifndef CONFIG_NO_VLAN
+	if (sta->vlan_id_bound) {
+		/*
+		 * Need to remove the STA entry before potentially removing the
+		 * VLAN.
+		 */
+		if (hapd->iface->driver_ap_teardown &&
+		    !(sta->flags & WLAN_STA_PREAUTH))
+			hostapd_drv_sta_remove(hapd, sta->addr);
+		vlan_remove_dynamic(hapd, sta->vlan_id_bound);
+	}
+#endif /* CONFIG_NO_VLAN */
+
 	ap_sta_hash_del(hapd, sta);
 	ap_sta_list_del(hapd, sta);
 
@@ -768,20 +781,13 @@
 #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 +811,14 @@
 			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 +852,7 @@
 			       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 +876,10 @@
 		}
 	}
 
+	/* ref counters have been increased, 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 +894,10 @@
 			       "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 d8d59cb..d192c71 100644
--- a/src/ap/sta_info.h
+++ b/src/ap/sta_info.h
@@ -121,6 +121,7 @@
 	struct hostapd_ssid *ssid_probe; /* SSID selection based on ProbeReq */
 
 	int vlan_id; /* 0: none, >0: VID */
+	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 @@
 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);