diff --git a/sys/net/wg/if_wg.c b/sys/net/wg/if_wg.c --- a/sys/net/wg/if_wg.c +++ b/sys/net/wg/if_wg.c @@ -41,6 +41,7 @@ #include /* SIOC* ioctl commands */ #include #include +#include #include @@ -261,6 +262,8 @@ uint64_t *p_tx_bytes; uint64_t *p_rx_bytes; + u_int p_refcnt; /* DF-0315: peer lifetime refcount */ + LIST_HEAD(, wg_aip) p_aips; size_t p_aips_num; }; @@ -588,6 +591,9 @@ peer = kmalloc(sizeof(*peer), M_WG, M_WAITOK | M_ZERO); + /* DF-0315: the create reference (owned by the sc->sc_peers table). */ + refcount_init(&peer->p_refcnt, 1); + peer->p_remote = noise_remote_alloc(sc->sc_local, pub_key, peer); if ((*errp = noise_remote_enable(peer->p_remote)) != 0) { noise_remote_free(peer->p_remote); @@ -636,6 +642,48 @@ return (peer); } +/* + * DF-0315: reference counting for wg_peer. The data-plane paths + * (wg_output/wg_aip_lookup, wg_encrypt/wg_decrypt, wg_deliver_in/out) obtain a + * peer and use it WITHOUT holding sc->sc_lock; the previous code kfree()'d the + * peer in wg_peer_destroy() while those paths could still be dereferencing it + * (the noise_remote refcount protects only the remote, not the peer). We now + * keep the peer alive until the last reference is dropped. + */ +struct wg_peer * +wg_peer_ref(struct wg_peer *peer) +{ + refcount_acquire(&peer->p_refcnt); + return (peer); +} + +void +wg_peer_put(struct wg_peer *peer) +{ + if (refcount_release(&peer->p_refcnt)) { + /* + * Last reference gone. All data-plane / task references have + * been released, so it is now safe to tear the peer down and + * free it. No sc_lock is required here. + */ + wg_queue_deinit(&peer->p_decrypt_serial); + wg_queue_deinit(&peer->p_encrypt_serial); + wg_queue_deinit(&peer->p_stage_queue); + + 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(peer->p_sc, "Peer %ld freed\n", peer->p_id); + kfree(peer, M_WG); + } +} + static void wg_peer_destroy(struct wg_peer *peer) { @@ -644,52 +692,33 @@ KKASSERT(lockstatus(&sc->sc_lock, curthread) == LK_EXCLUSIVE); /* - * Disable remote and timers. This will prevent any new handshakes - * from occuring. + * Disable remote and timers, and remove the peer's allowed IPs and its + * sc_peers entry. After this no NEW data-plane references can be taken + * (wg_aip_lookup() will no longer find it), so once the in-flight ones + * are dropped the peer will be freed by wg_peer_put(). */ noise_remote_disable(peer->p_remote); wg_timers_disable(peer); - /* - * Remove all allowed IPs, so no more packets will be routed to - * this peer. - */ wg_aip_remove_all(sc, peer); - /* Remove peer from the interface, then free. */ sc->sc_peers_num--; TAILQ_REMOVE(&sc->sc_peers, peer, p_entry); - /* - * While there are no references remaining, we may still have - * p_{send,recv}_task executing (think empty queue, but - * wg_deliver_{in,out} needs to check the queue). We should wait - * for them and then free. - */ - taskqueue_drain(peer->p_recv_taskqueue, &peer->p_recv_task); - taskqueue_drain(peer->p_send_taskqueue, &peer->p_send_task); - callout_terminate(&peer->p_new_handshake); callout_terminate(&peer->p_send_keepalive); callout_terminate(&peer->p_retry_handshake); callout_terminate(&peer->p_persistent_keepalive); callout_terminate(&peer->p_zero_key_material); - wg_queue_deinit(&peer->p_decrypt_serial); - wg_queue_deinit(&peer->p_encrypt_serial); - wg_queue_deinit(&peer->p_stage_queue); - - 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); + taskqueue_drain(peer->p_recv_taskqueue, &peer->p_recv_task); + taskqueue_drain(peer->p_send_taskqueue, &peer->p_send_task); DPRINTF(sc, "Peer %ld destroyed\n", peer->p_id); - kfree(peer, M_WG); + + /* Drop the table's reference; frees the peer once all data-plane + * references held by wg_output()/wg_aip_lookup() etc. are released. */ + wg_peer_put(peer); } static void @@ -878,6 +907,9 @@ if (node != NULL) { peer = ((struct wg_aip *)node)->a_peer; noise_remote_ref(peer->p_remote); + /* DF-0315: keep the peer (not just its remote) alive for the + * caller, which uses it without holding sc_lock. */ + wg_peer_ref(peer); } else { peer = NULL; } @@ -1848,6 +1880,7 @@ remote = noise_keypair_remote(pkt->p_keypair); peer = noise_remote_arg(remote); + wg_peer_ref(peer); /* DF-0315: hold peer ref for the encrypt path */ m = pkt->p_mbuf; padlen = calculate_padding(pkt); @@ -1872,6 +1905,7 @@ atomic_store_rel_int(&pkt->p_state, state); taskqueue_enqueue(peer->p_send_taskqueue, &peer->p_send_task); noise_remote_put(remote); + wg_peer_put(peer); /* DF-0315 */ } static void @@ -1885,6 +1919,7 @@ remote = noise_keypair_remote(pkt->p_keypair); peer = noise_remote_arg(remote); + wg_peer_ref(peer); /* DF-0315: hold peer ref for the decrypt path */ m = pkt->p_mbuf; pkt->p_counter = le64toh(mtod(m, struct wg_pkt_data *)->counter); @@ -1922,8 +1957,10 @@ } /* Drop the reference, since no need to dereference it. */ - if (allowed_peer != NULL) + if (allowed_peer != NULL) { noise_remote_put(allowed_peer->p_remote); + wg_peer_put(allowed_peer); /* DF-0315: wg_aip_lookup() ref */ + } if (__predict_false(peer != allowed_peer)) { DPRINTF(sc, "Packet has disallowed src IP from peer %ld\n", @@ -1938,6 +1975,7 @@ atomic_store_rel_int(&pkt->p_state, state); taskqueue_enqueue(peer->p_recv_taskqueue, &peer->p_recv_task); noise_remote_put(remote); + wg_peer_put(peer); /* DF-0315 */ } static void @@ -2186,12 +2224,14 @@ remote = noise_keypair_remote(pkt->p_keypair); peer = noise_remote_arg(remote); + wg_peer_ref(peer); /* DF-0315: hold peer ref across queue/decrypt */ if (!wg_queue_both(&sc->sc_decrypt_parallel, &peer->p_decrypt_serial, pkt)) IFNET_STAT_INC(sc->sc_ifp, iqdrops, 1); wg_decrypt_dispatch(sc); noise_remote_put(remote); + wg_peer_put(peer); /* DF-0315 */ return; } @@ -2350,6 +2390,7 @@ wg_queue_push_staged(&peer->p_stage_queue, pkt); wg_peer_send_staged(peer); noise_remote_put(peer->p_remote); + wg_peer_put(peer); /* DF-0315: drop the wg_aip_lookup() peer ref */ return (0); @@ -2374,8 +2415,10 @@ pkt->p_mbuf = NULL; /* m already freed above */ wg_packet_free(pkt); } - if (peer != NULL) + if (peer != NULL) { noise_remote_put(peer->p_remote); + wg_peer_put(peer); /* DF-0315: drop the wg_aip_lookup() peer ref */ + } return (ret); }