wireguard-go
wireguard-go copied to clipboard
tailscale
tun: fix race during WriteNotify and Close
During Close(), it's possible for WriteNotify() to race a channel send with the channel close. We noticed while testing our https://github.com/coder/wgtunnel project with the go race detector.
==================
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
i think we want to use stack.wait after stack.close to prevent this, or stack.destroy that does both
Not sure the full implications of that change, since at present the stack is never closed. I can try it out and see if anything breaks...