⬢ DragonFlyBSD Kernel Audit
DF-3388 / fix.diff
← back to finding ↓ download raw
diff --git a/sys/net/wg/wg_noise.h b/sys/net/wg/wg_noise.h
index 1111111..2222222 100644
--- a/sys/net/wg/wg_noise.h
+++ b/sys/net/wg/wg_noise.h
@@ -63,6 +63,13 @@ struct noise_remote;
 
 /* Remote configuration */
+/*
+ * Deferred-teardown callback: invoked from noise_remote_put() when the
+ * remote's last reference drops.  Owners (if_wg.c) use it to free the
+ * peer object that r_arg points at, so that in-flight consumers holding
+ * remote references can never touch a freed wg_peer.
+ */
+typedef void (*noise_remote_cleanup_t)(struct noise_remote *);
+
 struct noise_remote *
 		noise_remote_alloc(struct noise_local *,
 			   const uint8_t[NOISE_PUBLIC_KEY_LEN], void *);
diff --git a/sys/net/wg/wg_noise.c b/sys/net/wg/wg_noise.c
index 1111111..2222222 100644
--- a/sys/net/wg/wg_noise.c
+++ b/sys/net/wg/wg_noise.c
@@ -134,6 +134,9 @@ struct noise_remote {
 	u_int				 r_refcnt;
 	struct noise_local		*r_local;
 	void				*r_arg;
+	bool				 r_dying;
+	noise_remote_cleanup_t	 r_cleanup;
 
 	struct lock			 r_keypair_lock;
 	struct noise_keypair		*r_keypair_next;
@@ -595,6 +598,12 @@ noise_remote_put(struct noise_remote *r)
 		noise_local_put(r->r_local);
 		lockuninit(&r->r_handshake_lock);
 		lockuninit(&r->r_keypair_lock);
+		/*
+		 * Let the owner free everything it attached to this remote
+		 * (the wg_peer) only now that no consumer can still be
+		 * running on it (port of FreeBSD's NET_EPOCH_CALL(r_cleanup)
+		 * discipline; DragonFly has no epoch(9)).
+		 */
+		if (r->r_cleanup != NULL)
+			r->r_cleanup(r);
 		explicit_bzero(r, sizeof(*r));
 		kfree(r, M_NOISE);
 	}
@@ -606,6 +615,27 @@ noise_remote_free(struct noise_remote *r)
 {
 	noise_remote_disable(r);
+
+	/*
+	 * Mark the remote as dying so that consumers which already passed
+	 * noise_remote_lookup() abort instead of re-inserting r_index into
+	 * l_index_hash after noise_remote_handshake_clear() removed it.
+	 * Without this, the dangling LIST node survives the final put()
+	 * below and every later index lookup walks freed memory.
+	 */
+	lockmgr(&r->r_handshake_lock, LK_EXCLUSIVE);
+	r->r_dying = true;
+	lockmgr(&r->r_handshake_lock, LK_RELEASE);
+
 	noise_remote_handshake_clear(r);
 	noise_remote_keypairs_clear(r);
 	noise_remote_put(r);
@@ -1140,6 +1170,10 @@ noise_consume_initiation(struct noise_local *l, uint32_t s_idx,
 	 */
 	lockmgr(&r->r_handshake_lock, LK_EXCLUSIVE);
 
+	/* Peer is being destroyed; do not resurrect its index. */
+	if (r->r_dying)
+		goto error_set;
+
 	/* Replay */
 	if (memcmp(timestamp, r->r_timestamp, NOISE_TIMESTAMP_LEN) > 0)
 		memcpy(r->r_timestamp, timestamp, NOISE_TIMESTAMP_LEN);
@@ -1269,6 +1303,10 @@ noise_consume_response(struct noise_local *l, uint32_t s_idx, uint32_t r_idx,
 
 	lockmgr(&r->r_handshake_lock, LK_EXCLUSIVE);
 	if (r->r_handshake_state == HANDSHAKE_INITIATOR &&
+	    !r->r_dying &&
 	    r->r_index.i_local_index == r_idx) {
 		r->r_handshake = hs;
 		r->r_index.i_remote_index = s_idx;
@@ -1185,6 +1223,8 @@ noise_create_response(struct noise_remote *r, uint32_t *s_idx,
 	lockmgr(&l->l_identity_lock, LK_SHARED);
 	lockmgr(&r->r_handshake_lock, LK_EXCLUSIVE);
 
+	if (r->r_dying)
+		goto error;
 	if (r->r_handshake_state != HANDSHAKE_RESPONDER)
 		goto error;
diff --git a/sys/net/wg/if_wg.c b/sys/net/wg/if_wg.c
index 1111111..2222222 100644
--- a/sys/net/wg/if_wg.c
+++ b/sys/net/wg/if_wg.c
@@ -640,6 +640,22 @@ wg_peer_destroy(struct wg_peer *peer)
 	KKASSERT(lockstatus(&sc->sc_lock, curthread) == LK_EXCLUSIVE);
 
 	/*
+	 * NOTE: the peer object itself is freed by wg_peer_remote_cleanup()
+	 * when the remote's last reference drops (noise_remote_put()); we
+	 * must not kfree() it here because in-flight handshake consumers
+	 * may still reach it through noise_remote_arg() with a live remote
+	 * reference.
+	 */
+
+	/*
 	 * Disable remote and timers.  This will prevent any new handshakes
 	 * from occuring.
 	 */
@@ -686,11 +702,8 @@ wg_peer_destroy(struct wg_peer *peer)
 	kfree(peer->p_tx_bytes, M_WG);
 	kfree(peer->p_rx_bytes, M_WG);
 
-	lockuninit(&peer->p_endpoint_lock);
-	lockuninit(&peer->p_handshake_mtx);
-
 	noise_remote_free(peer->p_remote);
 	cookie_maker_free(peer->p_cookie);
-
-	DPRINTF(sc, "Peer %ld destroyed\n", peer->p_id);
-	kfree(peer, M_WG);
+	/* peer freed in wg_peer_remote_cleanup() */
 }
+
+static void
+wg_peer_remote_cleanup(struct noise_remote *remote)
+{
+	struct wg_peer *peer = noise_remote_arg(remote);
+
+	lockuninit(&peer->p_endpoint_lock);
+	lockuninit(&peer->p_handshake_mtx);
+	DPRINTF(peer->p_sc, "Peer %ld destroyed\n", peer->p_id);
+	kfree(peer, M_WG);
+}