device: make RoutineReadFromTUN keep encryption queue alive

RoutineReadFromTUN can trigger a call to SendStagedPackets.
SendStagedPackets attempts to protect against sending
on the encryption queue by checking peer.isRunning and device.isClosed.
However, those are subject to TOCTOU bugs.

If that happens, we get this:

goroutine 1254 [running]:
golang.zx2c4.com/wireguard/device.(*Peer).SendStagedPackets(0xc000798300)
        .../wireguard-go/device/send.go:321 +0x125
golang.zx2c4.com/wireguard/device.(*Device).RoutineReadFromTUN(0xc000014780)
        .../wireguard-go/device/send.go:271 +0x21c
created by golang.zx2c4.com/wireguard/device.NewDevice
        .../wireguard-go/device/device.go:315 +0x298

Fix this with a simple, big hammer: Keep the encryption queue
alive as long as it might be written to.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>

diff --git a/device/device.go b/device/device.go
index 586715e6190fd0251a95f0614f9f112d2ddf0fb8..9375448d6c1474c6d747065f29363f88457a117c 100644 (file)
--- a/device/device.go
+++ b/device/device.go
@@ -311,7 +311,8 @@ func NewDevice(tunDevice tun.Device, logger *Logger) *Device {
                go device.RoutineHandshake()
        }
 
-       device.state.stopping.Add(1) // read from TUN
+       device.state.stopping.Add(1)      // RoutineReadFromTUN
+       device.queue.encryption.wg.Add(1) // RoutineReadFromTUN
        go device.RoutineReadFromTUN()
        go device.RoutineTUNEventReader()
 

diff --git a/device/send.go b/device/send.go
index 783e5b9100c6faf3bc6fcd3dfa6ce07a4125bc5c..6a3b30b9b5c0661616661643d3aa623cdd0fe4e0 100644 (file)
--- a/device/send.go
+++ b/device/send.go
@@ -206,6 +206,7 @@ func (device *Device) RoutineReadFromTUN() {
        defer func() {
                device.log.Verbosef("Routine: TUN reader - stopped")
                device.state.stopping.Done()
+               device.queue.encryption.wg.Done()
        }()
 
        device.log.Verbosef("Routine: TUN reader - started")