[PATCH v5 21/22] VLAN: ensure destroying interfaces goes reverse as creating

[Michael Braun]

This fixes a race condition:
 1. STA removal triggers dynamic interface removal
 2. STA reconnects, finds VLAN still present but fails to set key
    (detecting the VLAN has been removed here would generate
     a NEWLINK messages after 3. but that would be pointless,
     due to 3. removing the vlan from hapd->conf->vlan, which
     is required for manual vlan interface removal)
 3. DELLINK is processed, removes the VLAN although in use

Signed-hostap: Michael Braun <michael-dev at fami-braun.de>
---
 src/ap/ap_config.h |    3 +++
 src/ap/vlan_init.c |   57 ++++++++++++++++++++++++++++++++++++----------------
 2 files changed, 43 insertions(+), 17 deletions(-)

diff --git a/src/ap/ap_config.h b/src/ap/ap_config.h
index f51b6a8..df30423 100644
--- a/src/ap/ap_config.h
+++ b/src/ap/ap_config.h
@@ -97,6 +97,9 @@ struct hostapd_vlan {
 	 * freed.
 	 */
 	int newlink_seen;
+	/* Make sure DELLINK processing does not affect newly created
+	 * interfaces. */
+	int ifidx;
 	int dynamic_vlan;
 #ifdef CONFIG_FULL_DYNAMIC_VLAN
 
diff --git a/src/ap/vlan_init.c b/src/ap/vlan_init.c
index ab6ac97..a6c21e2 100644
--- a/src/ap/vlan_init.c
+++ b/src/ap/vlan_init.c
@@ -653,14 +653,16 @@ static void vlan_newlink_vlan(const int vlan_id, char *ifname, int *clean,
 }
 
 
-static void vlan_newlink(char *ifname, struct hostapd_data *hapd)
+static void vlan_newlink(char *ifname, int ifidx, struct hostapd_data *hapd)
 {
 	struct hostapd_vlan *vlan;
 	char vlan_ifname[IFNAMSIZ+1];
 	int i, ret;
 
 	for (vlan = hapd->conf->vlan; vlan; vlan = vlan->next) {
-		if (os_strcmp(ifname, vlan->ifname) != 0 || vlan->newlink_seen)
+		if (os_strcmp(ifname, vlan->ifname) != 0 ||
+		    vlan->newlink_seen ||
+		    (vlan->ifidx > 0 && ifidx > 0 && vlan->ifidx != ifidx))
 			continue;
 
 		wpa_printf(MSG_DEBUG, "VLAN: vlan_newlink(%s)", ifname);
@@ -698,7 +700,7 @@ static void vlan_newlink(char *ifname, struct hostapd_data *hapd)
 		}
 
 		vlan->newlink_seen = 1;
-
+		vlan->ifidx = ifidx;
 		break;
 	}
 }
@@ -760,7 +762,7 @@ static void vlan_dellink_vlan(int vlan_id, char* ifname, int *clean,
 }
 
 
-static void vlan_dellink(char *ifname, struct hostapd_data *hapd)
+static void vlan_dellink(char *ifname, int ifidx, struct hostapd_data *hapd)
 {
 	struct hostapd_vlan *first, *prev, *vlan;
 	char vlan_ifname[IFNAMSIZ];
@@ -768,7 +770,8 @@ static void vlan_dellink(char *ifname, struct hostapd_data *hapd)
 
 	for (first = prev = vlan = hapd->conf->vlan; vlan;
 	     prev = vlan, vlan = vlan->next) {
-		if (os_strcmp(ifname, vlan->ifname) != 0)
+		if (os_strcmp(ifname, vlan->ifname) != 0 ||
+		    (vlan->ifidx > 0 && ifidx > 0 && vlan->ifidx != ifidx))
 			continue;
 
 		wpa_printf(MSG_DEBUG, "VLAN:%s: vlan_dellink(%s)",
@@ -820,7 +823,7 @@ vlan_read_ifnames(struct nlmsghdr *h, size_t len, int del,
 		  struct hostapd_data *hapd)
 {
 	struct ifinfomsg *ifi;
-	int attrlen, nlmsg_len, rta_len;
+	int attrlen, nlmsg_len, rta_len, ifidx;
 	struct rtattr *attr;
 
 	if (len < sizeof(*ifi))
@@ -834,6 +837,7 @@ vlan_read_ifnames(struct nlmsghdr *h, size_t len, int del,
 	if (attrlen < 0)
 		return;
 
+	ifidx = ifi->ifi_index;
 	attr = (struct rtattr *) (((char *) ifi) + nlmsg_len);
 
 	rta_len = RTA_ALIGN(sizeof(struct rtattr));
@@ -852,9 +856,9 @@ vlan_read_ifnames(struct nlmsghdr *h, size_t len, int del,
 			os_memcpy(ifname, ((char *) attr) + rta_len, n);
 
 			if (del)
-				vlan_dellink(ifname, hapd);
+				vlan_dellink(ifname, ifidx, hapd);
 			else
-				vlan_newlink(ifname, hapd);
+				vlan_newlink(ifname, ifidx, hapd);
 		}
 
 		attr = RTA_NEXT(attr, attrlen);
@@ -1030,21 +1034,29 @@ static void vlan_dynamic_remove(struct hostapd_data *hapd,
 				struct hostapd_vlan *vlan)
 {
 	struct hostapd_vlan *next;
+	char buf[IFNAMSIZ+1];
+	struct vlan_description vlan_id = VLAN_NULL;
 
 	while (vlan) {
 		next = vlan->next;
 
-		if (vlan_untagged(&vlan->vlan_id) != VLAN_ID_WILDCARD &&
-		    hostapd_vlan_if_remove(hapd, vlan->ifname)) {
-			wpa_printf(MSG_ERROR, "VLAN: Could not remove VLAN "
-				   "iface: %s: %s",
-				   vlan->ifname, strerror(errno));
-		}
+		vlan_alloc_copy(&vlan_id, &vlan->vlan_id);
+		os_strlcpy(buf, vlan->ifname, sizeof(buf));
+
 #ifdef CONFIG_FULL_DYNAMIC_VLAN
 		if (vlan->clean)
-			vlan_dellink(vlan->ifname, hapd);
+			vlan_dellink(buf, vlan->ifidx, hapd);
 #endif /* CONFIG_FULL_DYNAMIC_VLAN */
 
+		if (vlan_untagged(&vlan_id) != VLAN_ID_WILDCARD &&
+		    hostapd_vlan_if_remove(hapd, buf)) {
+			wpa_printf(MSG_ERROR, "VLAN: Could not remove VLAN "
+				   "iface: %s: %s",
+				   buf, strerror(errno));
+		}
+
+		vlan_free(&vlan_id);
+
 		vlan = next;
 	}
 }
@@ -1161,6 +1173,10 @@ struct hostapd_vlan * vlan_add_dynamic(struct hostapd_data *hapd,
 		os_free(n);
 		return NULL;
 	}
+#ifdef CONFIG_FULL_DYNAMIC_VLAN
+	// if id_add returned the ifidx, it could be reused here
+	n->ifidx = if_nametoindex(n->ifname);
+#endif
 
 	/* insert into list */
 	if (!prev) {
@@ -1183,6 +1199,7 @@ int vlan_remove_dynamic(struct hostapd_data *hapd,
                         struct vlan_description vlan_id)
 {
 	struct hostapd_vlan *vlan;
+	char buf[IFNAMSIZ+1];
 
 	if (vlan_untagged(&vlan_id) == VLAN_ID_WILDCARD)
 		return 1;
@@ -1203,8 +1220,14 @@ int vlan_remove_dynamic(struct hostapd_data *hapd,
 	if (vlan == NULL)
 		return 1;
 
-	if (vlan->dynamic_vlan == 0)
-		hostapd_vlan_if_remove(hapd, vlan->ifname);
+	if (vlan->dynamic_vlan == 0) {
+		os_strlcpy(buf, vlan->ifname, sizeof(buf));
+#ifdef CONFIG_FULL_DYNAMIC_VLAN
+		vlan_dellink(buf, vlan->ifidx, hapd);
+#endif
+		vlan = NULL; /* freed by vlan_dellink */
+		hostapd_vlan_if_remove(hapd, buf);
+	}
 
 	return 0;
 }