Skip to content

Commit 890623d

Browse files
spikecurtisspikecurtis
committed
Fix segfault
1 parent c820eee commit 890623d

File tree

2 files changed

+31
-20
lines changed

2 files changed

+31
-20
lines changed

‎coderd/devtunnel/tunnel.go

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ func NewWithConfig(ctx context.Context, logger slog.Logger, cfg Config) (*Tunnel
6060

6161
tun, tnet, err := netstack.CreateNetTUN(
6262
[]netip.Addr{server.ClientIP},
63-
[]netip.Addr{},
63+
[]netip.Addr{netip.AddrFrom4([4]byte{1, 1, 1, 1})},
6464
1280,
6565
)
6666
if err != nil {
@@ -71,17 +71,23 @@ func NewWithConfig(ctx context.Context, logger slog.Logger, cfg Config) (*Tunnel
7171
if err != nil {
7272
return nil, nil, xerrors.Errorf("resolve endpoint: %w", err)
7373
}
74+
// In IPv6, we need to enclose the address to in [] before passing to wireguard's endpoint key, like
75+
// [2001:abcd::1]:8888. We'll use netip.AddrPort to correctly handle this.
76+
wgAddr, err := netip.ParseAddr(wgip.String())
77+
if err != nil {
78+
return nil, nil, xerrors.Errorf("parse address: %w", err)
79+
}
80+
wgEndpoint := netip.AddrPortFrom(wgAddr, cfg.Tunnel.WireguardPort)
7481

75-
dev := device.NewDevice(tun, conn.NewDefaultBind(), device.NewLogger(device.LogLevelSilent, ""))
82+
dev := device.NewDevice(tun, conn.NewDefaultBind(), device.NewLogger(device.LogLevelError, "devtunnel "))
7683
err = dev.IpcSet(fmt.Sprintf(`private_key=%s
7784
public_key=%s
78-
endpoint=[%s]:%d
85+
endpoint=%s
7986
persistent_keepalive_interval=21
8087
allowed_ip=%s/128`,
8188
hex.EncodeToString(cfg.PrivateKey[:]),
8289
server.ServerPublicKey,
83-
wgip.IP.String(),
84-
cfg.Tunnel.WireguardPort,
90+
wgEndpoint.String(),
8591
server.ServerIP.String(),
8692
))
8793
if err != nil {
@@ -103,6 +109,9 @@ allowed_ip=%s/128`,
103109
select {
104110
case <-ctx.Done():
105111
_ = wgListen.Close()
112+
// We need to remove peers before closing to avoid a race condition between dev.Close() and the peer
113+
// goroutines which results in segfault.
114+
dev.RemoveAllPeers()
106115
dev.Close()
107116
<-routineEnd
108117
close(ch)

‎coderd/devtunnel/tunnel_test.go

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package devtunnel_test
22

33
import (
4+
"cdr.dev/slog"
45
"context"
56
"encoding/hex"
67
"encoding/json"
@@ -25,6 +26,18 @@ import (
2526
"github.com/coder/coder/coderd/devtunnel"
2627
)
2728

29+
const (
30+
ipByte1 = 0xfc
31+
ipByte2 = 0xca
32+
wgPort = 48732
33+
)
34+
35+
var (
36+
serverIP = netip.AddrFrom16([16]byte{ipByte1, ipByte2, 15: 0x1})
37+
dnsIP = netip.AddrFrom4([4]byte{1, 1, 1, 1})
38+
clientIP = netip.AddrFrom16([16]byte{ipByte1, ipByte2, 15: 0x2})
39+
)
40+
2841
// The tunnel leaks a few goroutines that aren't impactful to production scenarios.
2942
// func TestMain(m *testing.M) {
3043
// goleak.VerifyTestMain(m)
@@ -51,7 +64,7 @@ func TestTunnel(t *testing.T) {
5164
fTunServer := newFakeTunnelServer(t)
5265
cfg := fTunServer.config()
5366

54-
tun, errCh, err := devtunnel.NewWithConfig(ctx, slogtest.Make(t, nil), cfg)
67+
tun, errCh, err := devtunnel.NewWithConfig(ctx, slogtest.Make(t, nil).Leveled(slog.LevelDebug), cfg)
5568
require.NoError(t, err)
5669
t.Log(tun.URL)
5770

@@ -99,18 +112,6 @@ type fakeTunnelServer struct {
99112
server *httptest.Server
100113
}
101114

102-
const (
103-
ipByte1 = 0xfc
104-
ipByte2 = 0xca
105-
wgPort = 48732
106-
)
107-
108-
var (
109-
serverIP = netip.AddrFrom16([16]byte{ipByte1, ipByte2, 15: 0x1})
110-
dnsIP = netip.AddrFrom4([4]byte{1, 1, 1, 1})
111-
clientIP = netip.AddrFrom16([16]byte{ipByte1, ipByte2, 15: 0x2})
112-
)
113-
114115
func newFakeTunnelServer(t *testing.T) *fakeTunnelServer {
115116
priv, err := wgtypes.GeneratePrivateKey()
116117
privBytes := [32]byte(priv)
@@ -123,14 +124,15 @@ func newFakeTunnelServer(t *testing.T) *fakeTunnelServer {
123124
1280,
124125
)
125126
require.NoError(t, err)
126-
dev := device.NewDevice(tun, conn.NewDefaultBind(), device.NewLogger(device.LogLevelVerbose, ""))
127+
dev := device.NewDevice(tun, conn.NewDefaultBind(), device.NewLogger(device.LogLevelVerbose, "server "))
127128
err = dev.IpcSet(fmt.Sprintf(`private_key=%s
128129
listen_port=%d`,
129130
hex.EncodeToString(privBytes[:]),
130131
wgPort,
131132
))
132133
require.NoError(t, err)
133134
t.Cleanup(func() {
135+
dev.RemoveAllPeers()
134136
dev.Close()
135137
})
136138

@@ -193,7 +195,7 @@ allowed_ip=%s/128`,
193195
PublicKey: device.NoisePublicKey(pub),
194196
Tunnel: devtunnel.Node{
195197
HostnameHTTPS: strings.TrimPrefix(f.server.URL, "https://"),
196-
HostnameWireguard: "::1",
198+
HostnameWireguard: "localhost",
197199
WireguardPort: wgPort,
198200
},
199201
HTTPClient: f.server.Client(),

0 commit comments

Comments
 (0)