wgengine/magicsock: fix data race from undocumented wireguard-go requirement

Endpoints need to be Stringers apparently.

Fixes tailscale/corp#422
reviewable/pr526/r1
Brad Fitzpatrick 4 years ago
parent 9fbe8d7cf2
commit 5132edacf7

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

@ -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()
}

Loading…
Cancel
Save