diff mbox series

[RFC,2/2] driver_nl82011: wait for rtnetlink event before assoc event

Message ID 20230925092249.772638658e79.I75677b755f36ca63f8289d84de29b212f4c37ec0@changeid
State RFC
Headers show
Series nl80211: wait for carrier on | expand

Commit Message

Johannes Berg Sept. 25, 2023, 7:22 a.m. UTC 
From: Johannes Berg <johannes.berg@intel.com>

There's a race (see the comment in the code) between the
kernel and userspace that can lead to dropping TX packets
after the associated event was already received. Wait for
the an RT netlink event to be indicated, so that this is
no longer possible.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 src/drivers/driver_nl80211.c       | 10 ++++++--
 src/drivers/driver_nl80211.h       |  1 +
 src/drivers/driver_nl80211_event.c | 40 ++++++++++++++++++++++++++++++
 3 files changed, 49 insertions(+), 2 deletions(-)
diff mbox series

Patch

diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c
index 9bd6a58e0a6c..f409e7af708d 100644
--- a/src/drivers/driver_nl80211.c
+++ b/src/drivers/driver_nl80211.c
@@ -1273,6 +1273,7 @@  static void wpa_driver_nl80211_event_rtm_newlink(void *ctx,
 	char ifname[IFNAMSIZ + 1];
 	char extra[100], *pos, *end;
 	int init_failed;
+	int carrier = 0;
 
 	extra[0] = '\0';
 	pos = extra;
@@ -1304,14 +1305,17 @@  static void wpa_driver_nl80211_event_rtm_newlink(void *ctx,
 			pos += os_snprintf(pos, end - pos, " linkmode=%u",
 					   nla_get_u32((struct nlattr *) attr));
 			break;
+		case IFLA_CARRIER:
+			carrier = nla_get_u8((struct nlattr *) attr);
+			break;
 		}
 		attr = RTA_NEXT(attr, attrlen);
 	}
 	extra[sizeof(extra) - 1] = '\0';
 
-	wpa_printf(MSG_DEBUG, "RTM_NEWLINK: ifi_index=%d ifname=%s%s ifi_family=%d ifi_flags=0x%x (%s%s%s%s)",
+	wpa_printf(MSG_DEBUG, "RTM_NEWLINK: ifi_index=%d ifname=%s%s ifi_family=%d ifi_flags=0x%x carrier=%d (%s%s%s%s)",
 		   ifi->ifi_index, ifname, extra, ifi->ifi_family,
-		   ifi->ifi_flags,
+		   ifi->ifi_flags, carrier,
 		   (ifi->ifi_flags & IFF_UP) ? "[UP]" : "",
 		   (ifi->ifi_flags & IFF_RUNNING) ? "[RUNNING]" : "",
 		   (ifi->ifi_flags & IFF_LOWER_UP) ? "[LOWER_UP]" : "",
@@ -1323,6 +1327,8 @@  static void wpa_driver_nl80211_event_rtm_newlink(void *ctx,
 	if (init_failed)
 		return; /* do not update interface state */
 
+	drv->carrier = carrier;
+
 	if (!drv->if_disabled && !(ifi->ifi_flags & IFF_UP)) {
 		namebuf[0] = '\0';
 		if (if_indextoname(ifi->ifi_index, namebuf) &&
diff --git a/src/drivers/driver_nl80211.h b/src/drivers/driver_nl80211.h
index aee8c451243c..29680d501df0 100644
--- a/src/drivers/driver_nl80211.h
+++ b/src/drivers/driver_nl80211.h
@@ -202,6 +202,7 @@  struct wpa_driver_nl80211_data {
 	unsigned int secure_ranging_ctx_vendor_cmd_avail:1;
 	unsigned int puncturing:1;
 	unsigned int qca_ap_allowed_freqs:1;
+	unsigned int carrier:1;
 
 	u64 vendor_scan_cookie;
 	u64 remain_on_chan_cookie;
diff --git a/src/drivers/driver_nl80211_event.c b/src/drivers/driver_nl80211_event.c
index 9d39703e0bf6..2aabb08d34d8 100644
--- a/src/drivers/driver_nl80211_event.c
+++ b/src/drivers/driver_nl80211_event.c
@@ -19,6 +19,7 @@ 
 #include "common/ieee802_11_defs.h"
 #include "common/ieee802_11_common.h"
 #include "driver_nl80211.h"
+#include "netlink.h"
 
 
 static void
@@ -255,6 +256,42 @@  static void nl80211_parse_wmm_params(struct nlattr *wmm_attr,
 }
 
 
+/*
+ * Wait for an RTM newlink event to be indicated by the kernel.
+ *
+ * There's a race condition with mac80211 and the network stack, which
+ * mostly hits depending on scheduling in time-simulation (tests),
+ * which is that mac80211 will both indicate carrier on to the network
+ * stack and send the associated/... event to userspace. Now, the
+ * internal kernel function netif_carrier_ok() immediately returns
+ * true after this, however, the linkwatch work still needs to run
+ * and change the TX queue qdisc away from noop (which drops all TX
+ * packets).
+ *
+ * When the race happens, userspace (and in particular tests) can see
+ * the associated/... event and immediately try to send a frame, at a
+ * time that the linkwatch work hasn't run yet, causing the frame to
+ * be dropped.
+ *
+ * Thus, wait here for an RTM newlink event for our interface, so in
+ * in addition to seeing the associated/... event, we also know the
+ * carrier state has actually changed sufficiently to send packets,
+ * if it was meant to change.
+ *
+ * This works because the event to userspace is also sent from the
+ * asynchronous linkwatch work.
+ *
+ * Note also that sometimes with an immediate deauth after assoc we
+ * may not ever see the carrier on, so just wait for an event, that
+ * should be sufficient.
+ */
+static void nl80211_wait_for_carrier(struct wpa_driver_nl80211_data *drv)
+{
+	if (!drv->carrier)
+		netlink_process_one_event(drv->global->netlink);
+}
+
+
 static void mlme_event_assoc(struct wpa_driver_nl80211_data *drv,
 			     const u8 *frame, size_t len, struct nlattr *wmm,
 			     struct nlattr *req_ie)
@@ -332,6 +369,7 @@  static void mlme_event_assoc(struct wpa_driver_nl80211_data *drv,
 
 	nl80211_parse_wmm_params(wmm, &event.assoc_info.wmm_params);
 
+	nl80211_wait_for_carrier(drv);
 	wpa_supplicant_event(drv->ctx, EVENT_ASSOC, &event);
 }
 
@@ -1097,6 +1135,7 @@  static void mlme_event_connect(struct wpa_driver_nl80211_data *drv,
 	if (fils_pmkid)
 		event.assoc_info.fils_pmkid = nla_data(fils_pmkid);
 
+	nl80211_wait_for_carrier(drv);
 	wpa_supplicant_event(drv->ctx, EVENT_ASSOC, &event);
 
 	/* Avoid a race condition by stopping to ignore any following
@@ -1820,6 +1859,7 @@  static void mlme_event_join_ibss(struct wpa_driver_nl80211_data *drv,
 	os_memset(&event, 0, sizeof(event));
 	event.assoc_info.freq = freq;
 
+	nl80211_wait_for_carrier(drv);
 	wpa_supplicant_event(drv->ctx, EVENT_ASSOC, &event);
 }