From 7fcf86a14a13db4aa2f1365f7ccea039c7668dd4 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Mon, 30 Aug 2021 08:37:23 -0700 Subject: [PATCH] wgengine: fix link monitor / magicsock Start race Fixes #2733 Signed-off-by: Brad Fitzpatrick --- wgengine/userspace.go | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/wgengine/userspace.go b/wgengine/userspace.go index bc8b92e66..159e46b9a 100644 --- a/wgengine/userspace.go +++ b/wgengine/userspace.go @@ -84,6 +84,7 @@ type userspaceEngine struct { wgLogger *wglog.Logger //a wireguard-go logging wrapper reqCh chan struct{} waitCh chan struct{} // chan is closed when first Close call completes; contrast with closing bool + magicConnStarted chan struct{} // chan is closed after magicConn.Start timeNow func() mono.Time tundev *tstun.Wrapper wgdev *device.Device @@ -249,13 +250,14 @@ func NewUserspaceEngine(logf logger.Logf, conf Config) (_ Engine, reterr error) closePool.add(tsTUNDev) e := &userspaceEngine{ - timeNow: mono.Now, - logf: logf, - reqCh: make(chan struct{}, 1), - waitCh: make(chan struct{}), - tundev: tsTUNDev, - router: conf.Router, - confListenPort: conf.ListenPort, + timeNow: mono.Now, + logf: logf, + reqCh: make(chan struct{}, 1), + waitCh: make(chan struct{}), + tundev: tsTUNDev, + router: conf.Router, + confListenPort: conf.ListenPort, + magicConnStarted: make(chan struct{}), } e.isLocalAddr.Store(tsaddr.NewContainsIPFunc(nil)) e.isDNSIPOverTailscale.Store(tsaddr.NewContainsIPFunc(nil)) @@ -371,7 +373,7 @@ func NewUserspaceEngine(logf logger.Logf, conf Config) (_ Engine, reterr error) // It's a little pointless to apply no-op settings here (they // should already be empty?), but it at least exercises the - // router implementation early on the machine. + // router implementation early on. e.logf("Clearing router settings...") if err := e.router.Set(nil); err != nil { return nil, err @@ -380,6 +382,7 @@ func NewUserspaceEngine(logf logger.Logf, conf Config) (_ Engine, reterr error) e.linkMon.Start() e.logf("Starting magicsock...") e.magicConn.Start() + close(e.magicConnStarted) go e.pollResolver() @@ -1092,6 +1095,10 @@ func (e *userspaceEngine) LinkChange(_ bool) { } func (e *userspaceEngine) linkChange(changed bool, cur *interfaces.State) { + // Issue 2733: wait for e.magicConn to be started; there's two tiny + // windows at startup where this callback can be run before Start + <-e.magicConnStarted + up := cur.AnyInterfaceUp() if !up { e.logf("LinkChange: all links down; pausing: %v", cur)