From 28db566b37b14224ffbf1505b89d8eab2a0d0f32 Mon Sep 17 00:00:00 2001 From: Arceliar Date: Sat, 29 Jun 2019 18:44:24 -0500 Subject: [PATCH 1/3] fix concurrency bug in iface.go --- src/tuntap/iface.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tuntap/iface.go b/src/tuntap/iface.go index f6cfec9c..16a3b65d 100644 --- a/src/tuntap/iface.go +++ b/src/tuntap/iface.go @@ -225,11 +225,11 @@ func (tun *TunAdapter) reader() error { panic("Given empty dstNodeID and dstNodeIDMask - this shouldn't happen") } // Dial to the remote node + packet := append(util.GetBytes(), bs[:n]...) go func() { // FIXME just spitting out a goroutine to do this is kind of ugly and means we drop packets until the dial finishes tun.mutex.Lock() _, known := tun.dials[*dstNodeID] - packet := append(util.GetBytes(), bs[:n]...) tun.dials[*dstNodeID] = append(tun.dials[*dstNodeID], packet) for len(tun.dials[*dstNodeID]) > 32 { util.PutBytes(tun.dials[*dstNodeID][0]) From d39428735df66a39acef3da98c2ca3eac5237a15 Mon Sep 17 00:00:00 2001 From: Arceliar Date: Sat, 29 Jun 2019 18:50:21 -0500 Subject: [PATCH 2/3] recover if we try to send to a closed session worker due to a race between a Conn.Write call and a Conn.Close call --- src/yggdrasil/conn.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/yggdrasil/conn.go b/src/yggdrasil/conn.go index 38e4df9f..b4f68e18 100644 --- a/src/yggdrasil/conn.go +++ b/src/yggdrasil/conn.go @@ -236,6 +236,11 @@ func (c *Conn) Write(b []byte) (bytesWritten int, err error) { timer := getDeadlineTimer(&c.writeDeadline) defer util.TimerStop(timer) // Hand over to the session worker + defer func() { + if recover() != nil { + err = errors.New("write failed") + } + }() // In case we're racing with a close select { // Send to worker case sinfo.worker <- workerFunc: case <-timer.C: From 40553a6a44e88c3df8e164f7b2386d92c3bc93cc Mon Sep 17 00:00:00 2001 From: Arceliar Date: Sat, 29 Jun 2019 18:56:26 -0500 Subject: [PATCH 3/3] make GetSessions use the session workers to avoid races --- src/yggdrasil/api.go | 33 ++++++++++++++++++++++++--------- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/src/yggdrasil/api.go b/src/yggdrasil/api.go index 864e7f37..7270bc2e 100644 --- a/src/yggdrasil/api.go +++ b/src/yggdrasil/api.go @@ -211,16 +211,31 @@ func (c *Core) GetSessions() []Session { var sessions []Session getSessions := func() { for _, sinfo := range c.sessions.sinfos { - // TODO? skipped known but timed out sessions? - session := Session{ - Coords: append([]byte{}, sinfo.coords...), - MTU: sinfo.getMTU(), - BytesSent: sinfo.bytesSent, - BytesRecvd: sinfo.bytesRecvd, - Uptime: time.Now().Sub(sinfo.timeOpened), - WasMTUFixed: sinfo.wasMTUFixed, + var session Session + workerFunc := func() { + session := Session{ + Coords: append([]byte{}, sinfo.coords...), + MTU: sinfo.getMTU(), + BytesSent: sinfo.bytesSent, + BytesRecvd: sinfo.bytesRecvd, + Uptime: time.Now().Sub(sinfo.timeOpened), + WasMTUFixed: sinfo.wasMTUFixed, + } + copy(session.PublicKey[:], sinfo.theirPermPub[:]) } - copy(session.PublicKey[:], sinfo.theirPermPub[:]) + var skip bool + func() { + defer func() { + if recover() != nil { + skip = true + } + }() + sinfo.doWorker(workerFunc) + }() + if skip { + continue + } + // TODO? skipped known but timed out sessions? sessions = append(sessions, session) } }