[PATCH 1/2] Remove WPA per-VLAN groups when all stations left

[Michael Braun]

Currently, struct wpa_group is created when the first station enters the group.
But the struct wpa_group is not freed when all station left the group.

This causes a problem because wpa_group will enter FATAL_FAILURE when a
wpa_group is running while the AP_VLAN interface has already been removed.

So this adds a references counter to struct wpa_group and frees a group if it is
unused. This fixed hwsim test ap_vlan_wpa2_radius_id_change when changing back
to the old VLAN ID.

Signed-off-by: Michael Braun <michael-dev at fami-braun.de>
---
 src/ap/wpa_auth.c   | 82 +++++++++++++++++++++++++++++++++++++++++++++++++++--
 src/ap/wpa_auth_i.h |  2 ++
 2 files changed, 82 insertions(+), 2 deletions(-)

diff --git a/src/ap/wpa_auth.c b/src/ap/wpa_auth.c
index b83b460..fb61966 100644
--- a/src/ap/wpa_auth.c
+++ b/src/ap/wpa_auth.c
@@ -45,6 +45,12 @@ static int wpa_group_config_group_keys(struct wpa_authenticator *wpa_auth,
 				       struct wpa_group *group);
 static int wpa_derive_ptk(struct wpa_state_machine *sm, const u8 *snonce,
 			  const u8 *pmk, struct wpa_ptk *ptk);
+static void wpa_group_free(struct wpa_authenticator *wpa_auth,
+			   struct wpa_group *group);
+static void wpa_group_get(struct wpa_authenticator *wpa_auth,
+			  struct wpa_group *group);
+static void wpa_group_put(struct wpa_authenticator *wpa_auth,
+			  struct wpa_group *group);
 
 static const u32 dot11RSNAConfigGroupUpdateCount = 4;
 static const u32 dot11RSNAConfigPairwiseUpdateCount = 4;
@@ -262,15 +268,22 @@ static void wpa_rekey_gmk(void *eloop_ctx, void *timeout_ctx)
 static void wpa_rekey_gtk(void *eloop_ctx, void *timeout_ctx)
 {
 	struct wpa_authenticator *wpa_auth = eloop_ctx;
-	struct wpa_group *group;
+	struct wpa_group *group, *next;
 
 	wpa_auth_logger(wpa_auth, NULL, LOGGER_DEBUG, "rekeying GTK");
-	for (group = wpa_auth->group; group; group = group->next) {
+	group = wpa_auth->group;
+	while (group) {
+		wpa_group_get(wpa_auth, group);
+
 		group->GTKReKey = TRUE;
 		do {
 			group->changed = FALSE;
 			wpa_group_sm_step(wpa_auth, group);
 		} while (group->changed);
+
+		next = group->next;
+		wpa_group_put(wpa_auth, group);
+		group = next;
 	}
 
 	if (wpa_auth->conf.wpa_group_rekey) {
@@ -573,6 +586,7 @@ wpa_auth_sta_init(struct wpa_authenticator *wpa_auth, const u8 *addr,
 
 	sm->wpa_auth = wpa_auth;
 	sm->group = wpa_auth->group;
+	wpa_group_get(sm->wpa_auth, sm->group);
 
 	return sm;
 }
@@ -651,6 +665,7 @@ static void wpa_free_sta_sm(struct wpa_state_machine *sm)
 #endif /* CONFIG_IEEE80211R */
 	os_free(sm->last_rx_eapol_key);
 	os_free(sm->wpa_ie);
+	wpa_group_put(sm->wpa_auth, sm->group);
 	os_free(sm);
 }
 
@@ -3281,6 +3296,66 @@ void wpa_auth_pmksa_remove(struct wpa_authenticator *wpa_auth,
 }
 
 
+/**
+ * Remove and free the group from wpa_authenticator.
+ * This is triggered by a callback to make sure nobody
+ * is currently iterating the group list while it gets modified.
+ */
+static void wpa_group_free(struct wpa_authenticator *wpa_auth,
+			   struct wpa_group *group)
+{
+	struct wpa_group *prev = wpa_auth->group;
+
+	wpa_printf(MSG_DEBUG, "WPA: Remove group state machine for VLAN-ID %d",
+		   group->vlan_id);
+
+	while (prev) {
+		if (prev->next == group) {
+			/* This never frees the special first group as needed */
+			prev->next = group->next;
+			os_free(group);
+			break;
+		}
+		prev = prev->next;
+	}
+
+}
+
+/**
+ * Increase the reference counter for group
+ */
+static void wpa_group_get(struct wpa_authenticator *wpa_auth,
+			  struct wpa_group *group)
+{
+	// skip the special first group
+	if (wpa_auth->group == group)
+		return;
+
+	group->references++;
+}
+
+
+/**
+ * This decreases the reference counter and maybe frees the group.
+ */
+static void wpa_group_put(struct wpa_authenticator *wpa_auth,
+			  struct wpa_group *group)
+{
+	// skip the special first group
+	if (wpa_auth->group == group)
+		return;
+
+	group->references--;
+	if (group->references)
+		return;
+	wpa_group_free(wpa_auth, group);
+}
+
+
+/**
+ * This adds a group that has its references counter set to zero.
+ * Call wpa_group_get on the return value!
+ */
 static struct wpa_group *
 wpa_auth_add_group(struct wpa_authenticator *wpa_auth, int vlan_id)
 {
@@ -3331,7 +3406,10 @@ int wpa_auth_sta_set_vlan(struct wpa_state_machine *sm, int vlan_id)
 	wpa_printf(MSG_DEBUG, "WPA: Moving STA " MACSTR " to use group state "
 		   "machine for VLAN ID %d", MAC2STR(sm->addr), vlan_id);
 
+	wpa_group_get(sm->wpa_auth, group);
+	wpa_group_put(sm->wpa_auth, sm->group);
 	sm->group = group;
+
 	return 0;
 }
 
diff --git a/src/ap/wpa_auth_i.h b/src/ap/wpa_auth_i.h
index 7b2cd3e..4773915 100644
--- a/src/ap/wpa_auth_i.h
+++ b/src/ap/wpa_auth_i.h
@@ -169,6 +169,8 @@ struct wpa_group {
 	u8 IGTK[2][WPA_IGTK_MAX_LEN];
 	int GN_igtk, GM_igtk;
 #endif /* CONFIG_IEEE80211W */
+	/* number of references except those in struct wpa_group->next */
+	unsigned int references;
 };
 
 
-- 
1.9.1