From 781cd7571f8ba92fe9ab1834b30b457efb338cdc Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Sun, 21 Apr 2019 12:00:31 +0100 Subject: [PATCH] Fix race on tun conns, but still deadlocks if more than one connection is opened --- cmd/yggdrasil/main.go | 1 - src/tuntap/tun.go | 23 ++++++++++++++++++----- src/yggdrasil/conn.go | 2 -- 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/cmd/yggdrasil/main.go b/cmd/yggdrasil/main.go index f3d933c..9634983 100644 --- a/cmd/yggdrasil/main.go +++ b/cmd/yggdrasil/main.go @@ -265,7 +265,6 @@ func main() { // Start the TUN/TAP interface if listener, err := n.core.ConnListen(); err == nil { if dialer, err := n.core.ConnDialer(); err == nil { - logger.Println("Got listener", listener, "and dialer", dialer) n.tuntap.Init(state, logger, listener, dialer) if err := n.tuntap.Start(); err != nil { logger.Errorln("An error occurred starting TUN/TAP:", err) diff --git a/src/tuntap/tun.go b/src/tuntap/tun.go index 9e46124..acdcf3a 100644 --- a/src/tuntap/tun.go +++ b/src/tuntap/tun.go @@ -30,7 +30,6 @@ type TunAdapter struct { config *config.NodeState log *log.Logger reconfigure chan chan error - conns map[crypto.NodeID]*yggdrasil.Conn listener *yggdrasil.Listener dialer *yggdrasil.Dialer addr address.Address @@ -39,6 +38,7 @@ type TunAdapter struct { mtu int iface *water.Interface mutex sync.RWMutex // Protects the below + conns map[crypto.NodeID]*yggdrasil.Conn isOpen bool } @@ -173,13 +173,24 @@ func (tun *TunAdapter) handler() error { tun.log.Errorln("TUN/TAP error accepting connection:", err) return err } - tun.log.Println("Accepted connection from", conn.RemoteAddr()) go tun.connReader(conn) } } func (tun *TunAdapter) connReader(conn *yggdrasil.Conn) error { - tun.conns[conn.RemoteAddr()] = conn + remoteNodeID := conn.RemoteAddr() + tun.mutex.Lock() + if _, isIn := tun.conns[remoteNodeID]; isIn { + tun.mutex.Unlock() + return errors.New("duplicate connection") + } + tun.conns[remoteNodeID] = conn + tun.mutex.Unlock() + defer func() { + tun.mutex.Lock() + delete(tun.conns, remoteNodeID) + tun.mutex.Unlock() + }() b := make([]byte, 65535) for { n, err := conn.Read(b) @@ -242,8 +253,9 @@ func (tun *TunAdapter) ifaceReader() error { } dstNodeID, dstNodeIDMask = dstAddr.GetNodeIDandMask() // Do we have an active connection for this node ID? + tun.mutex.Lock() if conn, isIn := tun.conns[*dstNodeID]; isIn { - tun.log.Println("Got", &conn) + tun.mutex.Unlock() w, err := conn.Write(bs) if err != nil { tun.log.Println("Unable to write to remote:", err) @@ -253,11 +265,12 @@ func (tun *TunAdapter) ifaceReader() error { continue } } else { - tun.log.Println("Opening connection for", *dstNodeID) if conn, err := tun.dialer.DialByNodeIDandMask(dstNodeID, dstNodeIDMask); err == nil { tun.conns[*dstNodeID] = &conn + tun.mutex.Unlock() go tun.connReader(&conn) } else { + tun.mutex.Unlock() tun.log.Println("Error dialing:", err) } } diff --git a/src/yggdrasil/conn.go b/src/yggdrasil/conn.go index daba298..ed9eb6c 100644 --- a/src/yggdrasil/conn.go +++ b/src/yggdrasil/conn.go @@ -2,7 +2,6 @@ package yggdrasil import ( "errors" - "fmt" "sync" "sync/atomic" "time" @@ -124,7 +123,6 @@ func (c *Conn) Write(b []byte) (bytesWritten int, err error) { return 0, errors.New("session is closed") } if c.session == nil { - fmt.Println("No session found, starting search for", &c) c.core.router.doAdmin(func() { c.startSearch() })