From e413919810fbe4da024970aa25b7cb0b4caec6da Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Fri, 5 Jun 2026 15:19:27 +0000 Subject: [PATCH] derp: clone regions when merging DERP maps In-place node shuffling aliased regions shared with the served map. --- hscontrol/derp/derp.go | 8 ++++++-- hscontrol/derp/derp_merge_test.go | 28 ++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) create mode 100644 hscontrol/derp/derp_merge_test.go diff --git a/hscontrol/derp/derp.go b/hscontrol/derp/derp.go index 6f929c4a..3d0b7d5b 100644 --- a/hscontrol/derp/derp.go +++ b/hscontrol/derp/derp.go @@ -6,7 +6,6 @@ import ( "encoding/json" "hash/crc64" "io" - "maps" "math/rand" "net/http" "net/url" @@ -85,7 +84,12 @@ func mergeDERPMaps(derpMaps []*tailcfg.DERPMap) *tailcfg.DERPMap { } for _, derpMap := range derpMaps { - maps.Copy(result.Regions, derpMap.Regions) + // Clone each region: copying the pointer would let a later in-place + // shuffle (shuffleRegionNoClone) alias regions shared with the source + // map or a previously served map, racing concurrent readers. + for id, region := range derpMap.Regions { + result.Regions[id] = region.Clone() + } } for id, region := range result.Regions { diff --git a/hscontrol/derp/derp_merge_test.go b/hscontrol/derp/derp_merge_test.go new file mode 100644 index 00000000..e1ac4120 --- /dev/null +++ b/hscontrol/derp/derp_merge_test.go @@ -0,0 +1,28 @@ +package derp + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "tailscale.com/tailcfg" +) + +// TestMergeDERPMapsClonesRegions ensures merged DERP maps own their regions +// rather than aliasing the source pointers, so a later in-place node shuffle +// cannot mutate a shared or previously served map. +func TestMergeDERPMapsClonesRegions(t *testing.T) { + src := &tailcfg.DERPMap{ + Regions: map[int]*tailcfg.DERPRegion{ + 1: {RegionID: 1, Nodes: []*tailcfg.DERPNode{{Name: "a"}, {Name: "b"}}}, + }, + } + + merged := mergeDERPMaps([]*tailcfg.DERPMap{src}) + + assert.NotSame(t, src.Regions[1], merged.Regions[1], + "merged region must not alias the source region pointer") + + merged.Regions[1].Nodes[0] = &tailcfg.DERPNode{Name: "mutated"} + assert.Equal(t, "a", src.Regions[1].Nodes[0].Name, + "source region was mutated through a shared pointer") +}