diff mbox series

[wireless-next,3/3] wifi: nl80211: report carrier up count to userspace

Message ID 20231201114329.c43ed5db7146.Idd29862133993877b9fdff962dca3649e842249a@changeid (mailing list archive)
State Awaiting Upstream
Delegated to: Netdev Maintainers
Headers show
Series [wireless-next,1/3] wifi: nl80211: refactor nl80211_send_mlme_event() arguments | expand

Checks

Context Check Description
netdev/series_format warning Series does not have a cover letter; Target tree name not specified in the subject
netdev/tree_selection success Guessed tree name to be net-next
netdev/ynl success Generated files up to date; no warnings/errors;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1235 this patch: 1235
netdev/cc_maintainers warning 3 maintainers not CCed: kuba@kernel.org pabeni@redhat.com edumazet@google.com
netdev/build_clang success Errors and warnings before: 1165 this patch: 1165
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 1265 this patch: 1265
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 122 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 43 this patch: 43
netdev/source_inline success Was 0 now: 0

Commit Message

Johannes Berg Dec. 1, 2023, 10:41 a.m. UTC
From: Johannes Berg <johannes.berg@intel.com>

There's a race in the carrier change that happens if userspace
sees the RX association event via nl80211, but the driver (or
mac80211) just prior to that set the carrier on. The carrier
on event is actually only processed by the link watch work, so
userspace can (and we've seen this at least in simulation with
ARCH=um and time-travel) attempt to send a frame before that
has run, if it was just waiting for the association to finish
(only on open connections, of course, for encryption it has to
go through the 4-way-handshake first before sending data frames.)

There's really no completely good way to address this, I've
previously analyzed this here:
https://lore.kernel.org/netdev/346b21d87c69f817ea3c37caceb34f1f56255884.camel@sipsolutions.net/

This new solution requires both kernel and userspace work, it
basically builds on #3 outlined in the email linked above, but
with the addition of letting userspace _know_ that it may need
to wait for the rtnetlink event.

So to solve it, with this change userspace can see the value of
the carrier_up_count at the association event, and if it hasn't
yet seen the same value via an rtnetlink event (it is imperative
that it doesn't query, it must wait for events) then it can wait
for that event before trying to send data frames.

For now, implement this for association and IBSS joined events.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 include/uapi/linux/nl80211.h | 16 ++++++++++++
 net/wireless/nl80211.c       | 47 ++++++++++++++++--------------------
 2 files changed, 37 insertions(+), 26 deletions(-)
diff mbox series

Patch

diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 0cd1da2c2902..120936f81a28 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -2830,6 +2830,20 @@  enum nl80211_commands {
  * @NL80211_ATTR_MLO_LINK_DISABLED: Flag attribute indicating that the link is
  *	disabled.
  *
+ * @NL80211_ATTR_CARRIER_UP_COUNT: This u32 attribute is included in some
+ *	events that indicate a successful connection (notably association),
+ *	indicating the value of the netdev's carrier_up_count at the time
+ *	of sending this event. Userspace can use this to fix a race: when
+ *	the carrier is turned on, the actual handling thereof is done in
+ *	an asynchronous manner in the kernel. Thus, if userspace attempts
+ *	to send a frame immediately after receiving the event indicating
+ *	successful connection over nl80211, that may not go through if the
+ *	asynchronous processing in the kernel hasn't yet happened. To fix
+ *	it then userspace should be listening to rtnetlink events, and if
+ *	it didn't see the value of the carrier_up_count yet, it can wait
+ *	for a further rtnetlink event with a value equal to or bigger than
+ *	the value reported here, and only then transmit data.
+ *
  * @NUM_NL80211_ATTR: total number of nl80211_attrs available
  * @NL80211_ATTR_MAX: highest attribute number currently defined
  * @__NL80211_ATTR_AFTER_LAST: internal use
@@ -3368,6 +3382,8 @@  enum nl80211_attrs {
 
 	NL80211_ATTR_MLO_LINK_DISABLED,
 
+	NL80211_ATTR_CARRIER_UP_COUNT,
+
 	/* add attributes here, update the policy in nl80211.c */
 
 	__NL80211_ATTR_AFTER_LAST,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 403a4a38966a..d91a99f90aaa 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -818,6 +818,7 @@  static const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
 	[NL80211_ATTR_HW_TIMESTAMP_ENABLED] = { .type = NLA_FLAG },
 	[NL80211_ATTR_EMA_RNR_ELEMS] = { .type = NLA_NESTED },
 	[NL80211_ATTR_MLO_LINK_DISABLED] = { .type = NLA_FLAG },
+	[NL80211_ATTR_CARRIER_UP_COUNT] = { .type = NLA_REJECT },
 };
 
 /* policy for the key attributes */
@@ -17738,11 +17739,13 @@  void nl80211_common_reg_change_event(enum nl80211_commands cmd_id,
 
 struct nl80211_mlme_event {
 	enum nl80211_commands cmd;
+	const u8 *mac_addr;
 	const u8 *buf;
 	size_t buf_len;
 	int uapsd_queues;
 	const u8 *req_ies;
 	size_t req_ies_len;
+	u32 carrier_up_count;
 	bool reconnect;
 };
 
@@ -17766,12 +17769,17 @@  static void nl80211_send_mlme_event(struct cfg80211_registered_device *rdev,
 
 	if (nla_put_u32(msg, NL80211_ATTR_WIPHY, rdev->wiphy_idx) ||
 	    nla_put_u32(msg, NL80211_ATTR_IFINDEX, netdev->ifindex) ||
-	    nla_put(msg, NL80211_ATTR_FRAME, event->buf_len, event->buf) ||
+	    (event->buf &&
+	     nla_put(msg, NL80211_ATTR_FRAME, event->buf_len, event->buf)) ||
 	    (event->req_ies &&
 	     nla_put(msg, NL80211_ATTR_REQ_IE, event->req_ies_len,
 		     event->req_ies)))
 		goto nla_put_failure;
 
+	if (event->mac_addr &&
+	    nla_put(msg, NL80211_ATTR_MAC, ETH_ALEN, event->mac_addr))
+		goto nla_put_failure;
+
 	if (event->reconnect &&
 	    nla_put_flag(msg, NL80211_ATTR_RECONNECT_REQUESTED))
 		goto nla_put_failure;
@@ -17789,6 +17797,11 @@  static void nl80211_send_mlme_event(struct cfg80211_registered_device *rdev,
 		nla_nest_end(msg, nla_wmm);
 	}
 
+	if (event->carrier_up_count &&
+	    nla_put_u32(msg, NL80211_ATTR_CARRIER_UP_COUNT,
+			event->carrier_up_count))
+		goto nla_put_failure;
+
 	genlmsg_end(msg, hdr);
 
 	genlmsg_multicast_netns(&nl80211_fam, wiphy_net(&rdev->wiphy), msg, 0,
@@ -17824,6 +17837,7 @@  void nl80211_send_rx_assoc(struct cfg80211_registered_device *rdev,
 		.uapsd_queues = data->uapsd_queues,
 		.req_ies = data->req_ies,
 		.req_ies_len = data->req_ies_len,
+		.carrier_up_count = atomic_read(&netdev->carrier_up_count),
 	};
 
 	nl80211_send_mlme_event(rdev, netdev, &event, GFP_KERNEL);
@@ -18307,32 +18321,13 @@  void nl80211_send_ibss_bssid(struct cfg80211_registered_device *rdev,
 			     struct net_device *netdev, const u8 *bssid,
 			     gfp_t gfp)
 {
-	struct sk_buff *msg;
-	void *hdr;
+	struct nl80211_mlme_event event = {
+		.cmd = NL80211_CMD_JOIN_IBSS,
+		.mac_addr = bssid,
+		.carrier_up_count = atomic_read(&netdev->carrier_up_count),
+	};
 
-	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, gfp);
-	if (!msg)
-		return;
-
-	hdr = nl80211hdr_put(msg, 0, 0, 0, NL80211_CMD_JOIN_IBSS);
-	if (!hdr) {
-		nlmsg_free(msg);
-		return;
-	}
-
-	if (nla_put_u32(msg, NL80211_ATTR_WIPHY, rdev->wiphy_idx) ||
-	    nla_put_u32(msg, NL80211_ATTR_IFINDEX, netdev->ifindex) ||
-	    nla_put(msg, NL80211_ATTR_MAC, ETH_ALEN, bssid))
-		goto nla_put_failure;
-
-	genlmsg_end(msg, hdr);
-
-	genlmsg_multicast_netns(&nl80211_fam, wiphy_net(&rdev->wiphy), msg, 0,
-				NL80211_MCGRP_MLME, gfp);
-	return;
-
- nla_put_failure:
-	nlmsg_free(msg);
+	nl80211_send_mlme_event(rdev, netdev, &event, gfp);
 }
 
 void cfg80211_notify_new_peer_candidate(struct net_device *dev, const u8 *addr,