[PATCH 0/3] Fix two station roaming crashes

[PATCH 0/3] Fix two station roaming crashes

[Suchir Kavi]

We run iwd on a large number of embedded aarch64 Linux devices
(WPA3-SAE, frequently multi-AP mesh networks) and root-caused two
recurring segfaults from coredumps and journals. Both are present in
current master by inspection, and together they account for the
majority of the iwd crashes we see on current builds. Heads up that
this cover letter and patchset are primarily AI-generated (Fable) but
reviewed by me.

Crash 1: use-after-free of handshake_state when a reassociation
overlaps a pending connection attempt.

netdev_connect() refuses to start while an attempt is pending
(netdev->connected || netdev->connect_cmd_id || netdev->work.id ->
-EISCONN). netdev_reassociate() has no equivalent guard, and the
station layer can invoke it mid-connect: station_cannot_roam() does
not cover the connecting states, connected_bss is already set while
connecting, and station_roam_scan_notify() does not re-check state.
Captured sequence (journal shows "state, old: connecting (auto), new:
roaming" in the same second as the crash):

1. A fresh SAE-H2E connect is queued behind a scan - the auth proto is
   created holding its raw handshake pointer, netdev->work is queued
   and never starts (sm->group_retry == -1 in the core).
2. A roam scan completes mid-connect -> netdev_reassociate() to a BSS
   with a cached PMKSA. netdev_connect_common() takes the
   PMKSA/CMD_CONNECT path, so netdev->ap is NOT replaced;
   netdev->handshake is swapped; old_hs is unreffed here and the
   caller swaps station->hs - both references gone, the handshake is
   zeroed and freed under the stale auth proto. netdev_connect_common()
   also re-inserts the already-queued embedded work item, linking the
   same wiphy_radio_work_item into the queue twice.
3. The scan finishes -> the connect work runs ->
   netdev_begin_connection() sends the CMD_CONNECT, then starts the
   stale softmac auth proto -> sae_start() -> sae_choose_next_group()
   reads sm->handshake->ecc_sae_pts from the freed (zeroed) handshake
   -> NULL dereference.

Core anatomy: sm->handshake points at zeroed memory plus allocator
metadata; netdev->ap == sm while netdev->handshake is a different,
live handshake; station state == ROAMING.

Patch 2 adds the same guard netdev_connect() has. Both fields are
zero at any legitimate roam start since netdev_connect_ok() completes
the work item, and the station roam paths fail a rejected roam
cleanly - except station_preauthenticate_cb(), which is missing a
return in its error branch and falls through to handshake_state_ref()
on the new_hs it just unreffed. Patch 1 fixes that first so the new
error return cannot widen it.

Crash 2: station_ap_directed_roam() dereferences connected_bss before
its own state guard.

station_disconnect() clears connected_bss (via
station_reset_connection_state()) before entering DISCONNECTING, and
the WNM frame watch outlives the connection - so a BSS Transition
Management request arriving in that window (mesh APs send them around
disconnects; our journal shows connected -> disconnecting 2 s before
the crash) crashes on the ignore_candidates initializer that runs
before the guard meant to drop exactly that frame. The invariant
"state == CONNECTED => connected_bss != NULL" is otherwise sound -
the unguarded sanitize memcmp just below already relies on it.
Patch 3 assigns ignore_candidates after the state check.

Suchir Kavi (3):
  station: fix use-after-free in preauthenticate_cb
  netdev: reject reassociation while connect pending
  station: fix NULL deref in ap_directed_roam

 src/netdev.c  | 3 +++
 src/station.c | 7 +++++--
 2 files changed, 8 insertions(+), 2 deletions(-)

-- 
2.54.0

[PATCH 1/3] station: fix use-after-free in preauthenticate_cb

[Suchir Kavi]

station_preauthenticate_cb() falls through its error branch: after
unreffing new_hs on a failed station_transition_reassociate(), it
proceeds to swap station->hs to a reference on the freed handshake.
Return instead.

Assisted-by: Claude:claude-fable-5
---
 src/station.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/station.c b/src/station.c
index 8fcf8c70..0b2d7dc6 100644
--- a/src/station.c
+++ b/src/station.c
@@ -2598,6 +2598,7 @@ static void station_preauthenticate_cb(struct netdev *netdev,
 	if (station_transition_reassociate(station, bss, new_hs) < 0) {
 		handshake_state_unref(new_hs);
 		station_roam_failed(station);
+		return;
 	}
 
 	handshake_state_unref(station->hs);
-- 
2.54.0

[PATCH 2/3] netdev: reject reassociation while connect pending

[Suchir Kavi]

netdev_reassociate() replaces netdev->handshake and unrefs the old
handshake without freeing netdev->ap or dequeuing a pending connection
work item. If a reassociation begins while an earlier attempt's radio
work is still queued (reachable since station_cannot_roam() does not
cover the connecting states), the stale auth proto keeps a raw pointer
to the freed handshake. On the PMKSA-cache path netdev_connect_common()
does not replace netdev->ap, and it re-inserts the already-queued
embedded work item; when the work finally runs,
netdev_begin_connection() starts the stale auth proto and sae_start()
dereferences the freed handshake:

  #0  sae_choose_next_group   <- reads handshake->ecc_sae_pts == NULL
  #1  sae_start
  #2  netdev_begin_connection
  #3  netdev_connection_work_ready
  #4  wiphy_radio_work_next
  #5  get_scan_done

Mirror netdev_connect(), which already refuses to start while
netdev->connect_cmd_id or netdev->work.id is set. Both are zero at any
legitimate roam start since netdev_connect_ok() completes the work item,
and the station roam paths handle a negative return by failing the roam
cleanly.

Assisted-by: Claude:claude-fable-5
---
 src/netdev.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/netdev.c b/src/netdev.c
index e639a1f8..dadd90da 100644
--- a/src/netdev.c
+++ b/src/netdev.c
@@ -4394,6 +4394,9 @@ int netdev_reassociate(struct netdev *netdev, const struct scan_bss *target_bss,
 	struct handshake_state *old_hs;
 	struct eapol_sm *old_sm;
 
+	if (netdev->connect_cmd_id || netdev->work.id)
+		return -EBUSY;
+
 	old_sm = netdev->sm;
 	old_hs = netdev->handshake;
 
-- 
2.54.0

[PATCH 3/3] station: fix NULL deref in ap_directed_roam

[Suchir Kavi]

station_ap_directed_roam() initialized ignore_candidates from
station->connected_bss->vendor_quirks before the state != CONNECTED
guard. station_disconnect() clears connected_bss before entering
DISCONNECTING and the WNM frame watch outlives the connection, so a BSS
Transition Management frame arriving in that window crashes on the
dereference the guard was meant to prevent.

Assign it after the state check, which guarantees connected_bss is set
(the frame-sanitize memcmp below already relies on the same invariant).

Assisted-by: Claude:claude-fable-5
---
 src/station.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/station.c b/src/station.c
index 0b2d7dc6..e3aeb3fd 100644
--- a/src/station.c
+++ b/src/station.c
@@ -3273,8 +3273,7 @@ static void station_ap_directed_roam(struct station *station,
 	uint16_t dtimer;
 	uint8_t valid_interval;
 	bool can_roam = !station_cannot_roam(station);
-	bool ignore_candidates =
-		station->connected_bss->vendor_quirks.ignore_bss_tm_candidates;
+	bool ignore_candidates;
 
 	l_debug("ifindex: %u", netdev_get_ifindex(station->netdev));
 
@@ -3283,6 +3282,9 @@ static void station_ap_directed_roam(struct station *station,
 		return;
 	}
 
+	ignore_candidates =
+		station->connected_bss->vendor_quirks.ignore_bss_tm_candidates;
+
 	/*
 	 * Sanitize the frame to check that it is from our current AP.
 	 *
-- 
2.54.0