* wireguard-go data race in Peer.SendBuffer()
@ 2019-11-20 19:58 Ben Burkert
2019-11-27 12:40 ` Jason A. Donenfeld
0 siblings, 1 reply; 4+ messages in thread
From: Ben Burkert @ 2019-11-20 19:58 UTC (permalink / raw)
To: wireguard
Greetings,
The Go race detector is reporting a data race in my application due to
what appears to be concurrent calls to Peer.SendHandshakeInitiation().
It seems that it is unsafe to call golang.org/x/sys/unix.SendmsgN()
concurrently due to the sockaddr() implementation which mutates
memory: https://github.com/golang/sys/blob/bd437916bb0eb726b873ee8e9b2dcf212d32e2fd/unix/syscall_aix.go#L53-L55
I believe this data race can be fixed by switching to an exclusive
lock in the Peer.SendBuffer() method:
https://github.com/WireGuard/wireguard-go/blob/f2ea85e9f9921bc29a7dcb2007212b153540c01a/device/peer.go#L144-L145
If it helps I can mail a patch to this effect. I've copied the output
of the race detector below.
Best,
-Ben
==================
==================
WARNING: DATA RACE
Write at 0x00c0011f2740 by goroutine 27:
golang.org/x/sys/unix.(*SockaddrInet4).sockaddr()
/go/pkg/mod/golang.org/x/sys@v0.0.0-20191105231009-c1f44814a5cd/unix/syscall_linux.go:384
+0x114
golang.org/x/sys/unix.SendmsgN()
/go/pkg/mod/golang.org/x/sys@v0.0.0-20191105231009-c1f44814a5cd/unix/syscall_linux.go:1304
+0x288
golang.zx2c4.com/wireguard/device.send4()
/go/pkg/mod/golang.zx2c4.com/wireguard@v0.0.20191012/device/conn_linux.go:485
+0x11f
golang.zx2c4.com/wireguard/device.(*nativeBind).Send()
/go/pkg/mod/golang.zx2c4.com/wireguard@v0.0.20191012/device/conn_linux.go:268
+0x1d6
golang.zx2c4.com/wireguard/device.(*Peer).SendBuffer()
/go/pkg/mod/golang.zx2c4.com/wireguard@v0.0.20191012/device/peer.go:151
+0x285
golang.zx2c4.com/wireguard/device.(*Peer).SendHandshakeInitiation()
/go/pkg/mod/golang.zx2c4.com/wireguard@v0.0.20191012/device/send.go:163
+0x692
golang.zx2c4.com/wireguard/device.(*Device).RoutineReadFromTUN()
/go/pkg/mod/golang.zx2c4.com/wireguard@v0.0.20191012/device/send.go:318
+0x4b8
Previous write at 0x00c0011f2740 by goroutine 386:
golang.org/x/sys/unix.(*SockaddrInet4).sockaddr()
/go/pkg/mod/golang.org/x/sys@v0.0.0-20191105231009-c1f44814a5cd/unix/syscall_linux.go:384
+0x114
golang.org/x/sys/unix.SendmsgN()
/go/pkg/mod/golang.org/x/sys@v0.0.0-20191105231009-c1f44814a5cd/unix/syscall_linux.go:1304
+0x288
golang.zx2c4.com/wireguard/device.send4()
/go/pkg/mod/golang.zx2c4.com/wireguard@v0.0.20191012/device/conn_linux.go:485
+0x11f
golang.zx2c4.com/wireguard/device.(*nativeBind).Send()
/go/pkg/mod/golang.zx2c4.com/wireguard@v0.0.20191012/device/conn_linux.go:268
+0x1d6
golang.zx2c4.com/wireguard/device.(*Peer).SendBuffer()
/go/pkg/mod/golang.zx2c4.com/wireguard@v0.0.20191012/device/peer.go:151
+0x285
golang.zx2c4.com/wireguard/device.(*Peer).SendHandshakeInitiation()
/go/pkg/mod/golang.zx2c4.com/wireguard@v0.0.20191012/device/send.go:163
+0x692
golang.zx2c4.com/wireguard/device.expiredRetransmitHandshake()
/go/pkg/mod/golang.zx2c4.com/wireguard@v0.0.20191012/device/timers.go:110
+0x40c
golang.zx2c4.com/wireguard/device.(*Peer).NewTimer.func1()
/go/pkg/mod/golang.zx2c4.com/wireguard@v0.0.20191012/device/timers.go:42
+0xd8
Goroutine 27 (running) created at:
golang.zx2c4.com/wireguard/device.NewDevice()
/go/pkg/mod/golang.zx2c4.com/wireguard@v0.0.20191012/device/device.go:322
+0x5e8
main.main()
/go/src/x/main.go:102 +0x58e
Goroutine 386 (finished) created at:
time.goFunc()
/usr/local/go/src/time/sleep.go:168 +0x51
==================
_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: wireguard-go data race in Peer.SendBuffer()
2019-11-20 19:58 wireguard-go data race in Peer.SendBuffer() Ben Burkert
@ 2019-11-27 12:40 ` Jason A. Donenfeld
2019-11-27 17:47 ` Ben Burkert
0 siblings, 1 reply; 4+ messages in thread
From: Jason A. Donenfeld @ 2019-11-27 12:40 UTC (permalink / raw)
To: Ben Burkert; +Cc: WireGuard mailing list
Thanks for the report. Does this fix it for you?
https://git.zx2c4.com/wireguard-go/commit/?id=cd26ba8ee4a523437aba3b489422127f1293a16f
_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: wireguard-go data race in Peer.SendBuffer()
2019-11-27 12:40 ` Jason A. Donenfeld
@ 2019-11-27 17:47 ` Ben Burkert
2019-11-28 10:09 ` Jason A. Donenfeld
0 siblings, 1 reply; 4+ messages in thread
From: Ben Burkert @ 2019-11-27 17:47 UTC (permalink / raw)
To: Jason A. Donenfeld; +Cc: WireGuard mailing list
Thanks, unfortunately I'm getting a build error now. Looks like the
patch needs to switch "sync.Lock" to "sync.Mutex". I'm testing this
change and haven't hit any race conditions yet, I'll let you know if I
see any more issues.
On Wed, Nov 27, 2019 at 4:40 AM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> Thanks for the report. Does this fix it for you?
>
> https://git.zx2c4.com/wireguard-go/commit/?id=cd26ba8ee4a523437aba3b489422127f1293a16f
_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: wireguard-go data race in Peer.SendBuffer()
2019-11-27 17:47 ` Ben Burkert
@ 2019-11-28 10:09 ` Jason A. Donenfeld
0 siblings, 0 replies; 4+ messages in thread
From: Jason A. Donenfeld @ 2019-11-28 10:09 UTC (permalink / raw)
To: Ben Burkert; +Cc: WireGuard mailing list
How on earth did that get committed... Sorry. Fixed.
_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-11-28 10:09 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-20 19:58 wireguard-go data race in Peer.SendBuffer() Ben Burkert
2019-11-27 12:40 ` Jason A. Donenfeld
2019-11-27 17:47 ` Ben Burkert
2019-11-28 10:09 ` Jason A. Donenfeld
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.