diff mbox

[PATCHv2,06/21] 802.1X: Make VLAN-keys a list instead of an array.

Message ID b0925d0d868b6b3bde0075b4f258d46d04d492d1.1370512966.git.michael-dev@fami-braun.de
State Changes Requested
Headers show

Commit Message

michael-dev May 17, 2013, 8:42 a.m. UTC 
Currently, 802.1X code relies on vlan_id to be a small integer
so it can use an array to keep them. When increasing the number
of VLANs supported, this no longer is feasable.

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

Comments

Jouni Malinen June 25, 2013, 8:04 a.m. UTC | #1 
On Fri, May 17, 2013 at 10:42:33AM +0200, Michael Braun wrote:
> Currently, 802.1X code relies on vlan_id to be a small integer
> so it can use an array to keep them. When increasing the number
> of VLANs supported, this no longer is feasable.

> diff --git a/src/ap/ieee802_1x.c b/src/ap/ieee802_1x.c

> @@ -238,13 +241,21 @@ ieee802_1x_get_group(struct hostapd_data *hapd, struct hostapd_ssid *ssid,
>  		     size_t vlan_id)
>  {
>  	const char *ifname;
> +	int i;
> +	int free = -1;

This results in a compiler warnings with free shadowing a global
declaration (free()). Please use different variable name here.

> +	if (free < 0) {
> +		/* resize array */
> +		int free = ssid->count_dyn_vlan_keys; /* aka oldsize */
> +		int newsize = (free == 0) ? 1 : 2 * free;

And this is even more confusing since the inner free variable would
shadow the other one within this block in the function.

> +	ssid->dyn_vlan_keys[free] = ieee802_1x_group_alloc(hapd, ifname,
> +	                                                   vlan_id);

This going back to using the top level 'free' within the function..

Was this supposed to be two different variables? If so, please use
different names for them (neither of which should be 'free' to avoid
shadowing free()).
diff mbox

Patch

diff --git a/src/ap/ap_config.c b/src/ap/ap_config.c
index 4c69dd6..7a3a8d9 100644
--- a/src/ap/ap_config.c
+++ b/src/ap/ap_config.c
@@ -453,10 +453,10 @@  static void hostapd_config_free_bss(struct hostapd_bss_config *conf)
 	if (conf->ssid.dyn_vlan_keys) {
 		struct hostapd_ssid *ssid = &conf->ssid;
 		size_t i;
-		for (i = 0; i <= ssid->max_dyn_vlan_keys; i++) {
+		for (i = 0; i <= ssid->count_dyn_vlan_keys; i++) {
 			if (ssid->dyn_vlan_keys[i] == NULL)
 				continue;
-			hostapd_config_free_wep(ssid->dyn_vlan_keys[i]);
+			hostapd_config_free_wep(&ssid->dyn_vlan_keys[i]->key);
 			os_free(ssid->dyn_vlan_keys[i]);
 		}
 		os_free(ssid->dyn_vlan_keys);
diff --git a/src/ap/ap_config.h b/src/ap/ap_config.h
index 98783fe..7ca4647 100644
--- a/src/ap/ap_config.h
+++ b/src/ap/ap_config.h
@@ -40,6 +40,11 @@  struct hostapd_wep_keys {
 	size_t default_len; /* key length used for dynamic key generation */
 };
 
+struct hostapd_vlan_wep_keys {
+	int vlan_id;
+	struct hostapd_wep_keys key;
+};
+
 typedef enum hostap_security_policy {
 	SECURITY_PLAINTEXT = 0,
 	SECURITY_STATIC_WEP = 1,
@@ -74,8 +79,8 @@  struct hostapd_ssid {
 #ifdef CONFIG_FULL_DYNAMIC_VLAN
 	char *vlan_tagged_interface;
 #endif /* CONFIG_FULL_DYNAMIC_VLAN */
-	struct hostapd_wep_keys **dyn_vlan_keys;
-	size_t max_dyn_vlan_keys;
+	struct hostapd_vlan_wep_keys **dyn_vlan_keys;
+	size_t count_dyn_vlan_keys;
 };
 
 
diff --git a/src/ap/hostapd.c b/src/ap/hostapd.c
index 780b2e2..8860632 100644
--- a/src/ap/hostapd.c
+++ b/src/ap/hostapd.c
@@ -210,6 +210,8 @@  static int hostapd_broadcast_wep_set(struct hostapd_data *hapd)
 {
 	int errors = 0, idx;
 	struct hostapd_ssid *ssid = &hapd->conf->ssid;
+	const char *ifname;
+	struct hostapd_vlan_wep_keys *key;
 
 	idx = ssid->wep.idx;
 	if (ssid->wep.default_len &&
@@ -223,21 +225,20 @@  static int hostapd_broadcast_wep_set(struct hostapd_data *hapd)
 
 	if (ssid->dyn_vlan_keys) {
 		size_t i;
-		for (i = 0; i <= ssid->max_dyn_vlan_keys; i++) {
-			const char *ifname;
-			struct hostapd_wep_keys *key = ssid->dyn_vlan_keys[i];
+		for (i = 0; i <= ssid->count_dyn_vlan_keys; i++) {
+			key = ssid->dyn_vlan_keys[i];
 			if (key == NULL)
 				continue;
 			ifname = hostapd_get_vlan_id_ifname(hapd->conf->vlan,
-							    i);
+							    key->vlan_id);
 			if (ifname == NULL)
 				continue;
 
-			idx = key->idx;
+			idx = key->key.idx;
 			if (hostapd_drv_set_key(ifname, hapd, WPA_ALG_WEP,
 						broadcast_ether_addr, idx, 1,
-						NULL, 0, key->key[idx],
-						key->len[idx])) {
+						NULL, 0, key->key.key[idx],
+						key->key.len[idx])) {
 				wpa_printf(MSG_WARNING, "Could not set "
 					   "dynamic VLAN WEP encryption.");
 				errors++;
diff --git a/src/ap/ieee802_1x.c b/src/ap/ieee802_1x.c
index d3dda14..8b5076d 100644
--- a/src/ap/ieee802_1x.c
+++ b/src/ap/ieee802_1x.c
@@ -187,15 +187,18 @@  static void ieee802_1x_tx_key_one(struct hostapd_data *hapd,
 
 
 #ifndef CONFIG_NO_VLAN
-static struct hostapd_wep_keys *
-ieee802_1x_group_alloc(struct hostapd_data *hapd, const char *ifname)
+static struct hostapd_vlan_wep_keys *
+ieee802_1x_group_alloc(struct hostapd_data *hapd, const char *ifname,
+                       int vlan_id)
 {
+	struct hostapd_vlan_wep_keys *vlan_key;
 	struct hostapd_wep_keys *key;
-
-	key = os_zalloc(sizeof(*key));
-	if (key == NULL)
+	vlan_key = os_zalloc(sizeof(*vlan_key));
+	if (vlan_key == NULL)
 		return NULL;
+	key = &vlan_key->key;
 
+	vlan_key->vlan_id = vlan_id;
 	key->default_len = hapd->conf->default_wep_key_len;
 
 	if (key->idx >= hapd->conf->broadcast_key_idx_max ||
@@ -229,7 +232,7 @@  ieee802_1x_group_alloc(struct hostapd_data *hapd, const char *ifname)
 
 	hostapd_set_drv_ieee8021x(hapd, ifname, 1);
 
-	return key;
+	return vlan_key;
 }
 
 
@@ -238,13 +241,21 @@  ieee802_1x_get_group(struct hostapd_data *hapd, struct hostapd_ssid *ssid,
 		     size_t vlan_id)
 {
 	const char *ifname;
+	int i;
+	int free = -1;
 
 	if (vlan_id == 0)
 		return &ssid->wep;
 
-	if (vlan_id <= ssid->max_dyn_vlan_keys && ssid->dyn_vlan_keys &&
-	    ssid->dyn_vlan_keys[vlan_id])
-		return ssid->dyn_vlan_keys[vlan_id];
+	for (i = 0; i < ssid->count_dyn_vlan_keys; i++) {
+		struct hostapd_vlan_wep_keys *key = ssid->dyn_vlan_keys[i];
+		if (!key && free < 0)
+			free = i;
+		if (!key)
+			continue;
+		if (key->vlan_id == vlan_id)
+			return &key->key;
+	}
 
 	wpa_printf(MSG_DEBUG, "IEEE 802.1X: Creating new group "
 		   "state machine for VLAN ID %lu",
@@ -258,30 +269,26 @@  ieee802_1x_get_group(struct hostapd_data *hapd, struct hostapd_ssid *ssid,
 		return NULL;
 	}
 
-	if (ssid->dyn_vlan_keys == NULL) {
-		int size = (vlan_id + 1) * sizeof(ssid->dyn_vlan_keys[0]);
-		ssid->dyn_vlan_keys = os_zalloc(size);
-		if (ssid->dyn_vlan_keys == NULL)
-			return NULL;
-		ssid->max_dyn_vlan_keys = vlan_id;
-	}
+	if (free < 0) {
+		/* resize array */
+		int free = ssid->count_dyn_vlan_keys; /* aka oldsize */
+		int newsize = (free == 0) ? 1 : 2 * free;
 
-	if (ssid->max_dyn_vlan_keys < vlan_id) {
-		struct hostapd_wep_keys **na;
-		int size = (vlan_id + 1) * sizeof(ssid->dyn_vlan_keys[0]);
-		na = os_realloc(ssid->dyn_vlan_keys, size);
+		struct hostapd_vlan_wep_keys **na;
+		na = os_realloc_array(ssid->dyn_vlan_keys, newsize,
+		                       sizeof(*ssid->dyn_vlan_keys));
 		if (na == NULL)
 			return NULL;
+		os_memset(&ssid->dyn_vlan_keys[free + 1], 0,
+		          (newsize - free) * sizeof(ssid->dyn_vlan_keys[0]));
 		ssid->dyn_vlan_keys = na;
-		os_memset(&ssid->dyn_vlan_keys[ssid->max_dyn_vlan_keys + 1], 0,
-			  (vlan_id - ssid->max_dyn_vlan_keys) *
-			  sizeof(ssid->dyn_vlan_keys[0]));
-		ssid->max_dyn_vlan_keys = vlan_id;
+		ssid->count_dyn_vlan_keys = newsize;
 	}
 
-	ssid->dyn_vlan_keys[vlan_id] = ieee802_1x_group_alloc(hapd, ifname);
+	ssid->dyn_vlan_keys[free] = ieee802_1x_group_alloc(hapd, ifname,
+	                                                   vlan_id);
 
-	return ssid->dyn_vlan_keys[vlan_id];
+	return &ssid->dyn_vlan_keys[free]->key;
 }
 #endif /* CONFIG_NO_VLAN */