DF-3388 / fix.diff
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 | 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); +} |