diff mbox series

[2/2] D-Bus: share 'remove all networks' with CLI

Message ID 20200820020935.954467-2-briannorris@chromium.org
State Accepted
Headers show
Series [1/2] tests: dbus: add test for RemoveAllNetworks while connected | expand

Commit Message

Brian Norris Aug. 20, 2020, 2:09 a.m. UTC
The D-Bus implementation of RemoveAllNetworks differs wildly from the
CLI implementation. Let's share the implementations.

This resolves use-after-free bugs I noticed, where we continue to use
the 'wpa_s->current_ssid' wpa_ssid object after freeing it, because we
didn't bother to disconnect from (and set to NULL) current_ssid before
freeing it.

Signed-off-by: Brian Norris <briannorris@chromium.org>
---
 wpa_supplicant/ctrl_iface.c             | 28 +--------------
 wpa_supplicant/dbus/dbus_new_handlers.c | 24 +------------
 wpa_supplicant/wpa_supplicant.c         | 45 +++++++++++++++++++++++++
 wpa_supplicant/wpa_supplicant_i.h       |  1 +
 4 files changed, 48 insertions(+), 50 deletions(-)

Comments

Jouni Malinen Oct. 10, 2020, 6:50 p.m. UTC | #1
On Wed, Aug 19, 2020 at 07:09:35PM -0700, Brian Norris wrote:
> The D-Bus implementation of RemoveAllNetworks differs wildly from the
> CLI implementation. Let's share the implementations.
> 
> This resolves use-after-free bugs I noticed, where we continue to use
> the 'wpa_s->current_ssid' wpa_ssid object after freeing it, because we
> didn't bother to disconnect from (and set to NULL) current_ssid before
> freeing it.

Thanks, both patches applied.
diff mbox series

Patch

diff --git a/wpa_supplicant/ctrl_iface.c b/wpa_supplicant/ctrl_iface.c
index 5048ee83efff..df2a20c03281 100644
--- a/wpa_supplicant/ctrl_iface.c
+++ b/wpa_supplicant/ctrl_iface.c
@@ -3478,38 +3478,12 @@  static int wpa_supplicant_ctrl_iface_remove_network(
 	struct wpa_supplicant *wpa_s, char *cmd)
 {
 	int id;
-	struct wpa_ssid *ssid;
 	int result;
 
 	/* cmd: "<network id>" or "all" */
 	if (os_strcmp(cmd, "all") == 0) {
 		wpa_printf(MSG_DEBUG, "CTRL_IFACE: REMOVE_NETWORK all");
-		if (wpa_s->sched_scanning)
-			wpa_supplicant_cancel_sched_scan(wpa_s);
-
-		eapol_sm_invalidate_cached_session(wpa_s->eapol);
-		if (wpa_s->current_ssid) {
-#ifdef CONFIG_SME
-			wpa_s->sme.prev_bssid_set = 0;
-#endif /* CONFIG_SME */
-			wpa_sm_set_config(wpa_s->wpa, NULL);
-			eapol_sm_notify_config(wpa_s->eapol, NULL, NULL);
-			if (wpa_s->wpa_state >= WPA_AUTHENTICATING)
-				wpa_s->own_disconnect_req = 1;
-			wpa_supplicant_deauthenticate(
-				wpa_s, WLAN_REASON_DEAUTH_LEAVING);
-		}
-		ssid = wpa_s->conf->ssid;
-		while (ssid) {
-			struct wpa_ssid *remove_ssid = ssid;
-			id = ssid->id;
-			ssid = ssid->next;
-			if (wpa_s->last_ssid == remove_ssid)
-				wpa_s->last_ssid = NULL;
-			wpas_notify_network_removed(wpa_s, remove_ssid);
-			wpa_config_remove_network(wpa_s->conf, id);
-		}
-		return 0;
+		return wpa_supplicant_remove_all_networks(wpa_s);
 	}
 
 	id = atoi(cmd);
diff --git a/wpa_supplicant/dbus/dbus_new_handlers.c b/wpa_supplicant/dbus/dbus_new_handlers.c
index d1f9607c602d..4b27c42c1eaf 100644
--- a/wpa_supplicant/dbus/dbus_new_handlers.c
+++ b/wpa_supplicant/dbus/dbus_new_handlers.c
@@ -1797,25 +1797,6 @@  out:
 }
 
 
-static void remove_network(void *arg, struct wpa_ssid *ssid)
-{
-	struct wpa_supplicant *wpa_s = arg;
-
-	wpas_notify_network_removed(wpa_s, ssid);
-
-	if (wpa_config_remove_network(wpa_s->conf, ssid->id) < 0) {
-		wpa_printf(MSG_ERROR,
-			   "%s[dbus]: error occurred when removing network %d",
-			   __func__, ssid->id);
-		return;
-	}
-
-	if (ssid == wpa_s->current_ssid)
-		wpa_supplicant_deauthenticate(wpa_s,
-					      WLAN_REASON_DEAUTH_LEAVING);
-}
-
-
 /**
  * wpas_dbus_handler_remove_all_networks - Remove all configured networks
  * @message: Pointer to incoming dbus message
@@ -1827,11 +1808,8 @@  static void remove_network(void *arg, struct wpa_ssid *ssid)
 DBusMessage * wpas_dbus_handler_remove_all_networks(
 	DBusMessage *message, struct wpa_supplicant *wpa_s)
 {
-	if (wpa_s->sched_scanning)
-		wpa_supplicant_cancel_sched_scan(wpa_s);
-
 	/* NB: could check for failure and return an error */
-	wpa_config_foreach_network(wpa_s->conf, remove_network, wpa_s);
+	wpa_supplicant_remove_all_networks(wpa_s);
 	return NULL;
 }
 
diff --git a/wpa_supplicant/wpa_supplicant.c b/wpa_supplicant/wpa_supplicant.c
index 3d2c0a98501a..b79439f7118a 100644
--- a/wpa_supplicant/wpa_supplicant.c
+++ b/wpa_supplicant/wpa_supplicant.c
@@ -4117,6 +4117,51 @@  int wpa_supplicant_remove_network(struct wpa_supplicant *wpa_s, int id)
 }
 
 
+/**
+ * wpa_supplicant_remove_all_networks - Remove all configured networks
+ * @wpa_s: wpa_supplicant structure for a network interface
+ * Returns: 0 on success (errors are currently ignored)
+ *
+ * This function performs the following operations:
+ * 1. Remove all networks.
+ * 2. Send network removal notifications.
+ * 3. Update internal state machines.
+ * 4. Stop any running sched scans.
+ */
+int wpa_supplicant_remove_all_networks(struct wpa_supplicant *wpa_s)
+{
+	struct wpa_ssid *ssid;
+	int id;
+
+	if (wpa_s->sched_scanning)
+		wpa_supplicant_cancel_sched_scan(wpa_s);
+
+	eapol_sm_invalidate_cached_session(wpa_s->eapol);
+	if (wpa_s->current_ssid) {
+#ifdef CONFIG_SME
+		wpa_s->sme.prev_bssid_set = 0;
+#endif /* CONFIG_SME */
+		wpa_sm_set_config(wpa_s->wpa, NULL);
+		eapol_sm_notify_config(wpa_s->eapol, NULL, NULL);
+		if (wpa_s->wpa_state >= WPA_AUTHENTICATING)
+			wpa_s->own_disconnect_req = 1;
+		wpa_supplicant_deauthenticate(
+			wpa_s, WLAN_REASON_DEAUTH_LEAVING);
+	}
+	ssid = wpa_s->conf->ssid;
+	while (ssid) {
+		struct wpa_ssid *remove_ssid = ssid;
+		id = ssid->id;
+		ssid = ssid->next;
+		if (wpa_s->last_ssid == remove_ssid)
+			wpa_s->last_ssid = NULL;
+		wpas_notify_network_removed(wpa_s, remove_ssid);
+		wpa_config_remove_network(wpa_s->conf, id);
+	}
+	return 0;
+}
+
+
 /**
  * wpa_supplicant_enable_network - Mark a configured network as enabled
  * @wpa_s: wpa_supplicant structure for a network interface
diff --git a/wpa_supplicant/wpa_supplicant_i.h b/wpa_supplicant/wpa_supplicant_i.h
index 31a9b7427585..d680d81f4f9a 100644
--- a/wpa_supplicant/wpa_supplicant_i.h
+++ b/wpa_supplicant/wpa_supplicant_i.h
@@ -1376,6 +1376,7 @@  void wpa_supplicant_reconnect(struct wpa_supplicant *wpa_s);
 
 struct wpa_ssid * wpa_supplicant_add_network(struct wpa_supplicant *wpa_s);
 int wpa_supplicant_remove_network(struct wpa_supplicant *wpa_s, int id);
+int wpa_supplicant_remove_all_networks(struct wpa_supplicant *wpa_s);
 void wpa_supplicant_enable_network(struct wpa_supplicant *wpa_s,
 				   struct wpa_ssid *ssid);
 void wpa_supplicant_disable_network(struct wpa_supplicant *wpa_s,