e3ac4a0afb4e introduced a lightweight API that can be used instead of UAPI
to reconfigure peers. Peer state created via the new PeerLookupFunc
is not set to expire until the handshake succeeds, making device leak two
goroutines and a set of buffers for each failed handshake.
This change arms the expiry timer before the handshake gets to proceed.
Updates tailscale/tailscale#20183
Change-Id: Ibc0abb6eec97aca0a10f50515dea9e0d6a6a6964
Signed-off-by: Alex Valiushko <alexvaliushko@tailscale.com>
In PR #66, we tried to address a memory leak by avoiding
runtime.SetFinalizer for autodrainingInboundQueue and
autodrainingOutboundQueue unless there was something to do.
However, when there is work to be done, these finalizers still leak
memory because they’re still holding on to a cyclical reference to q.
This applies to any platform that relies on a bounded device.WaitPool,
like Android and iOS which both declare PreallocatedBuffersPerPool.
This patch converts this logic to runtime.AddCleanup which is designed
to avoid this problem.
Updates tailscale/corp#42776
Signed-off-by: Simon Law <sfllaw@tailscale.com>
Device-side portion of upstream tailscale/wireguard-go e3ac4a0
(device, cmd/check-lockorder: add static analysis tool for lock
ordering); the analyzer itself is not carried in this fork.
In tailscale/wireguard-go#65, @lkosewsk reproduced a memory leak seen
in prod with lots of wireguard-go instances being created and
destroyed, where they were still being retained forever due to cycles
in the runtime.SetFinalizer reference graph.
Really we shouldn't be using runtime.SetFinalizer anywhere. But we
still use it on mobile platforms in WaitPool. But those platforms
don't have thousands of tsnet.Server instances coming & going, so this
is a half fix: avoid the finalizer registration on Linux, etc where
the queue doesn't need to be drained and there's no WaitPool
accounting. Just let GC handle it, without adding finalizer cycle
complexity.
Updates tailscale/corp#42776
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Discovered by a tool + test that will come in a future change.
Updates tailscale/tailscale#19513
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
On a connected socket sendmsg_x sends a whole batch in one syscall;
msg_name is unsupported there, so batched sends require connecting to
the peer, which loses roaming and is therefore opt-in via
SetSinglePeerMode for single fixed-peer binds. recvmsg_x fills in
per-message source addresses, so batched receive works on unconnected
sockets too. Any unexpected errno permanently falls back to the generic
paths. iOS is excluded: in the Network Extension recvmsg_x on
unconnected UDP sockets delivers no data, and connected sockets stop
passing traffic after a rebind.
coalesceMessages copied every additional datagram into the spare
capacity of the first buffer, which no longer exists now that element
buffers are sized to their packet; append the datagrams as iovecs
instead, which also removes the copy.
Outbound element buffers now come from the sing allocator sized to the
actual packet instead of the bounded MaxMessageSize pool, element and
container pools become plain sync.Pools, and the bounded message buffer
pool serves only the receive path. Packets injected via
InputPacket/InputPackets are dropped before they are copied once a peer
has 2048 packets queued: injection runs on the caller's read loop, which
must never block on pool exhaustion, and the queues are bounded in
containers, so a flood was buffered instead of dropped.
(cherry picked from commit 7c2acadba17cadf8a1df957c49e1333130d460ad)
(cherry picked from commit a7bac1754e7717e1d4009d1ffd2d13330067d631)
(cherry picked from commit 7a2f11c693b49e784318bbf987173095c67b563d)
Kernels below 5.12 are missing this:
commit 98184612aca0a9ee42b8eb0262a49900ee9eef0d
Author: Norman Maurer <norman_maurer@apple.com>
Date: Thu Apr 1 08:59:17 2021
net: udp: Add support for getsockopt(..., ..., UDP_GRO, ..., ...);
Support for UDP_GRO was added in the past but the implementation for
getsockopt was missed which did lead to an error when we tried to
retrieve the setting for UDP_GRO. This patch adds the missing switch
case for UDP_GRO
Fixes: e20cf8d3f1f7 ("udp: implement GRO for plain UDP sockets.")
Signed-off-by: Norman Maurer <norman_maurer@apple.com>
Reviewed-by: David Ahern <dsahern@kernel.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
That means we can't set the option and then read it back later. Given
how buggy UDP_GRO is in general on odd kernels, just disable it on older
kernels all together.
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
This is already enforced in receive.go, but if these unmarshallers are
to have error return values anyway, make them as explicit as possible.
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
It should be POLLIN because closeFd is read-only file.
Signed-off-by: Kurnia D Win <kurnia.d.win@gmail.com>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
Colin's commit went a step further and protected tun.incomingPacket with
a lock on shutdown, but let's see if the tun.stack.Close() call actually
solves that on its own.
Suggested-by: kshangx <hikeshang@hotmail.com>
Suggested-by: Colin Adler <colin1adler@gmail.com>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
Since 3c75945fd ("netstack: remove PacketBuffer.IsNil()") this has been
invalid. Follow the replacement pattern of that commit.
The old definition inlined to the same code anyway:
func (pk *PacketBuffer) IsNil() bool {
return pk == nil
}
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
The sync.Locker used with a sync.Cond must be acquired when changing
the associated condition, otherwise there is a window within
sync.Cond.Wait() where a wake-up may be missed.
Fixes: 4846070 ("device: use a waiting sync.Pool instead of a channel")
Reviewed-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Signed-off-by: Jordan Whited <jordan@tailscale.com>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
There is a possible deadlock in `device.Close()` when you try to close
the device very soon after its start. The problem is that two different
methods acquire the same locks in different order:
1. device.Close()
- device.ipcMutex.Lock()
- device.state.Lock()
2. device.changeState(deviceState)
- device.state.Lock()
- device.ipcMutex.Lock()
Reproducer:
func TestDevice_deadlock(t *testing.T) {
d := randDevice(t)
d.Close()
}
Problem:
$ go clean -testcache && go test -race -timeout 3s -run TestDevice_deadlock ./device | grep -A 10 sync.runtime_SemacquireMutex
sync.runtime_SemacquireMutex(0xc000117d20?, 0x94?, 0x0?)
/usr/local/opt/go/libexec/src/runtime/sema.go:77 +0x25
sync.(*Mutex).lockSlow(0xc000130518)
/usr/local/opt/go/libexec/src/sync/mutex.go:171 +0x213
sync.(*Mutex).Lock(0xc000130518)
/usr/local/opt/go/libexec/src/sync/mutex.go:90 +0x55
golang.zx2c4.com/wireguard/device.(*Device).Close(0xc000130500)
/Users/martin.basovnik/git/basovnik/wireguard-go/device/device.go:373 +0xb6
golang.zx2c4.com/wireguard/device.TestDevice_deadlock(0x0?)
/Users/martin.basovnik/git/basovnik/wireguard-go/device/device_test.go:480 +0x2c
testing.tRunner(0xc00014c000, 0x131d7b0)
--
sync.runtime_SemacquireMutex(0xc000130564?, 0x60?, 0xc000130548?)
/usr/local/opt/go/libexec/src/runtime/sema.go:77 +0x25
sync.(*Mutex).lockSlow(0xc000130750)
/usr/local/opt/go/libexec/src/sync/mutex.go:171 +0x213
sync.(*Mutex).Lock(0xc000130750)
/usr/local/opt/go/libexec/src/sync/mutex.go:90 +0x55
sync.(*RWMutex).Lock(0xc000130750)
/usr/local/opt/go/libexec/src/sync/rwmutex.go:147 +0x45
golang.zx2c4.com/wireguard/device.(*Device).upLocked(0xc000130500)
/Users/martin.basovnik/git/basovnik/wireguard-go/device/device.go:179 +0x72
golang.zx2c4.com/wireguard/device.(*Device).changeState(0xc000130500, 0x1)
Signed-off-by: Martin Basovnik <martin.basovnik@gmail.com>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
Only bother updating the rxBytes counter once we've processed a whole
vector, since additions are atomic.
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
