Patchwork [PATCHv2,03/21] bridge: track inter-BSS usage

login
register
mail settings
Submitter michael-dev@fami-braun.de
Date May 17, 2013, 7:49 a.m.
Message ID <93c45d9fbbb7e4a3571655c7451553be5254c3cf.1370512966.git.michael-dev@fami-braun.de>
Download mbox | patch
Permalink /patch/249363/
State Accepted
Commit 4345fe963e7bc6461b7547766d47fd4722d72130
Headers show

Comments

michael-dev@fami-braun.de - May 17, 2013, 7:49 a.m.
Currently, struct hostapd_vlan is a per-BSS data structure which
also contains informations about whether to remove the bridge
or clear wlan / tagged-vlan interface from the bridge.

In a multi-interface multi-BSS setup, this can lead to the following
race condition:
 1. wlan0 creates VLAN A, sets DVLAN_CLEAN_BR and DVLAN_CLEAN_VLAN_PORT
 2. wlan1 creates VLAN A, does not set DVLAN_CLEAN_BR and DVLAN_CLEAN_VLAN_PORT
    as already there
 3. wlan0 removes VLAN A, removes tagged-interface from the bridge
    but not the bridge.
    Now wlan1 VLAN A is unusable due to the missing uplink.
 4. wlan1 removes VLAN A, does not cleanup

Solution: This requires an inter-BSS inter-interface data structure
          to track the bridge / bridge port usage within hostapd.
          This data structure could also be used to track any other
          device-has-been-created-by-hostapd information or when
          retarding interface freeing.

Signed-hostap: Michael Braun <michael-dev@fami-braun.de>
Jouni Malinen - June 25, 2013, 8:57 a.m.
On Fri, May 17, 2013 at 09:49:19AM +0200, Michael Braun wrote:
> Currently, struct hostapd_vlan is a per-BSS data structure which
> also contains informations about whether to remove the bridge
> or clear wlan / tagged-vlan interface from the bridge.
> 
> In a multi-interface multi-BSS setup, this can lead to the following
> race condition:
>  1. wlan0 creates VLAN A, sets DVLAN_CLEAN_BR and DVLAN_CLEAN_VLAN_PORT
>  2. wlan1 creates VLAN A, does not set DVLAN_CLEAN_BR and DVLAN_CLEAN_VLAN_PORT
>     as already there
>  3. wlan0 removes VLAN A, removes tagged-interface from the bridge
>     but not the bridge.
>     Now wlan1 VLAN A is unusable due to the missing uplink.
>  4. wlan1 removes VLAN A, does not cleanup
> 
> Solution: This requires an inter-BSS inter-interface data structure
>           to track the bridge / bridge port usage within hostapd.
>           This data structure could also be used to track any other
>           device-has-been-created-by-hostapd information or when
>           retarding interface freeing.

Thanks, applied.

Patch

diff --git a/src/ap/hostapd.h b/src/ap/hostapd.h
index 55f6dd8..75d9c66 100644
--- a/src/ap/hostapd.h
+++ b/src/ap/hostapd.h
@@ -25,6 +25,7 @@  enum wps_event;
 union wps_event_data;
 
 struct hostapd_iface;
+struct hostapd_dynamic_iface;
 
 struct hapd_interfaces {
 	int (*reload_config)(struct hostapd_iface *iface);
@@ -37,11 +38,13 @@  struct hapd_interfaces {
 	int (*driver_init)(struct hostapd_iface *iface);
 
 	size_t count;
+	size_t count_dynamic;
 	int global_ctrl_sock;
 	char *global_iface_path;
 	char *global_iface_name;
 	gid_t ctrl_iface_group;
 	struct hostapd_iface **iface;
+	struct hostapd_dynamic_iface **dynamic_iface;
 };
 
 
@@ -275,6 +278,16 @@  struct hostapd_iface {
 	void (*scan_cb)(struct hostapd_iface *iface);
 };
 
+/**
+ * struct hostapd_dynamic_iface - hostapd per dynamically allocated
+ * or added interface data structure
+ */
+struct hostapd_dynamic_iface {
+	char parent[IFNAMSIZ + 1];
+	char iface[IFNAMSIZ + 1];
+	unsigned int usage;
+};
+
 /* hostapd.c */
 int hostapd_for_each_interface(struct hapd_interfaces *interfaces,
 			       int (*cb)(struct hostapd_iface *iface,
diff --git a/src/ap/vlan_init.c b/src/ap/vlan_init.c
index 3cdd1a6..dd234a0 100644
--- a/src/ap/vlan_init.c
+++ b/src/ap/vlan_init.c
@@ -479,6 +479,120 @@  static int vlan_set_name_type(unsigned int name_type)
 
 #endif /* CONFIG_VLAN_NETLINK */
 
+/**
+ * Increase the usage counter for given parent/ifname combination.
+ * If create is set, then this iface is added to the global list.
+ * Returns
+ * 	-1 on error
+ * 	0 if iface is not in list
+ * 	1 if iface is in list (was there or has been added)
+ */
+static int hapd_get_dynamic_iface(char *parent, char *ifname, int create,
+                                  struct hostapd_data *hapd)
+{
+	int i;
+	struct hostapd_dynamic_iface *j = NULL, **tmp;
+	struct hapd_interfaces *hapd_global = hapd->iface->interfaces;
+
+	if (!parent)
+		parent = "";
+	
+	for (i = 0; i < hapd_global->count_dynamic; i++) {
+		j = hapd_global->dynamic_iface[i];
+		if (os_strncmp(j->iface, ifname, sizeof(j->iface)) == 0 &&
+		    os_strncmp(j->parent, parent, sizeof(j->parent)) == 0)
+			break;
+	}
+	if (i < hapd_global->count_dynamic) {
+		j->usage++;
+		return 1;
+	}
+
+	/* new entry required */
+	if (!create)
+		return 0;
+
+	j = os_zalloc(sizeof(*j));
+	if (!j)
+		return -1;
+	os_strncpy(j->iface, ifname, sizeof(j->iface));
+	os_strncpy(j->parent, parent, sizeof(j->parent));
+
+	tmp = os_realloc_array(hapd_global->dynamic_iface, i + 1,
+	                       sizeof(*hapd_global->dynamic_iface));
+	if (!tmp) {
+		wpa_printf(MSG_ERROR, "VLAN: failed to allocate memory in %s",
+		                      __FUNCTION__);
+		return -1;
+	}
+	hapd_global->count_dynamic++;
+	hapd_global->dynamic_iface = tmp;
+	hapd_global->dynamic_iface[i] = j;
+
+	return 1;
+}
+
+/**
+ * Decrease the usage counter for given ifname.
+ * Returns
+ *     -1 on error or if iface was not found
+ *     0 if iface was found and is still present
+ *     1 if iface was removed from global list
+ */
+static int hapd_put_dynamic_iface(char *parent, char *ifname,
+                                  struct hostapd_data *hapd)
+{
+	int i;
+	struct hostapd_dynamic_iface *j = NULL, **tmp;
+	struct hapd_interfaces *hapd_glob = hapd->iface->interfaces;
+
+	if (!parent)
+		parent = "";
+	
+	for (i = 0; i < hapd_glob->count_dynamic; i++) {
+		j = hapd_glob->dynamic_iface[i];
+		if (os_strncmp(j->iface, ifname, sizeof(j->iface)) == 0 &&
+		    os_strncmp(j->parent, parent, sizeof(j->parent)) == 0)
+			break;
+	}
+
+	if (i == hapd_glob->count_dynamic) {
+               /* interface not in global list
+                * Could happen if alloc in _get_ failed.
+                */
+               return -1;
+	}
+
+	if (j->usage > 0) {
+		j->usage--;
+		return 0;
+	}
+
+	os_free(j);
+	for (; i < hapd_glob->count_dynamic - 1; i++) {
+		hapd_glob->dynamic_iface[i] = hapd_glob->dynamic_iface[i + 1];
+	}
+	hapd_glob->dynamic_iface[hapd_glob->count_dynamic - 1] = NULL;
+	hapd_glob->count_dynamic--;
+
+	if (hapd_glob->count_dynamic == 0) {
+		os_free(hapd_glob->dynamic_iface);
+		hapd_glob->dynamic_iface = NULL;
+		return 1;
+	}
+
+	tmp = os_realloc_array(hapd_glob->dynamic_iface,
+	                       hapd_glob->count_dynamic,
+			       sizeof(*hapd_glob->dynamic_iface));
+	if (!tmp) {
+		wpa_printf(MSG_ERROR, "VLAN: failed to release memory in %s",
+		                      __FUNCTION__);
+		return -1;
+	}
+	hapd_glob->dynamic_iface = tmp;
+
+	return 1;
+}
 
 static void vlan_newlink(char *ifname, struct hostapd_data *hapd)
 {
@@ -487,6 +601,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 ret;
 
 	wpa_printf(MSG_DEBUG, "VLAN: vlan_newlink(%s)", ifname);
 
@@ -506,7 +621,9 @@  static void vlan_newlink(char *ifname, struct hostapd_data *hapd)
 				            "brvlan%d", vlan->vlan_id);
 			}
 
-			if (!br_addbr(br_name))
+			ret = br_addbr(br_name);
+			if (hapd_get_dynamic_iface(NULL, br_name, (ret == 0),
+			                           hapd))
 				vlan->clean |= DVLAN_CLEAN_BR;
 
 			ifconfig_up(br_name);
@@ -524,17 +641,24 @@  static void vlan_newlink(char *ifname, struct hostapd_data *hapd)
 						    "vlan%d", vlan->vlan_id);
 
 				ifconfig_up(tagged_interface);
-				if (!vlan_add(tagged_interface, vlan->vlan_id,
-					      vlan_ifname))
+				ret = vlan_add(tagged_interface, vlan->vlan_id,
+					      vlan_ifname);
+				if (hapd_get_dynamic_iface(NULL, vlan_ifname,
+				                           (ret == 0), hapd))
 					vlan->clean |= DVLAN_CLEAN_VLAN;
 
-				if (!br_addif(br_name, vlan_ifname))
+				ret = br_addif(br_name, vlan_ifname);
+				if (hapd_get_dynamic_iface(br_name,
+				                           vlan_ifname,
+							   (ret == 0), hapd))
 					vlan->clean |= DVLAN_CLEAN_VLAN_PORT;
 
 				ifconfig_up(vlan_ifname);
 			}
 
-			if (!br_addif(br_name, ifname))
+			ret = br_addif(br_name, ifname);
+			if (hapd_get_dynamic_iface(br_name, ifname, (ret == 0),
+			                           hapd))
 				vlan->clean |= DVLAN_CLEAN_WLAN_PORT;
 
 			ifconfig_up(ifname);
@@ -573,7 +697,9 @@  static void vlan_dellink(char *ifname, struct hostapd_data *hapd)
 				            "brvlan%d", vlan->vlan_id);
 			}
 
-			if (vlan->clean & DVLAN_CLEAN_WLAN_PORT)
+			if ((vlan->clean & DVLAN_CLEAN_WLAN_PORT) &&
+			    hapd_put_dynamic_iface(br_name, vlan->ifname,
+			     hapd))
 				br_delif(br_name, vlan->ifname);
 
 			if (tagged_interface) {
@@ -587,15 +713,20 @@  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)
+				if ((vlan->clean & DVLAN_CLEAN_VLAN_PORT) &&
+				    hapd_put_dynamic_iface(br_name,
+				                            vlan_ifname, hapd))
 					br_delif(br_name, vlan_ifname);
 				ifconfig_down(vlan_ifname);
 
-				if (vlan->clean & DVLAN_CLEAN_VLAN)
+				if ((vlan->clean & DVLAN_CLEAN_VLAN) &&
+				    hapd_put_dynamic_iface(NULL,
+				                            vlan_ifname, hapd))
 					vlan_rem(vlan_ifname);
 			}
 
 			if ((vlan->clean & DVLAN_CLEAN_BR) &&
+			    hapd_put_dynamic_iface(NULL, br_name, hapd) &&
 			    br_getnumports(br_name) == 0) {
 				ifconfig_down(br_name);
 				br_delbr(br_name);