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); +}