manager: Handle missing NEW_WIPHY events

The kernel emits NEW_WIPHY events whenever a new wiphy is registered.
Unfortunately these events are emitted under the 'legacy' semantics and
have a hard size limit of 4096 bytes.  Unfortunately, it is possible for
a NEW_WIPHY message to exceed this limit (ath10k cards seem to be
affected in particular), which results in the kernel never sending these
messages out.  This can lead to NEW_INTERFACE events being emitted with
a wiphy_id that had no corresponding NEW_WIPHY event emitted.  Such a
sequence can confuse iwd's hardware detection logic, particularly during
hot-plug or system boot.

Fix this by re-dumping the wiphy if such a condition is detected.  This
has some interaction with blacklisted wiphys, so the wiphy objects are
now always tracked and marked as blacklisted.  Before, the blacklisted
wiphys were simply not added to the iwd list of tracked wiphys.

src/manager.c

@@ -362,64 +362,6 @@ delete_interface:
 	state->pending_cmd_count++;
 }
 
-static struct wiphy_setup_state *manager_rx_cmd_new_wiphy(
-							struct l_genl_msg *msg)
-{
-	struct wiphy_setup_state *state = NULL;
-	struct wiphy *wiphy;
-	uint32_t id;
-	const char *name;
-	const char *driver, **driver_bad;
-
-	if (nl80211_parse_attrs(msg, NL80211_ATTR_WIPHY, &id,
-					NL80211_ATTR_WIPHY_NAME, &name,
-					NL80211_ATTR_UNSPEC) < 0)
-		return NULL;
-
-	/*
-	 * A Wiphy split dump can generate many (6+) NEW_WIPHY messages
-	 * We need to parse attributes from all of them, but only perform
-	 * initialization steps once for each new wiphy detected
-	 */
-	wiphy = wiphy_find(id);
-	if (wiphy)
-		goto done;
-
-	wiphy = wiphy_create(id, name);
-	if (!wiphy)
-		return NULL;
-
-	state = l_new(struct wiphy_setup_state, 1);
-	state->id = id;
-	state->wiphy = wiphy;
-	state->use_default = use_default;
-
-	l_queue_push_tail(pending_wiphys, state);
-
-	driver = wiphy_get_driver(wiphy);
-
-	for (driver_bad = default_if_driver_list; *driver_bad; driver_bad++)
-		if (fnmatch(*driver_bad, driver, 0) == 0)
-			state->use_default = true;
-
-	/*
-	 * If whitelist/blacklist were given only try to use existing
-	 * interfaces same as when the driver does not support NEW_INTERFACE
-	 * or DEL_INTERFACE, otherwise the interface names will become
-	 * meaningless after we've created our own interface(s).  Optimally
-	 * phy name white/blacklists should be used.
-	 */
-	if (whitelist_filter || blacklist_filter)
-		state->use_default = true;
-
-	if (state->use_default)
-		l_info("Wiphy %s will only use the default interface", name);
-
-done:
-	wiphy_update_from_genl(wiphy, name, msg);
-	return state;
-}
-
 static bool manager_wiphy_state_match(const void *a, const void *b)
 {
 	const struct wiphy_setup_state *state = a;
@@ -494,9 +436,33 @@ static void manager_interface_dump_callback(struct l_genl_msg *msg,
 static bool manager_check_create_interfaces(void *data, void *user_data)
 {
 	struct wiphy_setup_state *state = data;
-
 	wiphy_create_complete(state->wiphy);
 
+	state->use_default = use_default;
+
+	/*
+	 * If whitelist/blacklist were given only try to use existing
+	 * interfaces same as when the driver does not support NEW_INTERFACE
+	 * or DEL_INTERFACE, otherwise the interface names will become
+	 * meaningless after we've created our own interface(s).  Optimally
+	 * phy name white/blacklists should be used.
+	 */
+	if (whitelist_filter || blacklist_filter)
+		state->use_default = true;
+
+	if (!state->use_default) {
+		const char *driver = wiphy_get_driver(state->wiphy);
+		const char **e;
+
+		for (e = default_if_driver_list; *e; e++)
+			if (fnmatch(*e, driver, 0) == 0)
+				state->use_default = true;
+	}
+
+	if (state->use_default)
+		l_info("Wiphy %s will only use the default interface",
+				wiphy_get_name(state->wiphy));
+
 	if (!manager_wiphy_check_setup_done(state))
 		return false;
 
@@ -511,23 +477,61 @@ static void manager_interface_dump_done(void *user_data)
 				manager_check_create_interfaces, NULL);
 }
 
+/* We are dumping multiple wiphys for the very first time */
 static void manager_wiphy_dump_callback(struct l_genl_msg *msg, void *user_data)
 {
+	uint32_t id;
+	const char *name;
+	struct wiphy *wiphy;
+	struct wiphy_setup_state *state;
+
 	l_debug("");
 
-	manager_rx_cmd_new_wiphy(msg);
+	if (nl80211_parse_attrs(msg, NL80211_ATTR_WIPHY, &id,
-}
+					NL80211_ATTR_WIPHY_NAME, &name,
-
+					NL80211_ATTR_UNSPEC) < 0)
-static void manager_new_wiphy_event(struct l_genl_msg *msg)
-{
-	unsigned int wiphy_cmd_id;
-	unsigned int iface_cmd_id;
-	uint32_t wiphy_id;
-
-	if (!pending_wiphys)
 		return;
 
-	wiphy_id = manager_parse_wiphy_id(msg);
+	/*
+	 * A Wiphy split dump can generate many (6+) NEW_WIPHY messages
+	 * We need to parse attributes from all of them, but only perform
+	 * initialization steps once for each new wiphy detected
+	 */
+	wiphy = wiphy_find(id);
+	if (wiphy)
+		goto done;
+
+	wiphy = wiphy_create(id, name);
+	if (!wiphy || wiphy_is_blacklisted(wiphy))
+		return;
+
+	state = l_new(struct wiphy_setup_state, 1);
+	state->id = id;
+	state->wiphy = wiphy;
+
+	l_queue_push_tail(pending_wiphys, state);
+
+done:
+	wiphy_update_from_genl(wiphy, msg);
+}
+
+/* We are dumping a single wiphy, due to a NEW_WIPHY event */
+static void manager_wiphy_filtered_dump_callback(struct l_genl_msg *msg,
+								void *user_data)
+{
+	struct wiphy_setup_state *state = user_data;
+
+	wiphy_update_from_genl(state->wiphy, msg);
+}
+
+static int manager_wiphy_filtered_dump(uint32_t wiphy_id,
+						l_genl_msg_func_t cb,
+						void *user_data)
+{
+	struct l_genl_msg *msg;
+	unsigned int wiphy_cmd_id;
+	unsigned int iface_cmd_id;
+
 	/*
 	 * Until fixed, a NEW_WIPHY event will not include all the information
 	 * that may be available, but a dump will. Because of this we do both
@@ -537,13 +541,11 @@ static void manager_new_wiphy_event(struct l_genl_msg *msg)
 	l_genl_msg_append_attr(msg, NL80211_ATTR_SPLIT_WIPHY_DUMP, 0, NULL);
 	l_genl_msg_append_attr(msg, NL80211_ATTR_WIPHY, 4, &wiphy_id);
 
-	wiphy_cmd_id = l_genl_family_dump(nl80211, msg,
+	wiphy_cmd_id = l_genl_family_dump(nl80211, msg, cb, user_data, NULL);
-						manager_wiphy_dump_callback,
-						NULL, NULL);
 	if (!wiphy_cmd_id) {
 		l_error("Could not dump wiphy %u", wiphy_id);
 		l_genl_msg_unref(msg);
-		return;
+		return -EIO;
 	}
 
 	/*
@@ -570,15 +572,22 @@ static void manager_new_wiphy_event(struct l_genl_msg *msg)
 		l_error("Could not dump interface for wiphy %u", wiphy_id);
 		l_genl_family_cancel(nl80211, wiphy_cmd_id);
 		l_genl_msg_unref(msg);
+		return -EIO;
 	}
+
+	return 0;
 }
 
 static void manager_config_notify(struct l_genl_msg *msg, void *user_data)
 {
 	uint8_t cmd;
 	struct netdev *netdev;
+	uint32_t wiphy_id;
 	struct wiphy_setup_state *state;
 
+	if (!pending_wiphys)
+		return;
+
 	cmd = l_genl_msg_get_command(msg);
 
 	l_debug("Notification of command %s(%u)",
@@ -586,12 +595,52 @@ static void manager_config_notify(struct l_genl_msg *msg, void *user_data)
 
 	switch (cmd) {
 	case NL80211_CMD_NEW_WIPHY:
-		manager_new_wiphy_event(msg);
+	{
-		break;
+		const char *name;
+		struct wiphy *wiphy;
+
+		if (nl80211_parse_attrs(msg, NL80211_ATTR_WIPHY, &wiphy_id,
+					NL80211_ATTR_WIPHY_NAME, &name,
+					NL80211_ATTR_UNSPEC) < 0)
+			return;
+
+		/*
+		 * NEW_WIPHY events are sent in three cases:
+		 *	1. New wiphy is detected
+		 *	2. Wiphy is moved to a new namespace
+		 *	3. Wiphy is renamed
+		 *
+		 * Take care of case 3 here without re-parsing the entire
+		 * wiphy structure, potentially causing leaks, etc.
+		 */
+		wiphy = wiphy_find(wiphy_id);
+		if (wiphy) {
+			wiphy_update_name(wiphy, name);
+			return;
+		}
+
+		wiphy = wiphy_create(wiphy_id, name);
+		if (!wiphy || wiphy_is_blacklisted(wiphy))
+			return;
+
+		state = l_new(struct wiphy_setup_state, 1);
+		state->id = wiphy_id;
+		state->wiphy = wiphy;
+
+		if (manager_wiphy_filtered_dump(wiphy_id,
+					manager_wiphy_filtered_dump_callback,
+					state) < 0) {
+			wiphy_setup_state_free(state);
+			return;
+		}
+
+		l_queue_push_tail(pending_wiphys, state);
+		return;
+	}
 
 	case NL80211_CMD_DEL_WIPHY:
 		manager_del_wiphy_event(msg);
-		break;
+		return;
 
 	case NL80211_CMD_NEW_INTERFACE:
 		/*
@@ -611,17 +660,31 @@ static void manager_config_notify(struct l_genl_msg *msg, void *user_data)
 
 			if (manager_wiphy_check_setup_done(state))
 				wiphy_setup_state_destroy(state);
+
+			return;
 		}
 
-		break;
+		if (!wiphy_find(wiphy_id)) {
+			l_warn("Received a NEW_INTERFACE for a wiphy id"
+				" that isn't tracked.  This is most ikely a"
+				" kernel bug where NEW_WIPHY events that are"
+				" too large are dropped on the floor."
+				"  Attempting a workaround...");
+			manager_wiphy_filtered_dump(wiphy_id,
+						manager_wiphy_dump_callback,
+						NULL);
+			return;
+		}
+
+		return;
 
 	case NL80211_CMD_DEL_INTERFACE:
 		netdev = netdev_find(manager_parse_ifindex(msg));
 		if (!netdev)
-			break;
+			return;
 
 		netdev_destroy(netdev);
-		break;
+		return;
 	}
 }
 

src/wiphy.c

@@ -89,6 +89,8 @@ struct wiphy {
 	bool soft_rfkill : 1;
 	bool hard_rfkill : 1;
 	bool offchannel_tx_ok : 1;
+	bool blacklisted : 1;
+	bool registered : 1;
 };
 
 static struct l_queue *wiphy_list = NULL;
@@ -240,6 +242,11 @@ struct wiphy *wiphy_find(int wiphy_id)
 	return l_queue_find(wiphy_list, wiphy_match, L_UINT_TO_PTR(wiphy_id));
 }
 
+bool wiphy_is_blacklisted(const struct wiphy *wiphy)
+{
+	return wiphy->blacklisted;
+}
+
 static bool wiphy_is_managed(const char *phy)
 {
 	char *pattern;
@@ -978,37 +985,49 @@ static void wiphy_register(struct wiphy *wiphy)
 		l_info("Unable to add the %s interface to %s",
 				L_DBUS_INTERFACE_PROPERTIES,
 				wiphy_get_path(wiphy));
+
+	wiphy->registered = true;
 }
 
 struct wiphy *wiphy_create(uint32_t wiphy_id, const char *name)
 {
 	struct wiphy *wiphy;
 
-	if (!wiphy_is_managed(name))
-		return NULL;
-
 	wiphy = wiphy_new(wiphy_id);
 	l_strlcpy(wiphy->name, name, sizeof(wiphy->name));
 	l_queue_push_head(wiphy_list, wiphy);
 
-	wiphy_register(wiphy);
+	if (!wiphy_is_managed(name))
+		wiphy->blacklisted = true;
+
 	return wiphy;
 }
 
-void wiphy_update_from_genl(struct wiphy *wiphy, const char *name,
+void wiphy_update_from_genl(struct wiphy *wiphy, struct l_genl_msg *msg)
-				struct l_genl_msg *msg)
 {
+	if (wiphy->blacklisted)
+		return;
+
 	l_debug("");
 
+	wiphy_parse_attributes(wiphy, msg);
+}
+
+void wiphy_update_name(struct wiphy *wiphy, const char *name)
+{
+	bool updated = false;
+
 	if (strncmp(wiphy->name, name, sizeof(wiphy->name))) {
+		l_strlcpy(wiphy->name, name, sizeof(wiphy->name));
+		updated = true;
+	}
+
+	if (updated && wiphy->registered) {
 		struct l_dbus *dbus = dbus_get_bus();
 
-		l_strlcpy(wiphy->name, name, sizeof(wiphy->name));
 		l_dbus_property_changed(dbus, wiphy_get_path(wiphy),
 					IWD_WIPHY_INTERFACE, "Name");
 	}
-
-	wiphy_parse_attributes(wiphy, msg);
 }
 
 static void wiphy_set_station_capability_bits(struct wiphy *wiphy)
@@ -1067,6 +1086,8 @@ static void wiphy_setup_rm_enabled_capabilities(struct wiphy *wiphy)
 
 void wiphy_create_complete(struct wiphy *wiphy)
 {
+	wiphy_register(wiphy);
+
 	if (util_mem_is_zero(wiphy->permanent_addr, 6)) {
 		int err = wiphy_get_permanent_addr_from_sysfs(wiphy);
 
@@ -1088,7 +1109,8 @@ bool wiphy_destroy(struct wiphy *wiphy)
 	if (!l_queue_remove(wiphy_list, wiphy))
 		return false;
 
-	l_dbus_unregister_object(dbus_get_bus(), wiphy_get_path(wiphy));
+	if (wiphy->registered)
+		l_dbus_unregister_object(dbus_get_bus(), wiphy_get_path(wiphy));
 
 	wiphy_free(wiphy);
 	return true;

src/wiphy.h

@@ -44,12 +44,13 @@ enum ie_rsn_akm_suite wiphy_select_akm(struct wiphy *wiphy,
 					bool fils_capable_hint);
 
 struct wiphy *wiphy_find(int wiphy_id);
+bool wiphy_is_blacklisted(const struct wiphy *wiphy);
 
 struct wiphy *wiphy_create(uint32_t wiphy_id, const char *name);
+void wiphy_update_name(struct wiphy *wiphy, const char *name);
 void wiphy_create_complete(struct wiphy *wiphy);
 bool wiphy_destroy(struct wiphy *wiphy);
-void wiphy_update_from_genl(struct wiphy *wiphy, const char *name,
+void wiphy_update_from_genl(struct wiphy *wiphy, struct l_genl_msg *msg);
-				struct l_genl_msg *msg);
 
 bool wiphy_constrain_freq_set(const struct wiphy *wiphy,
 						struct scan_freq_set *set);