[PATCH 1/6] wfd-source: Fix some races on iwd name owner change

[PATCH 1/6] wfd-source: Fix some races on iwd name owner change

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 2519 bytes --]

Subscribe to InterfacesAdded/Removed/PropertiesChanged signals before
using GetManagedObjects.  For some reason when iwd starts after the
client, we consistently get the managed objects list from before Adapter
interfaces are added but we miss the subsequent InterfacesAdded
signals, probably has to do with the GetManagedObjects and the AddMatch
calls all being synchronous.

Secondly call self.populate_devices() on init as it won't be called if
IWD is not on the bus.
---
 test/wfd-source | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/test/wfd-source b/test/wfd-source
index e74d0adf..a83cd0cb 100755
--- a/test/wfd-source
+++ b/test/wfd-source
@@ -826,6 +826,7 @@ class WFDSource(Gtk.Window):
         self.rtsp_port = 7236
         self.devices = None
         self.objects = {}
+        self.populate_devices()
         self.dbus = dbus.SystemBus()
         self.dbus.watch_name_owner('net.connman.iwd', self.on_name_owner_change)
         self.on_name_owner_change('dummy' if self.dbus.name_has_owner('net.connman.iwd') else '')
@@ -857,17 +858,6 @@ class WFDSource(Gtk.Window):
             return True
 
         manager = dbus.Interface(self.dbus.get_object('net.connman.iwd', '/'), 'org.freedesktop.DBus.ObjectManager')
-        self.devices = {}
-        self.objects = manager.GetManagedObjects()
-
-        for path in self.objects:
-            if DEVICE_IF in self.objects[path]:
-                self.add_dev(path)
-        for path in self.objects:
-            if PEER_IF in self.objects[path]:
-                self.add_peer(path)
-
-        self.populate_devices()
 
         self.dbus.add_signal_receiver(self.on_properties_changed,
             bus_name="net.connman.iwd",
@@ -883,6 +873,18 @@ class WFDSource(Gtk.Window):
             dbus_interface="org.freedesktop.DBus.ObjectManager",
             signal_name="InterfacesRemoved")
 
+        self.objects = manager.GetManagedObjects()
+        self.devices = {}
+
+        for path in self.objects:
+            if DEVICE_IF in self.objects[path]:
+                self.add_dev(path)
+        for path in self.objects:
+            if PEER_IF in self.objects[path]:
+                self.add_peer(path)
+
+        self.populate_devices()
+
         svc_mgr = dbus.Interface(self.dbus.get_object('net.connman.iwd', '/net/connman/iwd'), SVC_MGR_IF)
         svc_mgr.RegisterDisplayService({
             'Source': True,
-- 
2.25.1

[PATCH 2/6] wfd-source: Update the switch state using set_state

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 1481 bytes --]

The Gtk.Switch representing the p2p.Device's Enabled property should use
the "delayed state change" logic as described in Gtk.Switch docs, i.e.
we need to use .set_state() instead of .set_active() when we get
confirmation of the property having changed its value in the
PropertiesChanged handler.  The ::active property is automatically
changed by Gtk.Switch on user input.

This way the UI gives visual feedback of when the device enable/disable
op starts and ends (or fails).
---
 test/wfd-source | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/test/wfd-source b/test/wfd-source
index a83cd0cb..e99ae818 100755
--- a/test/wfd-source
+++ b/test/wfd-source
@@ -1116,7 +1116,7 @@ class WFDSource(Gtk.Window):
         dev_str = self.get_dev_string(path)
         name = str(device.props['Name'])
         label.set_markup(dev_str + '\n<small>' + ('Local name: ' + name + '\n' if dev_str != name else '') + 'State: ' + state + '</small>')
-        switch.set_active(device.props['Enabled'])
+        switch.set_state(device.props['Enabled'])
 
     def update_peer_props(self, dev_path, path):
         device = self.devices[dev_path]
@@ -1560,7 +1560,6 @@ class WFDSource(Gtk.Window):
         device = self.devices[path]
         if device.props['Enabled'] == state:
             return
-        device.props['Enabled'] = state
         device.props_proxy.Set(DEVICE_IF, 'Enabled', state)
         return True
 
-- 
2.25.1

[PATCH 3/6] scan: Always allocate results->bss_list

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 1924 bytes --]

Instead of creating the results->bss_list l_queue lazily, always create
one before sending the GET_SCAN command.  This is to make sure that an
empty list is passed to the scan callback (e.g. in station.c) instead of
a NULL.  Passing NULL has been causing difficult to debug crashes in
station.c, in fact I think I've been seeing them for over a year now
but can't be sure.  station_set_scan_results has been taking ownership
of the new BSS list and, if station->connected_bss was not on the list,
it would try to add it not realizing that l_queue_push_tail() was doing
nothing.  Always passing a valid list may help us prevent similar
problems in the future.

The crash might start with:
==120489== Invalid read of size 8
==120489==    at 0x425D38: network_bss_select (network.c:709)
==120489==    by 0x415BD1: station_try_next_bss (station.c:2263)
==120489==    by 0x415E31: station_retry_with_status (station.c:2323)
==120489==    by 0x415E31: station_connect_cb (station.c:2367)
==120489==    by 0x407E66: netdev_connect_failed (netdev.c:569)
==120489==    by 0x40B93D: netdev_connect_event (netdev.c:1801)
==120489==    by 0x40B93D: netdev_mlme_notify (netdev.c:3678)
---
 src/scan.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/src/scan.c b/src/scan.c
index d131b1f4..c93beabd 100644
--- a/src/scan.c
+++ b/src/scan.c
@@ -1448,9 +1448,6 @@ static void get_scan_callback(struct l_genl_msg *msg, void *user_data)
 
 	l_debug("get_scan_callback");
 
-	if (!results->bss_list)
-		results->bss_list = l_queue_new();
-
 	bss = scan_parse_result(msg, &wdev_id);
 	if (!bss)
 		return;
@@ -1694,6 +1691,7 @@ static void scan_notify(struct l_genl_msg *msg, void *user_data)
 		results->sc = sc;
 		results->time_stamp = l_time_now();
 		results->sr = sr;
+		results->bss_list = l_queue_new();
 
 		scan_parse_new_scan_results(msg, results);
 
-- 
2.25.1

[PATCH 4/6] ap: Put a public api between AP logic and DBus code

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 18270 bytes --]

Separate AP logic from DBus code, add a public API to make the AP
logic reusable from other files.
---
 Makefile.am |   2 +-
 src/ap.c    | 377 +++++++++++++++++++++++++++++++++++++---------------
 src/ap.h    |  56 ++++++++
 3 files changed, 328 insertions(+), 107 deletions(-)
 create mode 100644 src/ap.h

diff --git a/Makefile.am b/Makefile.am
index 57c694df..e83dbeee 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -207,7 +207,7 @@ src_iwd_SOURCES = src/main.c linux/nl80211.h src/iwd.h src/missing.h \
 					src/knownnetworks.c \
 					src/rfkill.h src/rfkill.c \
 					src/ft.h src/ft.c \
-					src/ap.c src/adhoc.c \
+					src/ap.h src/ap.c src/adhoc.c \
 					src/sae.h src/sae.c \
 					src/nl80211util.h src/nl80211util.c \
 					src/nl80211cmd.h src/nl80211cmd.c \
diff --git a/src/ap.c b/src/ap.c
index 9c2ce272..dd0aa224 100644
--- a/src/ap.c
+++ b/src/ap.c
@@ -45,12 +45,17 @@
 #include "src/dbus.h"
 #include "src/nl80211util.h"
 #include "src/frame-xchg.h"
+#include "src/ap.h"
 
 struct ap_state {
 	struct netdev *netdev;
 	struct l_genl_family *nl80211;
+	ap_event_func_t event_func;
+	ap_stopped_func_t stopped_func;
+	void *user_data;
 	char *ssid;
 	uint8_t channel;
+
 	unsigned int ciphers;
 	enum ie_rsn_cipher_suite group_cipher;
 	uint32_t beacon_interval;
@@ -64,7 +69,6 @@ struct ap_state {
 	uint16_t last_aid;
 	struct l_queue *sta_states;
 
-	struct l_dbus_message *pending;
 	bool started : 1;
 	bool gtk_set : 1;
 };
@@ -114,16 +118,12 @@ static void ap_reset(struct ap_state *ap)
 {
 	struct netdev *netdev = ap->netdev;
 
-	if (!ap->started)
-		return;
-
-	if (ap->pending)
-		dbus_pending_reply(&ap->pending,
-				dbus_error_aborted(ap->pending));
-
 	l_free(ap->ssid);
 
-	memset(ap->pmk, 0, sizeof(ap->pmk));
+	explicit_bzero(ap->pmk, sizeof(ap->pmk));
+
+	if (ap->mlme_watch)
+		l_genl_family_unregister(ap->nl80211, ap->mlme_watch);
 
 	frame_watch_wdev_remove(netdev_get_wdev_id(netdev));
 
@@ -136,28 +136,28 @@ static void ap_reset(struct ap_state *ap)
 		l_uintset_free(ap->rates);
 
 	ap->started = false;
-
-	l_dbus_property_changed(dbus_get_bus(), netdev_get_path(ap->netdev),
-						IWD_AP_INTERFACE, "Started");
-}
-
-static void ap_free(void *data)
-{
-	struct ap_state *ap = data;
-
-	ap_reset(ap);
-	l_genl_family_free(ap->nl80211);
-	l_free(ap);
 }
 
 static void ap_del_station(struct sta_state *sta, uint16_t reason,
 				bool disassociate)
 {
 	struct ap_state *ap = sta->ap;
+	struct ap_event_station_removed_data event_data;
+	bool send_event = false;
 
 	netdev_del_station(ap->netdev, sta->addr, reason, disassociate);
 	sta->associated = false;
-	sta->rsna = false;
+
+	if (sta->rsna) {
+		if (ap->event_func) {
+			memset(&event_data, 0, sizeof(event_data));
+			event_data.mac = sta->addr;
+			event_data.reason = reason;
+			send_event = true;
+		}
+
+		sta->rsna = false;
+	}
 
 	if (sta->gtk_query_cmd_id) {
 		l_genl_family_cancel(ap->nl80211, sta->gtk_query_cmd_id);
@@ -172,6 +172,10 @@ static void ap_del_station(struct sta_state *sta, uint16_t reason,
 
 	sta->hs = NULL;
 	sta->sm = NULL;
+
+	if (send_event)
+		ap->event_func(AP_EVENT_STATION_REMOVED, &event_data,
+				ap->user_data);
 }
 
 static bool ap_sta_match_addr(const void *a, const void *b)
@@ -205,13 +209,19 @@ static void ap_del_key_cb(struct l_genl_msg *msg, void *user_data)
 
 static void ap_new_rsna(struct sta_state *sta)
 {
+	struct ap_state *ap = sta->ap;
+
 	l_debug("STA "MAC" authenticated", MAC_STR(sta->addr));
 
 	sta->rsna = true;
-	/*
-	 * TODO: Once new AP interface is implemented this is where a
-	 * new "ConnectedPeer" property will be added.
-	 */
+
+	if (ap->event_func) {
+		struct ap_event_station_added_data event_data = {};
+		event_data.mac = sta->addr;
+		event_data.rsn_ie = sta->assoc_rsne;
+		ap->event_func(AP_EVENT_STATION_ADDED, &event_data,
+				ap->user_data);
+	}
 }
 
 static void ap_drop_rsna(struct sta_state *sta)
@@ -250,6 +260,13 @@ static void ap_drop_rsna(struct sta_state *sta)
 
 	sta->hs = NULL;
 	sta->sm = NULL;
+
+	if (ap->event_func) {
+		struct ap_event_station_removed_data event_data = {};
+		event_data.mac = sta->addr;
+		ap->event_func(AP_EVENT_STATION_REMOVED, &event_data,
+				ap->user_data);
+	}
 }
 
 static void ap_set_rsn_info(struct ap_state *ap, struct ie_rsn_info *rsn)
@@ -1305,26 +1322,18 @@ static void ap_start_cb(struct l_genl_msg *msg, void *user_data)
 
 	ap->start_stop_cmd_id = 0;
 
-	if (!ap->pending)
-		return;
-
 	if (l_genl_msg_get_error(msg) < 0) {
 		l_error("START_AP failed: %i", l_genl_msg_get_error(msg));
 
-		dbus_pending_reply(&ap->pending,
-				dbus_error_invalid_args(ap->pending));
+		ap->event_func(AP_EVENT_START_FAILED, NULL, ap->user_data);
 		ap_reset(ap);
-
+		l_genl_family_free(ap->nl80211);
+		l_free(ap);
 		return;
 	}
 
-	dbus_pending_reply(&ap->pending,
-			l_dbus_message_new_method_return(ap->pending));
-
 	ap->started = true;
-
-	l_dbus_property_changed(dbus_get_bus(), netdev_get_path(ap->netdev),
-						IWD_AP_INTERFACE, "Started");
+	ap->event_func(AP_EVENT_STARTED, NULL, ap->user_data);
 }
 
 static struct l_genl_msg *ap_build_cmd_start_ap(struct ap_state *ap)
@@ -1406,25 +1415,53 @@ static void ap_mlme_notify(struct l_genl_msg *msg, void *user_data)
 
 	switch (l_genl_msg_get_command(msg)) {
 	case NL80211_CMD_STOP_AP:
-		if (ap->start_stop_cmd_id)
-			break;
+		if (ap->start_stop_cmd_id) {
+			l_genl_family_cancel(ap->nl80211, ap->start_stop_cmd_id);
+			ap->start_stop_cmd_id = 0;
+			ap->event_func(AP_EVENT_START_FAILED, NULL, ap->user_data);
+		} else if (ap->started) {
+			ap->started = false;
+			ap->event_func(AP_EVENT_STOPPING, NULL, ap->user_data);
+		}
 
 		ap_reset(ap);
+		l_genl_family_free(ap->nl80211);
+		l_free(ap);
 		break;
 	}
 }
 
-static int ap_start(struct ap_state *ap, const char *ssid, const char *psk,
-		struct l_dbus_message *message)
+/*
+ * Start a simple independent WPA2 AP on given netdev.
+ *
+ * @event_func is required and must react to AP_EVENT_START_FAILED
+ * and AP_EVENT_STOPPING by forgetting the ap_state struct, which
+ * is going to be freed automatically.
+ * @channel is optional.
+ */
+struct ap_state *ap_start(struct netdev *netdev, const char *ssid,
+				const char *psk, int channel,
+				ap_event_func_t event_func, void *user_data)
 {
-	struct netdev *netdev = ap->netdev;
+	struct ap_state *ap;
 	struct wiphy *wiphy = netdev_get_wiphy(netdev);
 	struct l_genl_msg *cmd;
 	uint64_t wdev_id = netdev_get_wdev_id(netdev);
 
+	ap = l_new(struct ap_state, 1);
+	ap->nl80211 = l_genl_family_new(iwd_get_genl(), NL80211_GENL_NAME);
 	ap->ssid = l_strdup(ssid);
-	/* TODO: Start a Get Survey to decide the channel */
-	ap->channel = 6;
+	ap->netdev = netdev;
+	ap->event_func = event_func;
+	ap->user_data = user_data;
+
+	if (channel)
+		ap->channel = channel;
+	else {
+		/* TODO: Start a Get Survey to decide the channel */
+		ap->channel = 6;
+	}
+
 	/* TODO: Add all ciphers supported by wiphy */
 	ap->ciphers = wiphy_select_cipher(wiphy, 0xffff);
 	ap->group_cipher = wiphy_select_cipher(wiphy, 0xffff);
@@ -1485,37 +1522,30 @@ static int ap_start(struct ap_state *ap, const char *ssid, const char *psk,
 		goto error;
 	}
 
-	ap->pending = l_dbus_message_ref(message);
-
-	return 0;
+	return ap;
 
 error:
 	ap_reset(ap);
-
-	return -EIO;
+	l_genl_family_free(ap->nl80211);
+	l_free(ap);
+	return NULL;
 }
 
 static void ap_stop_cb(struct l_genl_msg *msg, void *user_data)
 {
 	struct ap_state *ap = user_data;
+	int error = l_genl_msg_get_error(msg);
 
 	ap->start_stop_cmd_id = 0;
 
-	if (!ap->pending)
-		goto end;
+	if (error < 0)
+		l_error("STOP_AP failed: %s (%i)", strerror(error), error);
 
-	if (l_genl_msg_get_error(msg) < 0) {
-		l_error("STOP_AP failed: %i", l_genl_msg_get_error(msg));
-		dbus_pending_reply(&ap->pending,
-				dbus_error_failed(ap->pending));
-		goto end;
-	}
-
-	dbus_pending_reply(&ap->pending,
-			l_dbus_message_new_method_return(ap->pending));
+	if (ap->stopped_func)
+		ap->stopped_func(ap->user_data);
 
-end:
-	ap_reset(ap);
+	l_genl_family_free(ap->nl80211);
+	l_free(ap);
 }
 
 static struct l_genl_msg *ap_build_cmd_stop_ap(struct ap_state *ap)
@@ -1529,81 +1559,207 @@ static struct l_genl_msg *ap_build_cmd_stop_ap(struct ap_state *ap)
 	return cmd;
 }
 
-static int ap_stop(struct ap_state *ap, struct l_dbus_message *message)
+/*
+ * Schedule the running @ap to be stopped and freed.  The original
+ * event_func and user_data are forgotten and a new callback can be
+ * provided if the caller needs to know when the interface becomes
+ * free, for example for a new ap_start call.
+ *
+ * The user must forget @ap when @stopped_func is called.  If the
+ * @user_data ends up being destroyed before that, ap_free(ap) should
+ * be used to prevent @stopped_func from being called.
+ * If @stopped_func is not provided, the caller must forget @ap
+ * immediately.
+ */
+void ap_shutdown(struct ap_state *ap, ap_stopped_func_t stopped_func,
+			void *user_data)
 {
 	struct l_genl_msg *cmd;
 
-	cmd = ap_build_cmd_stop_ap(ap);
-	if (!cmd)
-		return -ENOMEM;
+	if (ap->started) {
+		ap->started = false;
+		ap->event_func(AP_EVENT_STOPPING, NULL, ap->user_data);
+	}
 
-	if (ap->start_stop_cmd_id)
-		l_genl_family_cancel(ap->nl80211, ap->start_stop_cmd_id);
+	ap_reset(ap);
 
-	if (ap->mlme_watch)
-		l_genl_family_unregister(ap->nl80211, ap->mlme_watch);
+	if (ap->gtk_set) {
+		ap->gtk_set = false;
+
+		cmd = ap_build_cmd_del_key(ap);
+		if (!cmd) {
+			l_error("ap_build_cmd_del_key failed");
+			goto free_ap;
+		}
+
+		if (!l_genl_family_send(ap->nl80211, cmd, ap_gtk_op_cb, NULL,
+					NULL)) {
+			l_genl_msg_unref(cmd);
+			l_error("Issuing DEL_KEY failed");
+			goto free_ap;
+		}
+	}
+
+	cmd = ap_build_cmd_stop_ap(ap);
+	if (!cmd) {
+		l_error("ap_build_cmd_stop_ap failed");
+		goto free_ap;
+	}
 
 	ap->start_stop_cmd_id = l_genl_family_send(ap->nl80211, cmd, ap_stop_cb,
 							ap, NULL);
 	if (!ap->start_stop_cmd_id) {
 		l_genl_msg_unref(cmd);
-		return -EIO;
+		l_error("Sending STOP_AP failed");
+		goto free_ap;
 	}
 
-	if (ap->gtk_set) {
-		struct l_genl_msg *msg;
+	ap->stopped_func = stopped_func;
+	ap->user_data = user_data;
+	return;
 
-		ap->gtk_set = false;
+free_ap:
+	if (stopped_func)
+		stopped_func(user_data);
 
-		msg = ap_build_cmd_del_key(ap);
-		if (!l_genl_family_send(ap->nl80211, msg, ap_gtk_op_cb, NULL,
-					NULL)) {
-			l_genl_msg_unref(msg);
-			l_error("Issuing DEL_KEY failed");
-		}
-	}
+	l_genl_family_free(ap->nl80211);
+	l_free(ap);
+}
 
-	ap->pending = l_dbus_message_ref(message);
+/* Free @ap without a graceful shutdown */
+void ap_free(struct ap_state *ap)
+{
+	ap_reset(ap);
+	l_genl_family_free(ap->nl80211);
+	l_free(ap);
+}
 
-	return 0;
+bool ap_station_disconnect(struct ap_state *ap, const uint8_t *mac,
+				enum mmpdu_reason_code reason)
+{
+	struct sta_state *sta;
+
+	if (!ap->started)
+		return false;
+
+	sta = l_queue_remove_if(ap->sta_states, ap_sta_match_addr, mac);
+	if (!sta)
+		return false;
+
+	ap_del_station(sta, reason, false);
+	ap_sta_free(sta);
+	return true;
+}
+
+struct ap_if_data {
+	struct netdev *netdev;
+	struct ap_state *ap;
+	struct l_dbus_message *pending;
+};
+
+static void ap_if_event_func(enum ap_event_type type, const void *event_data,
+				void *user_data)
+{
+	struct ap_if_data *ap_if = user_data;
+	struct l_dbus_message *reply;
+
+	switch (type) {
+	case AP_EVENT_START_FAILED:
+		if (L_WARN_ON(!ap_if->pending))
+			break;
+
+		reply = dbus_error_failed(ap_if->pending);
+		dbus_pending_reply(&ap_if->pending, reply);
+		ap_if->ap = NULL;
+		break;
+
+	case AP_EVENT_STARTED:
+		if (L_WARN_ON(!ap_if->pending))
+			break;
+
+		reply = l_dbus_message_new_method_return(ap_if->pending);
+		dbus_pending_reply(&ap_if->pending, reply);
+		l_dbus_property_changed(dbus_get_bus(),
+					netdev_get_path(ap_if->netdev),
+					IWD_AP_INTERFACE, "Started");
+		break;
+
+	case AP_EVENT_STOPPING:
+		l_dbus_property_changed(dbus_get_bus(),
+					netdev_get_path(ap_if->netdev),
+					IWD_AP_INTERFACE, "Started");
+
+		if (!ap_if->pending)
+			ap_if->ap = NULL;
+
+		break;
+
+	case AP_EVENT_STATION_ADDED:
+	case AP_EVENT_STATION_REMOVED:
+		/* Ignored */
+		break;
+	}
 }
 
 static struct l_dbus_message *ap_dbus_start(struct l_dbus *dbus,
 		struct l_dbus_message *message, void *user_data)
 {
-	struct ap_state *ap = user_data;
+	struct ap_if_data *ap_if = user_data;
 	const char *ssid, *wpa2_psk;
 
-	if (ap->pending)
-		return dbus_error_busy(message);
-
-	if (ap->started)
+	if (ap_if->ap && ap_if->ap->started)
 		return dbus_error_already_exists(message);
 
+	if (ap_if->ap || ap_if->pending)
+		return dbus_error_busy(message);
+
 	if (!l_dbus_message_get_arguments(message, "ss", &ssid, &wpa2_psk))
 		return dbus_error_invalid_args(message);
 
-	if (ap_start(ap, ssid, wpa2_psk, message) < 0)
+	ap_if->ap = ap_start(ap_if->netdev, ssid, wpa2_psk, 0,
+				ap_if_event_func, ap_if);
+	if (!ap_if->ap)
 		return dbus_error_invalid_args(message);
 
+	ap_if->pending = l_dbus_message_ref(message);
 	return NULL;
 }
 
+static void ap_dbus_stop_cb(void *user_data)
+{
+	struct ap_if_data *ap_if = user_data;
+	struct l_dbus_message *reply;
+
+	if (L_WARN_ON(!ap_if->pending))
+		return;
+
+	reply = l_dbus_message_new_method_return(ap_if->pending);
+	dbus_pending_reply(&ap_if->pending, reply);
+	ap_if->ap = NULL;
+}
+
 static struct l_dbus_message *ap_dbus_stop(struct l_dbus *dbus,
 		struct l_dbus_message *message, void *user_data)
 {
-	struct ap_state *ap = user_data;
+	struct ap_if_data *ap_if = user_data;
 
-	if (ap->pending)
-		return dbus_error_busy(message);
+	if (!ap_if->ap) {
+		if (ap_if->pending)
+			return dbus_error_busy(message);
 
-	/* already stopped, no-op */
-	if (!ap->started)
+		/* already stopped, no-op */
 		return l_dbus_message_new_method_return(message);
+	}
 
-	if (ap_stop(ap, message) < 0)
-		return dbus_error_failed(message);
+	if (ap_if->pending) {
+		struct l_dbus_message *reply;
+
+		reply = dbus_error_aborted(ap_if->pending);
+		dbus_pending_reply(&ap_if->pending, reply);
+	}
 
+	ap_if->pending = l_dbus_message_ref(message);
+	ap_shutdown(ap_if->ap, ap_dbus_stop_cb, ap_if);
 	return NULL;
 }
 
@@ -1612,8 +1768,8 @@ static bool ap_dbus_property_get_started(struct l_dbus *dbus,
 					struct l_dbus_message_builder *builder,
 					void *user_data)
 {
-	struct ap_state *ap = user_data;
-	bool started = ap->started;
+	struct ap_if_data *ap_if = user_data;
+	bool started = ap_if->ap && ap_if->ap->started;
 
 	l_dbus_message_builder_append_basic(builder, 'b', &started);
 
@@ -1632,27 +1788,36 @@ static void ap_setup_interface(struct l_dbus_interface *interface)
 
 static void ap_destroy_interface(void *user_data)
 {
-	struct ap_state *ap = user_data;
+	struct ap_if_data *ap_if = user_data;
+
+	if (ap_if->pending) {
+		struct l_dbus_message *reply;
+
+		reply = dbus_error_aborted(ap_if->pending);
+		dbus_pending_reply(&ap_if->pending, reply);
+	}
 
-	ap_free(ap);
+	if (ap_if->ap)
+		ap_free(ap_if->ap);
+
+	l_free(ap_if);
 }
 
 static void ap_add_interface(struct netdev *netdev)
 {
-	struct ap_state *ap;
+	struct ap_if_data *ap_if;
 
 	/*
 	 * TODO: Check wiphy supported channels and NL80211_ATTR_TX_FRAME_TYPES
 	 */
 
 	/* just allocate/set device, Start method will complete setup */
-	ap = l_new(struct ap_state, 1);
-	ap->netdev = netdev;
-	ap->nl80211 = l_genl_family_new(iwd_get_genl(), NL80211_GENL_NAME);
+	ap_if = l_new(struct ap_if_data, 1);
+	ap_if->netdev = netdev;
 
 	/* setup ap dbus interface */
 	l_dbus_object_add_interface(dbus_get_bus(),
-			netdev_get_path(netdev), IWD_AP_INTERFACE, ap);
+			netdev_get_path(netdev), IWD_AP_INTERFACE, ap_if);
 }
 
 static void ap_remove_interface(struct netdev *netdev)
diff --git a/src/ap.h b/src/ap.h
new file mode 100644
index 00000000..1fd8b63e
--- /dev/null
+++ b/src/ap.h
@@ -0,0 +1,56 @@
+/*
+ *
+ *  Wireless daemon for Linux
+ *
+ *  Copyright (C) 2020  Intel Corporation. All rights reserved.
+ *
+ *  This library is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU Lesser General Public
+ *  License as published by the Free Software Foundation; either
+ *  version 2.1 of the License, or (at your option) any later version.
+ *
+ *  This library is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ *  Lesser General Public License for more details.
+ *
+ *  You should have received a copy of the GNU Lesser General Public
+ *  License along with this library; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+struct ap_state;
+struct iovec;
+
+enum ap_event_type {
+	AP_EVENT_START_FAILED,
+	AP_EVENT_STARTED,
+	AP_EVENT_STOPPING,
+	AP_EVENT_STATION_ADDED,
+	AP_EVENT_STATION_REMOVED,
+};
+
+struct ap_event_station_added_data {
+	const uint8_t *mac;
+	const uint8_t *rsn_ie;
+};
+
+struct ap_event_station_removed_data {
+	const uint8_t *mac;
+	enum mmpdu_reason_code reason;
+};
+
+typedef void (*ap_event_func_t)(enum ap_event_type type, const void *event_data,
+				void *user_data);
+typedef void (*ap_stopped_func_t)(void *user_data);
+
+struct ap_state *ap_start(struct netdev *netdev, const char *ssid,
+				const char *psk, int channel,
+				ap_event_func_t event_func, void *user_data);
+void ap_shutdown(struct ap_state *ap, ap_stopped_func_t stopped_func,
+			void *user_data);
+void ap_free(struct ap_state *ap);
+
+bool ap_station_disconnect(struct ap_state *ap, const uint8_t *mac,
+				enum mmpdu_reason_code reason);
-- 
2.25.1

[PATCH 5/6] ap: Add a no_cck_rates flag

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 3588 bytes --]

---
 src/ap.c | 33 ++++++++++++++++++++++++++-------
 src/ap.h |  2 +-
 2 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/src/ap.c b/src/ap.c
index dd0aa224..412f307d 100644
--- a/src/ap.c
+++ b/src/ap.c
@@ -71,6 +71,7 @@ struct ap_state {
 
 	bool started : 1;
 	bool gtk_set : 1;
+	bool no_cck_rates : 1;
 };
 
 struct sta_state {
@@ -382,6 +383,11 @@ static uint32_t ap_send_mgmt_frame(struct ap_state *ap,
 		l_genl_msg_append_attr(msg, NL80211_ATTR_DONT_WAIT_FOR_ACK,
 					0, NULL);
 
+	if (ap->no_cck_rates)
+		l_genl_msg_append_attr(msg, NL80211_ATTR_TX_NO_CCK_RATE, 0,
+					NULL);
+
+
 	id = l_genl_family_send(ap->nl80211, msg, callback, user_data, NULL);
 
 	if (!id)
@@ -1440,7 +1446,7 @@ static void ap_mlme_notify(struct l_genl_msg *msg, void *user_data)
  * @channel is optional.
  */
 struct ap_state *ap_start(struct netdev *netdev, const char *ssid,
-				const char *psk, int channel,
+				const char *psk, int channel, bool no_cck_rates,
 				ap_event_func_t event_func, void *user_data)
 {
 	struct ap_state *ap;
@@ -1452,6 +1458,7 @@ struct ap_state *ap_start(struct netdev *netdev, const char *ssid,
 	ap->nl80211 = l_genl_family_new(iwd_get_genl(), NL80211_GENL_NAME);
 	ap->ssid = l_strdup(ssid);
 	ap->netdev = netdev;
+	ap->no_cck_rates = no_cck_rates;
 	ap->event_func = event_func;
 	ap->user_data = user_data;
 
@@ -1466,11 +1473,23 @@ struct ap_state *ap_start(struct netdev *netdev, const char *ssid,
 	ap->ciphers = wiphy_select_cipher(wiphy, 0xffff);
 	ap->group_cipher = wiphy_select_cipher(wiphy, 0xffff);
 	ap->beacon_interval = 100;
-	/* TODO: Use actual supported rates */
-	ap->rates = l_uintset_new(200);
-	l_uintset_put(ap->rates, 2); /* 1 Mbps*/
-	l_uintset_put(ap->rates, 11); /* 5.5 Mbps*/
-	l_uintset_put(ap->rates, 22); /* 11 Mbps*/
+
+	/* TODO: Pick from actual supported rates */
+	if (no_cck_rates) {
+		l_uintset_put(ap->rates, 12); /* 6 Mbps*/
+		l_uintset_put(ap->rates, 18); /* 9 Mbps*/
+		l_uintset_put(ap->rates, 24); /* 12 Mbps*/
+		l_uintset_put(ap->rates, 36); /* 18 Mbps*/
+		l_uintset_put(ap->rates, 48); /* 24 Mbps*/
+		l_uintset_put(ap->rates, 72); /* 36 Mbps*/
+		l_uintset_put(ap->rates, 96); /* 48 Mbps*/
+		l_uintset_put(ap->rates, 108); /* 54 Mbps*/
+	} else {
+		ap->rates = l_uintset_new(200);
+		l_uintset_put(ap->rates, 2); /* 1 Mbps*/
+		l_uintset_put(ap->rates, 11); /* 5.5 Mbps*/
+		l_uintset_put(ap->rates, 22); /* 11 Mbps*/
+	}
 
 	if (crypto_psk_from_passphrase(psk, (uint8_t *) ssid, strlen(ssid),
 					ap->pmk) < 0)
@@ -1716,7 +1735,7 @@ static struct l_dbus_message *ap_dbus_start(struct l_dbus *dbus,
 	if (!l_dbus_message_get_arguments(message, "ss", &ssid, &wpa2_psk))
 		return dbus_error_invalid_args(message);
 
-	ap_if->ap = ap_start(ap_if->netdev, ssid, wpa2_psk, 0,
+	ap_if->ap = ap_start(ap_if->netdev, ssid, wpa2_psk, 0, false,
 				ap_if_event_func, ap_if);
 	if (!ap_if->ap)
 		return dbus_error_invalid_args(message);
diff --git a/src/ap.h b/src/ap.h
index 1fd8b63e..2da0596f 100644
--- a/src/ap.h
+++ b/src/ap.h
@@ -46,7 +46,7 @@ typedef void (*ap_event_func_t)(enum ap_event_type type, const void *event_data,
 typedef void (*ap_stopped_func_t)(void *user_data);
 
 struct ap_state *ap_start(struct netdev *netdev, const char *ssid,
-				const char *psk, int channel,
+				const char *psk, int channel, bool no_cck_rates,
 				ap_event_func_t event_func, void *user_data);
 void ap_shutdown(struct ap_state *ap, ap_stopped_func_t stopped_func,
 			void *user_data);
-- 
2.25.1

[PATCH 6/6] ap: Add authorized_macs parameter

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 3036 bytes --]

---
 src/ap.c | 31 ++++++++++++++++++++++++++++++-
 src/ap.h |  1 +
 2 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/src/ap.c b/src/ap.c
index 412f307d..f7fb17ea 100644
--- a/src/ap.c
+++ b/src/ap.c
@@ -55,6 +55,8 @@ struct ap_state {
 	void *user_data;
 	char *ssid;
 	uint8_t channel;
+	uint8_t *authorized_macs;
+	int authorized_macs_num;
 
 	unsigned int ciphers;
 	enum ie_rsn_cipher_suite group_cipher;
@@ -136,6 +138,7 @@ static void ap_reset(struct ap_state *ap)
 	if (ap->rates)
 		l_uintset_free(ap->rates);
 
+	l_free(ap->authorized_macs);
 	ap->started = false;
 }
 
@@ -1243,6 +1246,19 @@ static void ap_auth_cb(const struct mmpdu_header *hdr, const void *body,
 			memcmp(hdr->address_3, bssid, 6))
 		return;
 
+	if (ap->authorized_macs) {
+		int i;
+
+		for (i = 0; i < ap->authorized_macs_num; i++)
+			if (!memcmp(from, ap->authorized_macs + i * 6, 6))
+				break;
+
+		if (i == ap->authorized_macs_num) {
+			ap_auth_reply(ap, from, MMPDU_REASON_CODE_UNSPECIFIED);
+			return;
+		}
+	}
+
 	/* Only Open System authentication implemented here */
 	if (L_LE16_TO_CPU(auth->algorithm) !=
 			MMPDU_AUTH_ALGO_OPEN_SYSTEM) {
@@ -1447,6 +1463,7 @@ static void ap_mlme_notify(struct l_genl_msg *msg, void *user_data)
  */
 struct ap_state *ap_start(struct netdev *netdev, const char *ssid,
 				const char *psk, int channel, bool no_cck_rates,
+				const uint8_t **authorized_macs,
 				ap_event_func_t event_func, void *user_data)
 {
 	struct ap_state *ap;
@@ -1469,6 +1486,18 @@ struct ap_state *ap_start(struct netdev *netdev, const char *ssid,
 		ap->channel = 6;
 	}
 
+	if (authorized_macs) {
+		int i;
+
+		for (i = 0; authorized_macs[i]; i++);
+		ap->authorized_macs = l_malloc(i * 6);
+		ap->authorized_macs_num = i;
+
+		for (i = 0; authorized_macs[i]; i++)
+			memcpy(ap->authorized_macs + i * 6, authorized_macs[i],
+				6);
+	}
+
 	/* TODO: Add all ciphers supported by wiphy */
 	ap->ciphers = wiphy_select_cipher(wiphy, 0xffff);
 	ap->group_cipher = wiphy_select_cipher(wiphy, 0xffff);
@@ -1735,7 +1764,7 @@ static struct l_dbus_message *ap_dbus_start(struct l_dbus *dbus,
 	if (!l_dbus_message_get_arguments(message, "ss", &ssid, &wpa2_psk))
 		return dbus_error_invalid_args(message);
 
-	ap_if->ap = ap_start(ap_if->netdev, ssid, wpa2_psk, 0, false,
+	ap_if->ap = ap_start(ap_if->netdev, ssid, wpa2_psk, 0, false, NULL,
 				ap_if_event_func, ap_if);
 	if (!ap_if->ap)
 		return dbus_error_invalid_args(message);
diff --git a/src/ap.h b/src/ap.h
index 2da0596f..b007b657 100644
--- a/src/ap.h
+++ b/src/ap.h
@@ -47,6 +47,7 @@ typedef void (*ap_stopped_func_t)(void *user_data);
 
 struct ap_state *ap_start(struct netdev *netdev, const char *ssid,
 				const char *psk, int channel, bool no_cck_rates,
+				const uint8_t **authorized_macs,
 				ap_event_func_t event_func, void *user_data);
 void ap_shutdown(struct ap_state *ap, ap_stopped_func_t stopped_func,
 			void *user_data);
-- 
2.25.1

Re: [PATCH 6/6] ap: Add authorized_macs parameter

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 973 bytes --]

Hi Andrew,

On 8/4/20 9:53 AM, Andrew Zaborowski wrote:
> ---
>   src/ap.c | 31 ++++++++++++++++++++++++++++++-
>   src/ap.h |  1 +
>   2 files changed, 31 insertions(+), 1 deletion(-)
> 

<snip>

> diff --git a/src/ap.h b/src/ap.h
> index 2da0596f..b007b657 100644
> --- a/src/ap.h
> +++ b/src/ap.h
> @@ -47,6 +47,7 @@ typedef void (*ap_stopped_func_t)(void *user_data);
>   
>   struct ap_state *ap_start(struct netdev *netdev, const char *ssid,
>   				const char *psk, int channel, bool no_cck_rates,
> +				const uint8_t **authorized_macs,
>   				ap_event_func_t event_func, void *user_data);

By the way, you're pretty much at the limit of sane number of arguments here. 
If you decide to add any more, I suggest a struct ap_config object of some sort 
with a nice set/get api that ap_start could take ownership of.

>   void ap_shutdown(struct ap_state *ap, ap_stopped_func_t stopped_func,
>   			void *user_data);
> 

Regards,
-Denis