From 4eab21a7b7192dc806a3d60c46827866fc8942fe Mon Sep 17 00:00:00 2001 From: Josh Bleecher Snyder Date: Tue, 9 Feb 2021 09:53:00 -0800 Subject: [PATCH] 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 --- device/device.go | 3 ++- device/send.go | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/device/device.go b/device/device.go index 586715e..9375448 100644 --- 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 783e5b9..6a3b30b 100644 --- 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")