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>
This commit is contained in:
David Crawshaw 2020-02-28 22:27:17 -05:00
parent 8aa2090919
commit d417be6a4b
2 changed files with 19 additions and 1 deletions

View File

@ -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

View File

@ -7,6 +7,7 @@
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 {