Fix removal of tagged interface and bridge when multiple BSS share them

Currently, if multiple bss share are bridge and tagged vlan interface,
only the first instance of struct hostapd_vlan for this vlanid will have
the DVLAN_CLEAN_VLAN flag added. Thus, when this instance is removed,
the tagged vlan interface will be removed from bridge, thought other bss
might still need it. Similarily, the bridge will be left over, as the
does not have zero ports when the first instance of a struct
hostapd_vlan is freed.

This patch fixes this by having a global (per process) reference counter
for dynamic tagged vlan and dynamically created bridge interfaces, so
they are only removed after all local users are freed. (struct
hapd_interfaces *)->vlan_priv is used to hold src/ap/vlan_init.c global
per-process data like drv_priv does; right now this is only used for the
interface reference counting, but could get extended when needed. Then
possibly some vlan_global_init / vlan_global_deinit should be added, but
this is not required right now.

Additionally, vlan->configured is checked to avoid reference counter
decreasing before vlan_newlink increased them.

In order to avoid race conditions, vlan_dellink is called explicitly
after hostapd_vlan_if_remove. Otherwise there would be a short timeframe
between hostapd_vlan_if_remove and vlan_dellink during which the struct
hostapd_vlan still exists, so ap_sta_bind_vlan would try to attach
stations to it.

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

src/ap/ap_config.h

@@ -117,9 +117,6 @@ struct hostapd_vlan {
 	int dynamic_vlan;
 #ifdef CONFIG_FULL_DYNAMIC_VLAN
 
-#define DVLAN_CLEAN_BR 	0x1
-#define DVLAN_CLEAN_VLAN	0x2
-#define DVLAN_CLEAN_VLAN_PORT	0x4
 #define DVLAN_CLEAN_WLAN_PORT	0x8
 	int clean;
 #endif /* CONFIG_FULL_DYNAMIC_VLAN */

src/ap/hostapd.h

@@ -49,6 +49,9 @@ struct hapd_interfaces {
 	struct hostapd_iface **iface;
 
 	size_t terminate_on_error;
+#ifndef CONFIG_NO_VLAN
+	struct dynamic_iface *vlan_priv;
+#endif /* CONFIG_NO_VLAN */
 };
 
 enum hostapd_chan_status {

src/ap/vlan_init.c

@@ -35,6 +35,90 @@ struct full_dynamic_vlan {
 	int s; /* socket on which to listen for new/removed interfaces. */
 };
 
+#define DVLAN_CLEAN_BR         0x1
+#define DVLAN_CLEAN_VLAN       0x2
+#define DVLAN_CLEAN_VLAN_PORT  0x4
+
+struct dynamic_iface {
+	char ifname[IFNAMSIZ + 1];
+	int usage;
+	int clean;
+	struct dynamic_iface *next;
+};
+
+
+/* Increment ref counter for ifname and add clean flag.
+ * If not in list, add it only if some flags are given.
+ */
+static void dyn_iface_get(struct hostapd_data *hapd, const char *ifname,
+			  int clean)
+{
+	struct dynamic_iface *next, **dynamic_ifaces;
+	struct hapd_interfaces *interfaces;
+
+	interfaces = hapd->iface->interfaces;
+	dynamic_ifaces = &interfaces->vlan_priv;
+
+	for (next = *dynamic_ifaces; next; next = next->next) {
+		if (os_strcmp(ifname, next->ifname) == 0)
+			break;
+	}
+
+	if (next) {
+		next->usage++;
+		next->clean |= clean;
+		return;
+	}
+
+	if (!clean)
+		return;
+
+	next = os_zalloc(sizeof(*next));
+	if (!next)
+		return;
+	os_strlcpy(next->ifname, ifname, sizeof(next->ifname));
+	next->usage = 1;
+	next->clean = clean;
+	next->next = *dynamic_ifaces;
+	*dynamic_ifaces = next;
+}
+
+
+/* Decrement reference counter for given ifname.
+ * Return clean flag iff reference counter was decreased to zero, else zero
+ */
+static int dyn_iface_put(struct hostapd_data *hapd, const char *ifname)
+{
+	struct dynamic_iface *next, *prev = NULL, **dynamic_ifaces;
+	struct hapd_interfaces *interfaces;
+	int clean;
+
+	interfaces = hapd->iface->interfaces;
+	dynamic_ifaces = &interfaces->vlan_priv;
+
+	for (next = *dynamic_ifaces; next; next = next->next) {
+		if (os_strcmp(ifname, next->ifname) == 0)
+			break;
+		prev = next;
+	}
+
+	if (!next)
+		return 0;
+
+	next->usage--;
+	if (next->usage)
+		return 0;
+
+	if (prev)
+		prev->next = next->next;
+	else
+		*dynamic_ifaces = next->next;
+	clean = next->clean;
+	os_free(next);
+
+	return clean;
+}
+
 
 static int ifconfig_helper(const char *if_name, int up)
 {
@@ -482,6 +566,7 @@ static void vlan_newlink(char *ifname, struct hostapd_data *hapd)
 	struct hostapd_vlan *vlan = hapd->conf->vlan;
 	char *tagged_interface = hapd->conf->ssid.vlan_tagged_interface;
 	int vlan_naming = hapd->conf->ssid.vlan_naming;
+	int clean;
 
 	wpa_printf(MSG_DEBUG, "VLAN: vlan_newlink(%s)", ifname);
 
@@ -502,8 +587,8 @@ static void vlan_newlink(char *ifname, struct hostapd_data *hapd)
 				            "brvlan%d", vlan->vlan_id);
 			}
 
-			if (!br_addbr(br_name))
+			dyn_iface_get(hapd, br_name,
-				vlan->clean |= DVLAN_CLEAN_BR;
+				      br_addbr(br_name) ? 0 : DVLAN_CLEAN_BR);
 
 			ifconfig_up(br_name);
 
@@ -519,13 +604,16 @@ static void vlan_newlink(char *ifname, struct hostapd_data *hapd)
 						    sizeof(vlan_ifname),
 						    "vlan%d", vlan->vlan_id);
 
+				clean = 0;
 				ifconfig_up(tagged_interface);
 				if (!vlan_add(tagged_interface, vlan->vlan_id,
 					      vlan_ifname))
-					vlan->clean |= DVLAN_CLEAN_VLAN;
+					clean |= DVLAN_CLEAN_VLAN;
 
 				if (!br_addif(br_name, vlan_ifname))
-					vlan->clean |= DVLAN_CLEAN_VLAN_PORT;
+					clean |= DVLAN_CLEAN_VLAN_PORT;
+
+				dyn_iface_get(hapd, vlan_ifname, clean);
 
 				ifconfig_up(vlan_ifname);
 			}
@@ -549,13 +637,15 @@ static void vlan_dellink(char *ifname, struct hostapd_data *hapd)
 	struct hostapd_vlan *first, *prev, *vlan = hapd->conf->vlan;
 	char *tagged_interface = hapd->conf->ssid.vlan_tagged_interface;
 	int vlan_naming = hapd->conf->ssid.vlan_naming;
+	int clean;
 
 	wpa_printf(MSG_DEBUG, "VLAN: vlan_dellink(%s)", ifname);
 
 	first = prev = vlan;
 
 	while (vlan) {
-		if (os_strcmp(ifname, vlan->ifname) == 0) {
+		if (os_strcmp(ifname, vlan->ifname) == 0 &&
+		    vlan->configured) {
 			if (hapd->conf->vlan_bridge[0]) {
 				os_snprintf(br_name, sizeof(br_name), "%s%d",
 					    hapd->conf->vlan_bridge,
@@ -583,20 +673,27 @@ static void vlan_dellink(char *ifname, struct hostapd_data *hapd)
 					os_snprintf(vlan_ifname,
 						    sizeof(vlan_ifname),
 						    "vlan%d", vlan->vlan_id);
-				if (vlan->clean & DVLAN_CLEAN_VLAN_PORT)
-					br_delif(br_name, vlan_ifname);
-				ifconfig_down(vlan_ifname);
 
-				if (vlan->clean & DVLAN_CLEAN_VLAN)
+				clean = dyn_iface_put(hapd, vlan_ifname);
+
+				if (clean & DVLAN_CLEAN_VLAN_PORT)
+					br_delif(br_name, vlan_ifname);
+
+				if (clean & DVLAN_CLEAN_VLAN) {
+					ifconfig_down(vlan_ifname);
 					vlan_rem(vlan_ifname);
 				}
+			}
 
-			if ((vlan->clean & DVLAN_CLEAN_BR) &&
+			clean = dyn_iface_put(hapd, br_name);
+			if ((clean & DVLAN_CLEAN_BR) &&
 			    br_getnumports(br_name) == 0) {
 				ifconfig_down(br_name);
 				br_delbr(br_name);
 			}
+		}
 
+		if (os_strcmp(ifname, vlan->ifname) == 0) {
 			if (vlan == first) {
 				hapd->conf->vlan = vlan->next;
 			} else {
@@ -975,8 +1072,12 @@ int vlan_remove_dynamic(struct hostapd_data *hapd, int vlan_id)
 	if (vlan == NULL)
 		return 1;
 
-	if (vlan->dynamic_vlan == 0)
+	if (vlan->dynamic_vlan == 0) {
 		hostapd_vlan_if_remove(hapd, vlan->ifname);
+#ifdef CONFIG_FULL_DYNAMIC_VLAN
+		vlan_dellink(vlan->ifname, hapd);
+#endif /* CONFIG_FULL_DYNAMIC_VLAN */
+	}
 
 	return 0;
 }