Skip to content

fix: correct gvisor replace directive to match module path#26822

Merged
denisra merged 2 commits into
mainfrom
denisra/fix-gvisor-replace-path
Jul 1, 2026
Merged

fix: correct gvisor replace directive to match module path#26822
denisra merged 2 commits into
mainfrom
denisra/fix-gvisor-replace-path

Conversation

@denisra

@denisra denisra commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes the gvisor replace directive in go.mod to target the correct module path.

Supersedes #26750 (closed).

Problem

PR #23055 added a replace directive to use the coder/gvisor fork (which fixes an integer overflow causing panic: length < 0 crashes). However, the directive targeted the wrong module path:

replace gvisor.dev => github.com/coder/gvisor v0.0.0-20260313164934-7a658db7b714

The actual module path declared in gvisor's go.mod is gvisor.dev/gvisor, not gvisor.dev. Go module replace directives require an exact module path match, so the previous directive was a no-op and the patched fork was never used.

Fix

-replace gvisor.dev => github.com/coder/gvisor v0.0.0-20260313164934-7a658db7b714
+replace gvisor.dev/gvisor => github.com/coder/gvisor v0.0.0-20260313164934-7a658db7b714

Validation

Verified locally with go list -m:

Before (no-op replace):

$ go list -m gvisor.dev/gvisor
gvisor.dev/gvisor v0.0.0-20240509041132-65b30f7869dc

After (correct replace):

$ go list -m gvisor.dev/gvisor
gvisor.dev/gvisor v0.0.0-20240509041132-65b30f7869dc => github.com/coder/gvisor v0.0.0-20260313164934-7a658db7b714

The => confirms the fork is now applied.

Note: go.sum will need a go mod tidy run to update checksums. CI should flag this.

Fixes #20885


Investigation context
  • The coder/gvisor fork (commit 7a658db7b714) declares module gvisor.dev/gvisor in its go.mod
  • Customer runtime stack traces show gvisor.dev/gvisor@v0.0.0-20240509041132-65b30f7869dc (unpatched upstream), confirming the fork was not applied
  • The crash is panic: length < 0 in gvisor.dev/gvisor/pkg/tcpip/transport/tcp.(*sender).splitSeg
  • Related Linear ticket: ENT-118

Generated by Coder Agents on behalf of @denisra

@denisra denisra marked this pull request as draft June 29, 2026 16:16
denisra and others added 2 commits June 29, 2026 17:45
The replace directive targeted `gvisor.dev` but the actual module path
declared in gvisor's go.mod is `gvisor.dev/gvisor`. Go module replace
directives require an exact module path match, so the previous directive
was a no-op and the patched fork was never used.

Fixes #20885
@denisra denisra force-pushed the denisra/fix-gvisor-replace-path branch from 756ab93 to 47fb272 Compare June 29, 2026 16:45
@denisra

denisra commented Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

Validation

Fully validated this fix by reproducing the bug on the unpatched gVisor version and confirming it's resolved with the fork.

Reproducer tool

Added scripts/gvisor-overflow-repro/, a standalone tool that reproduces the integer overflow from #20885. It creates a real gVisor userspace TCP stack, establishes a connection, then uses reflect+unsafe to force SndCwnd to the overflow threshold (simulating weeks of CUBIC congestion window growth with no packet loss). A subsequent write exercises the sendData path where (SndCwnd - Outstanding) * MaxPayloadSize overflows.

Run with:

go run ./scripts/gvisor-overflow-repro/
go test -v ./scripts/gvisor-overflow-repro/

Scenario 1: Unpatched gVisor (v2.33.5 customer version)

Built the reproducer against the raw upstream gvisor.dev/gvisor@v0.0.0-20240509041132-65b30f7869dc (the version v2.33.5 actually uses, since the old replace gvisor.dev => directive was a no-op).

=== gVisor TCP SndCwnd Overflow Reproducer ===
Issue: https://github.com/coder/coder/issues/20885

[1] Connection established, sending initial data...
[2] Extracting TCP endpoint internals via reflect+unsafe...
    MaxPayloadSize (MSS): 65483
    SndCwnd before:       11
    Overflow threshold:   140851397108482 (MaxInt/MSS)
    SndCwnd set to:       140851397108483 (threshold + 1)

[!] SndCwnd accepted uncapped value 140851397108483. The vulnerability may be present.

[3] Writing data to trigger TCP send path (cwndLimit calculation)...
    On vulnerable gVisor: this triggers panic('length < 0')
    On fixed gVisor:      SetSndCwnd caps the value, preventing overflow

[FAIL] PANIC caught: length < 0
       The gVisor TCP integer overflow bug is PRESENT.
       The coder/gvisor fork fix is NOT active.

=== Reproducer complete ===

Result: panic: length < 0 — matches the customer stack traces in #20885.

Scenario 2: Patched gVisor (this PR)

Built against the coder/gvisor fork (github.com/coder/gvisor@v0.0.0-20260313164934-7a658db7b714) with the corrected replace directive.

=== gVisor TCP SndCwnd Overflow Reproducer ===
Issue: https://github.com/coder/coder/issues/20885

[1] Connection established, sending initial data...
[2] Extracting TCP endpoint internals via reflect+unsafe...
    MaxPayloadSize (MSS): 65483
    SndCwnd before:       11
    Overflow threshold:   140851397108482 (MaxInt/MSS)
    SndCwnd set to:       140851397108483 (threshold + 1)

[!] SndCwnd accepted uncapped value 140851397108483. The vulnerability may be present.

[3] Writing data to trigger TCP send path (cwndLimit calculation)...
    On vulnerable gVisor: this triggers panic('length < 0')
    On fixed gVisor:      SetSndCwnd caps the value, preventing overflow

[OK] Write succeeded without panic.
     The overflow did NOT cause a panic. Fix is working.

=== Reproducer complete ===

Result: no panic. The fork's cwndLimit < 0 guard in sendData catches the overflow.

Note: the SndCwnd accepted uncapped value message appears because the reproducer writes directly to the field via unsafe (bypassing SetSndCwnd) to test the second defense layer. In normal operation, SetSndCwnd would cap the value at MaxCwnd (1<<30) before it ever reaches the overflow threshold.

Unit tests

Two tests validate both defense layers of the fork:

=== RUN   TestCwndLimitOverflowGuard
    main_test.go:122: product (SndCwnd * MSS) = -9223372036854759327 (WOULD overflow)
    main_test.go:132: Write completed without panic. cwndLimit overflow guard is working.
--- PASS: TestCwndLimitOverflowGuard (0.05s)
=== RUN   TestSetSndCwndCapsOverflow
    main_test.go:175: SetSndCwnd(1073741825) correctly capped to MaxCwnd=1073741824
    main_test.go:189: SetSndCwnd(-1) correctly capped to MaxCwnd=1073741824
--- PASS: TestSetSndCwndCapsOverflow (0.05s)
PASS

Additional validation

  • go list -m gvisor.dev/gvisor confirms the fork is active: => github.com/coder/gvisor v0.0.0-20260313164934-7a658db7b714
  • go version -m on the compiled binary confirms the fork is linked
  • Dev server built and deployed successfully from this branch

Generated by Coder Agents on behalf of @denisra

@denisra denisra force-pushed the denisra/fix-gvisor-replace-path branch 3 times, most recently from 31cc6a7 to f19c471 Compare June 30, 2026 18:21
@denisra denisra marked this pull request as ready for review June 30, 2026 19:17
@denisra denisra force-pushed the denisra/fix-gvisor-replace-path branch from f19c471 to 9c36ec8 Compare July 1, 2026 12:51
@linear-code

linear-code Bot commented Jul 1, 2026

Copy link
Copy Markdown

ENT-118

@denisra denisra force-pushed the denisra/fix-gvisor-replace-path branch 2 times, most recently from 54a8d9e to db37136 Compare July 1, 2026 14:40

@sreya sreya left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGMT but I'd remove the scripts/ additions, I think a regression test is unnecessary here

@denisra denisra force-pushed the denisra/fix-gvisor-replace-path branch from db37136 to 47fb272 Compare July 1, 2026 15:43
@denisra denisra merged commit f77d006 into main Jul 1, 2026
56 of 57 checks passed
@denisra denisra deleted the denisra/fix-gvisor-replace-path branch July 1, 2026 16:11
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 1, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: Panic after weeks of running

2 participants