diff --git a/wgengine/magicsock/magicsock.go b/wgengine/magicsock/magicsock.go index 6c8114876..bd80e67ee 100644 --- a/wgengine/magicsock/magicsock.go +++ b/wgengine/magicsock/magicsock.go @@ -125,7 +125,7 @@ type Conn struct { packetListener nettype.PacketListener // ============================================================ - mu sync.Mutex // guards all following fields + mu sync.Mutex // guards all following fields; see userspaceEngine lock ordering rules muCond *sync.Cond // canCreateEPUnlocked tracks at one place whether mu is diff --git a/wgengine/userspace.go b/wgengine/userspace.go index c7bef0ad7..c10bb04ad 100644 --- a/wgengine/userspace.go +++ b/wgengine/userspace.go @@ -116,7 +116,7 @@ type userspaceEngine struct { pingers map[wgcfg.Key]*pinger // legacy pingers for pre-discovery peers linkState *interfaces.State - // Lock ordering: wgLock, then mu. + // Lock ordering: magicsock.Conn.mu, wgLock, then mu. } // RouterGen is the signature for a function that creates a @@ -883,6 +883,11 @@ func (e *userspaceEngine) getStatusCallback() StatusCallback { // TODO: this function returns an error but it's always nil, and when // there's actually a problem it just calls log.Fatal. Why? func (e *userspaceEngine) getStatus() (*Status, error) { + // Grab derpConns before acquiring wgLock to not violate lock ordering; + // the DERPs method acquires magicsock.Conn.mu. + // (See comment in userspaceEngine's declaration.) + derpConns := e.magicConn.DERPs() + e.wgLock.Lock() defer e.wgLock.Unlock() @@ -998,7 +1003,7 @@ func (e *userspaceEngine) getStatus() (*Status, error) { return &Status{ LocalAddrs: append([]string(nil), e.endpoints...), Peers: peers, - DERPs: e.magicConn.DERPs(), + DERPs: derpConns, }, nil }