waterutil.IsIPv4 and IsIPv6 index packet[0] with no length check, and a
tun device can legally return zero bytes with no error — an Android
VpnService fd does exactly that. routing such a read into a classifier
panics with "index out of range [0] with length 0", and in the c-shared
library the phone loads the runtime cannot unwind past the cgo boundary,
so the panic took the app: the spoke died on 2026-10-12 with three such
lines in crash.log, no stack, and a 17-minute gap before Android
restarted it.
The knowledge was already in the tree — sourceOf and destinationOf in
handler/tun/p2p.go carry the guard, with a comment naming this exact
panic — but the eight reads in the client and server loops and the
association classifier in handler/router did not. The asymmetry was the
bug.
The guard now lives once, in internal/util/tun, as IPv4Packet and
IPv6Packet, and every classifier goes through it. Tested at the real
entry point: a fake device that returns (0, nil) before handing over a
real IPv4 packet, asserted to be survived and forwarded.
Its keepalive handling, route lookups and both delivery loops move onto
peerTable unchanged: the same magic-header reply, the same gateway
fallback, the same discard for an unrouted packet. Only the route's
value is now a name, resolved at the point of delivery. A test that
needed updating to pass would mean the socket path moved when it must
not have, so the line-by-line diff is the evidence here — no test in
this package exercises server.go.
The route table's reverse index is gone. It was a second copy of a fact
already in the route's name, and two maps cannot be made consistent:
concurrent set calls could leave the index naming one peer while the
route belonged to another, and dropPeer would then delete the winner's
route on the loser's teardown. dropPeer now scans the routes themselves
and deletes only entries still owned by the peer it is dropping.
A lock would have been the wrong fix: lookup runs per delivered packet
and dropPeer once per stream close, so the cost would have landed on the
hot path to settle a race that only matters at reclamation.
Three fixes found while wiring the p2p hub topology:
- The client keepalive was gated on network == "udp", but on a p2p link
raddr is the peer's base64 key (no port), so SplitHostPort fails and
network is "ip": the keepalive never ran, a tun server never learned the
client's route, and the read deadline was never armed. Gate on the udp
network OR a configured keepalive period; the udp branch keeps the one-shot
registration handshake it always sent.
- Liveness was refreshed only by the keepalive echo; a transient channel
hiccup can drop that single packet and redial a healthy tunnel after
3xperiod. Refresh the read deadline after any successful read instead.
- Routes were never removed: a departed client's IP -> addr stayed forever
and the device kept blackholing to a dead address. Expire a route once its
last keepalive is older than 3x the configured period, checked lazily on
the write path (no sweeper). The TTL is opt-in: without a server-side
keepalive/ttl, routes keep their current never-expiring behavior.
transportClient and transportServer each launch two goroutines for
bidirectional copy between the TUN device and the remote connection.
Previously, when one goroutine errored, the function returned
immediately while the sibling goroutine continued running — reading
from and writing to connections that would soon be closed by the
caller's deferred cleanup. In the retry loop of handleClient and
handleServer, stale goroutines accumulated across iterations, racing
with new goroutines for the TUN device and writing to closed pipes.
This caused the "io: read/write on closed pipe" error reported in
go-gost/gost#345, and more importantly, prevented recovery because
subsequent retry iterations operated on a corrupted state.
Fix:
- Derive a per-transport context in transportClient/transportServer
and check it at the top of each goroutine's loop to allow the
first-failing goroutine to signal its sibling to exit
- Replace the single-error select with a collectFirstError helper
that drains both goroutines and includes a timeout guard (5s) to
prevent deadlock when a goroutine is stuck in a non-cancelable read
- Use a per-iteration context in handleClient for the keepalive
goroutine so it is properly cancelled when an iteration ends
- Fix handleServer to use errors.Is(err, ErrTun) for consistency
- Fix transportServer's errc buffer from 1 to 2 to prevent a
goroutine leak when both goroutines error simultaneously
ginuerzh