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/[email protected]/tun/netstack/tun.go:178 +0xde
golang.zx2c4.com/wireguard/device.(*Device).Close()
/home/runner/go/pkg/mod/golang.zx2c4.com/[email protected]/device/device.go:379 +0x181
github.com/coder/wgtunnel/tunneld.(*API).Close()
/home/runner/go/pkg/mod/github.com/coder/[email protected]/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/[email protected]/tun/netstack/tun.go:165 +0xe6
gvisor.dev/gvisor/pkg/tcpip/link/channel.(*queue).Write()
/home/runner/go/pkg/mod/github.com/coder/[email protected]/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/[email protected]/pkg/tcpip/link/channel/channel.go:258 +0xb7
gvisor.dev/gvisor/pkg/tcpip/stack.(*delegatingQueueingDiscipline).WritePacket()
/home/runner/go/pkg/mod/github.com/coder/[email protected]/pkg/tcpip/stack/nic.go:146 +0x97
gvisor.dev/gvisor/pkg/tcpip/stack.(*nic).writeRawPacket()
/home/runner/go/pkg/mod/github.com/coder/[email protected]/pkg/tcpip/stack/nic.go:392 +0x84
gvisor.dev/gvisor/pkg/tcpip/stack.(*nic).writePacket()
/home/runner/go/pkg/mod/github.com/coder/[email protected]/pkg/tcpip/stack/nic.go:386 +0x59
gvisor.dev/gvisor/pkg/tcpip/stack.(*nic).WritePacket()
/home/runner/go/pkg/mod/github.com/coder/[email protected]/pkg/tcpip/stack/nic.go:347 +0x22b
gvisor.dev/gvisor/pkg/tcpip/network/ipv6.(*endpoint).writePacket()
/home/runner/go/pkg/mod/github.com/coder/[email protected]/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/[email protected]/pkg/tcpip/network/ipv6/ipv6.go:829 +0x2d7
gvisor.dev/gvisor/pkg/tcpip/stack.(*Route).WritePacket()
/home/runner/go/pkg/mod/github.com/coder/[email protected]/pkg/tcpip/stack/route.go:495 +0xf8
gvisor.dev/gvisor/pkg/tcpip/transport/tcp.sendTCP()
/home/runner/go/pkg/mod/github.com/coder/[email protected]/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/[email protected]/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/[email protected]/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/[email protected]/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/[email protected]/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/[email protected]/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/[email protected]/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/[email protected]/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/[email protected]/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/[email protected]/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/[email protected]/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/[email protected]/pkg/tcpip/adapters/gonet/gonet.go:417 +0x4a
github.com/coder/wgtunnel/tunneld.(*tracingConnWrapper).Close()
/home/runner/go/pkg/mod/github.com/coder/[email protected]/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 <[email protected]>
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: