diff --git a/wgengine/magicsock/magicsock.go b/wgengine/magicsock/magicsock.go index 0655a36d0..4c08f144b 100644 --- a/wgengine/magicsock/magicsock.go +++ b/wgengine/magicsock/magicsock.go @@ -2750,6 +2750,13 @@ func (de *discoEndpoint) initFakeUDPAddr() { de.fakeWGAddrStd = de.fakeWGAddr.UDPAddr() } +// String exists purely so wireguard-go internals can log.Printf("%v") +// its internal conn.Endpoints and we don't end up with data races +// from fmt (via log) reading mutex fields and such. +func (de *discoEndpoint) String() string { + return fmt.Sprintf("magicsock.discoEndpoint{%v, %v}", de.publicKey.ShortString(), de.discoShort) +} + func (de *discoEndpoint) Addrs() []wgcfg.Endpoint { // This has to be the same string that was passed to // CreateEndpoint, otherwise Reconfig will end up recreating diff --git a/wgengine/magicsock/magicsock_test.go b/wgengine/magicsock/magicsock_test.go index f95064196..b5e7d5c55 100644 --- a/wgengine/magicsock/magicsock_test.go +++ b/wgengine/magicsock/magicsock_test.go @@ -10,6 +10,7 @@ import ( "crypto/tls" "encoding/binary" "fmt" + "io/ioutil" "net" "net/http" "net/http/httptest" @@ -880,3 +881,22 @@ func TestDiscoMessage(t *testing.T) { t.Error("failed to open it") } } + +// tests that having a discoEndpoint.String prevents wireguard-go's +// log.Printf("%v") of its conn.Endpoint values from using reflect to +// walk into read mutex while they're being used and then causing data +// races. +func TestDiscoStringLogRace(t *testing.T) { + de := new(discoEndpoint) + var wg sync.WaitGroup + wg.Add(2) + go func() { + defer wg.Done() + fmt.Fprintf(ioutil.Discard, "%v", de) + }() + go func() { + defer wg.Done() + de.mu.Lock() + }() + wg.Wait() +}