Skip to content

libtailscale: frame the listener fd-passing protocol, harden accept path && ruby bump - #65

Open
raggi wants to merge 2 commits into
mainfrom
raggi/libtailscale-addr-race
Open

raggi wants to merge 2 commits into
mainfrom
raggi/libtailscale-addr-race

Conversation

@raggi

@raggi raggi commented Oct 9, 2026 •

Copy link
Copy Markdown
Member

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

rake 13.4.2, "ruby" platform entry, CHECKSUMS section, BUNDLED WITH
4.0.22.
@raggi
raggi requested a review from barnstar October 9, 2026 20:41
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
raggi force-pushed the raggi/libtailscale-addr-race branch from 8084f50 to f32cf38 Compare October 9, 2026 21:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

libtailscale: sendmsg/SCM_RIGHTS Changing Connection File Descriptors

1 participant