tun/netstack: fix race during (*netTun).Close()
#85
Conversation
This file contains 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
During `Close()`, it's possible for `WriteNotify()` to race a channel
send with the channel close. See the race below:
```
==================
WARNING: DATA RACE
Write at 0x00c000055450 by goroutine 18:
runtime.closechan()
/opt/hostedtoolcache/go/1.20.7/x64/src/runtime/chan.go:357 +0x0
golang.zx2c4.com/wireguard/tun/netstack.(*netTun).Close()
/home/runner/go/pkg/mod/golang.zx2c4.com/wireguard@v0.0.0-20230325221338-052af4a8072b/tun/netstack/tun.go:178 +0xde
golang.zx2c4.com/wireguard/device.(*Device).Close()
/home/runner/go/pkg/mod/golang.zx2c4.com/wireguard@v0.0.0-20230325221338-052af4a8072b/device/device.go:379 +0x181
github.com/coder/wgtunnel/tunneld.(*API).Close()
/home/runner/go/pkg/mod/github.com/coder/wgtunnel@v0.1.5/tunneld/tunneld.go:78 +0x64
github.com/coder/coder/coderd/devtunnel_test.newTunnelServer.func2()
/home/runner/actions-runner/_work/coder/coder/coderd/devtunnel/tunnel_test.go:211 +0x30
testing.(*common).Cleanup.func1()
/opt/hostedtoolcache/go/1.20.7/x64/src/testing/testing.go:1150 +0x193
testing.(*common).runCleanup()
/opt/hostedtoolcache/go/1.20.7/x64/src/testing/testing.go:1328 +0x1e9
testing.tRunner.func2()
/opt/hostedtoolcache/go/1.20.7/x64/src/testing/testing.go:1570 +0x52
runtime.deferreturn()
/opt/hostedtoolcache/go/1.20.7/x64/src/runtime/panic.go:476 +0x32
testing.(*T).Run.func1()
/opt/hostedtoolcache/go/1.20.7/x64/src/testing/testing.go:1629 +0x47
Previous read at 0x00c000055450 by goroutine 244:
runtime.chansend()
/opt/hostedtoolcache/go/1.20.7/x64/src/runtime/chan.go:160 +0x0
golang.zx2c4.com/wireguard/tun/netstack.(*netTun).WriteNotify()
/home/runner/go/pkg/mod/golang.zx2c4.com/wireguard@v0.0.0-20230325221338-052af4a8072b/tun/netstack/tun.go:165 +0xe6
gvisor.dev/gvisor/pkg/tcpip/link/channel.(*queue).Write()
/home/runner/go/pkg/mod/github.com/coder/gvisor@v0.0.0-20230714132058-be2e4ac102c3/pkg/tcpip/link/channel/channel.go:100 +0x183
gvisor.dev/gvisor/pkg/tcpip/link/channel.(*Endpoint).WritePackets()
/home/runner/go/pkg/mod/github.com/coder/gvisor@v0.0.0-20230714132058-be2e4ac102c3/pkg/tcpip/link/channel/channel.go:258 +0xb7
gvisor.dev/gvisor/pkg/tcpip/stack.(*delegatingQueueingDiscipline).WritePacket()
/home/runner/go/pkg/mod/github.com/coder/gvisor@v0.0.0-20230714132058-be2e4ac102c3/pkg/tcpip/stack/nic.go:146 +0x97
gvisor.dev/gvisor/pkg/tcpip/stack.(*nic).writeRawPacket()
/home/runner/go/pkg/mod/github.com/coder/gvisor@v0.0.0-20230714132058-be2e4ac102c3/pkg/tcpip/stack/nic.go:392 +0x84
gvisor.dev/gvisor/pkg/tcpip/stack.(*nic).writePacket()
/home/runner/go/pkg/mod/github.com/coder/gvisor@v0.0.0-20230714132058-be2e4ac102c3/pkg/tcpip/stack/nic.go:386 +0x59
gvisor.dev/gvisor/pkg/tcpip/stack.(*nic).WritePacket()
/home/runner/go/pkg/mod/github.com/coder/gvisor@v0.0.0-20230714132058-be2e4ac102c3/pkg/tcpip/stack/nic.go:347 +0x22b
gvisor.dev/gvisor/pkg/tcpip/network/ipv6.(*endpoint).writePacket()
/home/runner/go/pkg/mod/github.com/coder/gvisor@v0.0.0-20230714132058-be2e4ac102c3/pkg/tcpip/network/ipv6/ipv6.go:878 +0x408
gvisor.dev/gvisor/pkg/tcpip/network/ipv6.(*endpoint).WritePacket()
/home/runner/go/pkg/mod/github.com/coder/gvisor@v0.0.0-20230714132058-be2e4ac102c3/pkg/tcpip/network/ipv6/ipv6.go:829 +0x2d7
gvisor.dev/gvisor/pkg/tcpip/stack.(*Route).WritePacket()
/home/runner/go/pkg/mod/github.com/coder/gvisor@v0.0.0-20230714132058-be2e4ac102c3/pkg/tcpip/stack/route.go:495 +0xf8
gvisor.dev/gvisor/pkg/tcpip/transport/tcp.sendTCP()
/home/runner/go/pkg/mod/github.com/coder/gvisor@v0.0.0-20230714132058-be2e4ac102c3/pkg/tcpip/transport/tcp/connect.go:918 +0x3fb
gvisor.dev/gvisor/pkg/tcpip/transport/tcp.(*endpoint).sendTCP()
/home/runner/go/pkg/mod/github.com/coder/gvisor@v0.0.0-20230714132058-be2e4ac102c3/pkg/tcpip/transport/tcp/connect.go:816 +0x199
gvisor.dev/gvisor/pkg/tcpip/transport/tcp.(*endpoint).sendRaw()
/home/runner/go/pkg/mod/github.com/coder/gvisor@v0.0.0-20230714132058-be2e4ac102c3/pkg/tcpip/transport/tcp/connect.go:985 +0x53a
gvisor.dev/gvisor/pkg/tcpip/transport/tcp.(*sender).sendSegmentFromPacketBuffer()
/home/runner/go/pkg/mod/github.com/coder/gvisor@v0.0.0-20230714132058-be2e4ac102c3/pkg/tcpip/transport/tcp/snd.go:1686 +0x26d
gvisor.dev/gvisor/pkg/tcpip/transport/tcp.(*sender).sendSegment()
/home/runner/go/pkg/mod/github.com/coder/gvisor@v0.0.0-20230714132058-be2e4ac102c3/pkg/tcpip/transport/tcp/snd.go:1647 +0x386
gvisor.dev/gvisor/pkg/tcpip/transport/tcp.(*sender).maybeSendSegment()
/home/runner/go/pkg/mod/github.com/coder/gvisor@v0.0.0-20230714132058-be2e4ac102c3/pkg/tcpip/transport/tcp/snd.go:877 +0x704
gvisor.dev/gvisor/pkg/tcpip/transport/tcp.(*sender).sendData()
/home/runner/go/pkg/mod/github.com/coder/gvisor@v0.0.0-20230714132058-be2e4ac102c3/pkg/tcpip/transport/tcp/snd.go:981 +0x4fa
gvisor.dev/gvisor/pkg/tcpip/transport/tcp.(*endpoint).sendData()
/home/runner/go/pkg/mod/github.com/coder/gvisor@v0.0.0-20230714132058-be2e4ac102c3/pkg/tcpip/transport/tcp/connect.go:1009 +0xc4
gvisor.dev/gvisor/pkg/tcpip/transport/tcp.(*endpoint).shutdownLocked()
/home/runner/go/pkg/mod/github.com/coder/gvisor@v0.0.0-20230714132058-be2e4ac102c3/pkg/tcpip/transport/tcp/endpoint.go:2536 +0x44f
gvisor.dev/gvisor/pkg/tcpip/transport/tcp.(*endpoint).closeLocked()
/home/runner/go/pkg/mod/github.com/coder/gvisor@v0.0.0-20230714132058-be2e4ac102c3/pkg/tcpip/transport/tcp/endpoint.go:1063 +0xa9
gvisor.dev/gvisor/pkg/tcpip/transport/tcp.(*endpoint).Close()
/home/runner/go/pkg/mod/github.com/coder/gvisor@v0.0.0-20230714132058-be2e4ac102c3/pkg/tcpip/transport/tcp/endpoint.go:1033 +0x5d
gvisor.dev/gvisor/pkg/tcpip/adapters/gonet.(*TCPConn).Close()
/home/runner/go/pkg/mod/github.com/coder/gvisor@v0.0.0-20230714132058-be2e4ac102c3/pkg/tcpip/adapters/gonet/gonet.go:417 +0x4a
github.com/coder/wgtunnel/tunneld.(*tracingConnWrapper).Close()
/home/runner/go/pkg/mod/github.com/coder/wgtunnel@v0.1.5/tunneld/tracing.go:39 +0x7d
net/http.(*persistConn).closeLocked()
/opt/hostedtoolcache/go/1.20.7/x64/src/net/http/transport.go:2732 +0x222
net/http.(*persistConn).readLoopPeekFailLocked()
/opt/hostedtoolcache/go/1.20.7/x64/src/net/http/transport.go:2267 +0x344
net/http.(*persistConn).readLoop()
/opt/hostedtoolcache/go/1.20.7/x64/src/net/http/transport.go:2111 +0x1029
net/http.(*Transport).dialConn.func5()
/opt/hostedtoolcache/go/1.20.7/x64/src/net/http/transport.go:1765 +0x39
```
Signed-off-by: Colin Adler <colin1adler@gmail.com>
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.
During
Close(), it's possible forWriteNotify()to race a channel send with the channel close. See the race below: