Repository navigation
Conversation
rake 13.4.2, "ruby" platform entry, CHECKSUMS section, BUNDLED WITH 4.0.22.
4e3b222 passes the remote address as variable-length sendmsg data next to the fd rights, but the length is nowhere in the protocol and the socketpair is a stream socket: nothing ties a read to exactly one record. Linux happens not to glue reads across skbs that carry fds, but that is an implementation detail, not something to build a protocol on, and a misframed record is undetectable. Frame the records explicitly. Fixed 64-byte NUL-padded records, one per connection, and TsnetAccept reads exactly one record per accept with MSG_WAITALL, asking only for the remaining bytes of the head record. The socket is a FIFO, so a read never straddles two records, under concurrent tailscale_accept calls or EINTR alike. The map stores the extracted IP string, so getremoteaddr is a plain copy. Also: - getremoteaddr now sets an error message before returning EBADF on a lookup miss; errmsg was left unset. - newConn's copy goroutines called r.Fd() after io.Copy, racing with connCleanup closing r in the other direction. Fd() on a closed os.File can return a since-reused number, so the Shutdown could hit an unrelated fd. Both copy directions now finish before connCleanup runs. - TailscaleKit: re-enable the remoteAddress assertion, disabled as flaky; the flakiness was the fd/address mixup. TestGetRemoteAddr alternates two client nodes over 200 connections, each announcing its own tailnet IP on the connection, and checks getremoteaddr against what the peer wrote. Fixes tailscale/tailscale#18310 Fixes tailscale/corp#49796
raggi
force-pushed
the
raggi/libtailscale-addr-race
branch
from
October 9, 2026 21:40
8084f50 to
f32cf38
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
4e3b222 passes the remote address as variable-length sendmsg data next
to the fd rights, but the length is nowhere in the protocol and the
socketpair is a stream socket: nothing ties a read to exactly one
record. Linux happens not to glue reads across skbs that carry fds, but
that is an implementation detail, not something to build a protocol on,
and a misframed record is undetectable.
Frame the records explicitly. Fixed 64-byte NUL-padded records, one per
connection, and TsnetAccept reads exactly one record per accept with
MSG_WAITALL, asking only for the remaining bytes of the head record.
The socket is a FIFO, so a read never straddles two records, under
concurrent tailscale_accept calls or EINTR alike. The map stores the
extracted IP string, so getremoteaddr is a plain copy.
Also:
getremoteaddr now sets an error message before returning EBADF on a
lookup miss; errmsg was left unset.
newConn's copy goroutines called r.Fd() after io.Copy, racing with
connCleanup closing r in the other direction. Fd() on a closed
os.File can return a since-reused number, so the Shutdown could hit
an unrelated fd. Both copy directions now finish before connCleanup
runs.
TailscaleKit: re-enable the remoteAddress assertion, disabled as
flaky; the flakiness was the fd/address mixup.
TestGetRemoteAddr alternates two client nodes over 200 connections, each
announcing its own tailnet IP on the connection, and checks
getremoteaddr against what the peer wrote.
Fixes tailscale/tailscale#18310
Fixes https://github.com/tailscale/corp/issues/49796