From d417be6a4bfc4e6d3097827fdebad7c8644a47ea Mon Sep 17 00:00:00 2001
From: David Crawshaw <crawshaw@tailscale.com>
Date: Fri, 28 Feb 2020 22:27:17 -0500
Subject: [PATCH] controlclinet: clone filter.MatchAllowAll

This avoids a non-obvious data race, where the JSON decoder ends
up creating do-nothing writes into global variables.

	==================
	WARNING: DATA RACE
	Write at 0x0000011e1860 by goroutine 201:
	  tailscale.com/wgengine/packet.(*IP).UnmarshalJSON()
	      /home/crawshaw/repo/corp/oss/wgengine/packet/packet.go:83 +0x2d9
	  encoding/json.(*decodeState).literalStore()
	      /home/crawshaw/go/go/src/encoding/json/decode.go:877 +0x445e
	...
	  encoding/json.Unmarshal()
	      /home/crawshaw/go/go/src/encoding/json/decode.go:107 +0x1de
	  tailscale.com/control/controlclient.(*Direct).decodeMsg()
	      /home/crawshaw/repo/corp/oss/control/controlclient/direct.go:615 +0x1ab
	  tailscale.com/control/controlclient.(*Direct).PollNetMap()
	      /home/crawshaw/repo/corp/oss/control/controlclient/direct.go:525 +0x1053
	  tailscale.com/control/controlclient.(*Client).mapRoutine()
	      /home/crawshaw/repo/corp/oss/control/controlclient/auto.go:428 +0x3a6
	Previous read at 0x0000011e1860 by goroutine 86:
	  tailscale.com/wgengine/filter.matchIPWithoutPorts()
	      /home/crawshaw/repo/corp/oss/wgengine/filter/match.go:108 +0x91
	  tailscale.com/wgengine/filter.(*Filter).runIn()
	      /home/crawshaw/repo/corp/oss/wgengine/filter/filter.go:147 +0x3c6
	  tailscale.com/wgengine/filter.(*Filter).RunIn()
	      /home/crawshaw/repo/corp/oss/wgengine/filter/filter.go:127 +0xb0
	  tailscale.com/wgengine.(*userspaceEngine).SetFilter.func1()
	      /home/crawshaw/repo/corp/oss/wgengine/userspace.go:390 +0xfc
	  github.com/tailscale/wireguard-go/device.(*Device).RoutineDecryption()
	      /home/crawshaw/repo/corp/wireguard-go/device/receive.go:295 +0xa1f

For #112

Signed-off-by: David Crawshaw <crawshaw@tailscale.com>
---
 control/controlclient/direct.go |  2 +-
 wgengine/filter/match.go        | 18 ++++++++++++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/control/controlclient/direct.go b/control/controlclient/direct.go
index d7aa80ecf..85b196a23 100644
--- a/control/controlclient/direct.go
+++ b/control/controlclient/direct.go
@@ -520,7 +520,7 @@ func (c *Direct) PollNetMap(ctx context.Context, maxPolls int, cb func(*NetworkM
 		// support). If even an empty PacketFilter is provided, this
 		// will be overwritten.
 		// TODO(apenwarr 2020-02-01): remove after tailcontrol is fully deployed.
-		resp.PacketFilter = filter.MatchAllowAll
+		resp.PacketFilter = filter.MatchAllowAll.Clone()
 
 		if err := c.decodeMsg(msg, &resp); err != nil {
 			return err
diff --git a/wgengine/filter/match.go b/wgengine/filter/match.go
index 27e0b8f98..2f89ab924 100644
--- a/wgengine/filter/match.go
+++ b/wgengine/filter/match.go
@@ -7,6 +7,7 @@ package filter
 import (
 	"fmt"
 	"strings"
+
 	"tailscale.com/wgengine/packet"
 )
 
@@ -48,6 +49,16 @@ type Match struct {
 	SrcIPs   []IP
 }
 
+func (m Match) Clone() (res Match) {
+	if m.DstPorts != nil {
+		res.DstPorts = append([]IPPortRange{}, m.DstPorts...)
+	}
+	if m.SrcIPs != nil {
+		res.SrcIPs = append([]IP{}, m.SrcIPs...)
+	}
+	return res
+}
+
 func (m Match) String() string {
 	srcs := []string{}
 	for _, srcip := range m.SrcIPs {
@@ -74,6 +85,13 @@ func (m Match) String() string {
 
 type Matches []Match
 
+func (m Matches) Clone() (res Matches) {
+	for _, match := range m {
+		res = append(res, match.Clone())
+	}
+	return res
+}
+
 func ipInList(ip IP, iplist []IP) bool {
 	for _, ipp := range iplist {
 		if ipp == IPAny || ipp == ip {