Skip to content

fix(network-stack): Fix TCP sequence number wraparound in qemu network stack - #119

Open
dizk wants to merge 1 commit into
earendil-works:mainfrom
dizk:fix/tcp-seq-ack-wrap
Open

fix(network-stack): Fix TCP sequence number wraparound in qemu network stack#119
dizk wants to merge 1 commit into
earendil-works:mainfrom
dizk:fix/tcp-seq-ack-wrap

Conversation

@dizk

@dizk dizk commented Jun 4, 2026

Copy link
Copy Markdown

Hi, thanks for creating gondolin 😄

I hit an error today, you can see claude code's assessment below. It makes sense conceptually to me that you need to wrap the TCP seq, but it's a while since I had networking 101 at uni.

Everything after this line (and the code) was generated by Claude.


Summary

The userspace TCP stack tracked sequence/acknowledgement numbers as plain JS numbers that only ever increased and never wrapped at 2^32. TCP seq/ack fields are 32 bits wide and are required to wrap; not wrapping them caused both a hard crash and latent reassembly bugs near the wrap boundary.

The crash

A downstream project running v0.12.0 hit this uncaught, taking down the whole process:

RangeError [ERR_OUT_OF_RANGE]: The value of "value" is out of range.
It must be >= 0 and <= 4294967295. Received 4_294_967_340
    at Buffer.writeUInt32BE (node:internal/buffer)
    at NetworkStack.sendTCP          (host/src/qemu/network-stack.ts:1001)
    at NetworkStack.drainOutboundTcp (host/src/qemu/network-stack.ts:1422)
    at NetworkStack.handleTcpEnd     (host/src/qemu/network-stack.ts:1497)

4_294_967_340 = 2^32 + 44 — a seq/ack value that exceeded uint32 and was written to the wire without wrapping.

Root cause

  • session.myAck is seeded from the guest's SYN ISN (myAck = seq + 1) — a full random 32-bit number — then grows on every inbound byte; mySeq grows on every outbound byte. Neither was ever reduced mod 2^32.
  • A connection whose guest ISN is near 0xFFFFFFFF therefore overflows myAck after only a few dozen received bytes, and the unmasked header.writeUInt32BE(ack, 8) in sendTCP throws. The throw surfaces on the teardown path (handleTcpEnd -> drainOutboundTcp), which has no surrounding try/catch, so it is fatal.
  • Separately, every seq/ack comparison used raw < / >, which give the wrong answer once values wrap (e.g. 0 is "after" 0xFFFFFFFF but compares as smaller) — silently desyncing in-order reassembly and duplicate detection even when it does not crash.

Fix

  • Keep every counter (mySeq / myAck / vmSeq / vmAck) wrapped into [0, 2^32) at each mutation site via wrapSeq(x) = x >>> 0.
  • Compare seq/ack with RFC 1982 serial-number arithmetic — seqGt / seqLt / seqLe over a signed seqDistance(a, b) = (a - b) | 0 — instead of raw < / >. This is the same construction the Linux kernel uses (before() / after()).
  • Mask seq/ack again at the sendTCP write boundary as defense in depth, so the serialization step can never emit an out-of-range uint32.

Tests

  • Adds a deterministic regression test that reproduces the exact reported crash (crafted high guest ISN -> small inbound data -> teardown FIN) and asserts that a wrapped uint32 reaches the wire and the stack does not throw. RED before this change, GREEN after.
  • Full network-stack test suite passes (23/23).

By submitting this pull request, I confirm the following:

I understand that the entity Earendil Inc. (incorporated in the state of Delaware in 2025) needs some rights from me in order to utilize my contributions in this PR. As a contributor I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Earendil Inc. can use, modify, copy, and redistribute my contributions, under Earendil Inc.'s choice of terms.

TCP seq/ack numbers were tracked as plain JS numbers that only grew and
never wrapped at 2^32. A guest SYN ISN near 0xFFFFFFFF overflowed
session.myAck after a few dozen received bytes, and sendTCP's unmasked
header.writeUInt32BE(ack, 8) threw RangeError [ERR_OUT_OF_RANGE] uncaught
via handleTcpEnd -> drainOutboundTcp, taking down the whole process.

Keep every counter (mySeq/myAck/vmSeq/vmAck) wrapped at its mutation site,
compare seq/ack with RFC 1982 serial-number arithmetic (seqGt/seqLt/seqLe
over a signed (a-b)|0 distance) instead of raw </>, and mask again at the
sendTCP write boundary as defense in depth. Raw comparisons near the wrap
boundary would otherwise desync reassembly even without crashing.

Adds a deterministic regression test driving the exact reported crash
(high guest ISN -> small inbound data -> teardown FIN): it asserts a
wrapped uint32 reaches the wire and the stack does not throw.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.

1 participant