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 <[email protected]>
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 <[email protected]>
Suggested-by: Colin Adler <[email protected]>
Signed-off-by: Jason A. Donenfeld <[email protected]>
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 <[email protected]>
Only bother updating the rxBytes counter once we've processed a whole
vector, since additions are atomic.
Signed-off-by: Jason A. Donenfeld <[email protected]>
GRO requires big allocations to be efficient. This isn't great, as there
might be Android memory usage issues. So we should revisit this commit.
But at least it gets things working again.
Signed-off-by: Jason A. Donenfeld <[email protected]>
Otherwise in the event that we're using GSO without sticky sockets, we
pass garbage OOB buffers to sendmmsg, making a EINVAL, when GSO doesn't
set its header.
Signed-off-by: Jason A. Donenfeld <[email protected]>
Otherwise GRO gets enabled on Android, but the conn doesn't use it,
resulting in bundled packets being discarded.
Signed-off-by: Jason A. Donenfeld <[email protected]>
Looks like a simple copy&paste error.
Fixes: 9e2f386 ("conn, device, tun: implement vectorized I/O on Linux")
Signed-off-by: Jason A. Donenfeld <[email protected]>
We can't have the netlink listener socket, so it's not possible to
support it. Plus, android networking stack complexity makes it a bit
tricky anyway, so best to leave it disabled.
Signed-off-by: Jason A. Donenfeld <[email protected]>
Cmsghdr uses uint32 and uint64 on 32-bit and 64-bit respectively for the
Len member, which makes assignments and comparisons slightly more
irksome than usual.
Signed-off-by: Jason A. Donenfeld <[email protected]>
There's not really a use at the moment for making this configurable, and
once bind_windows.go behaves like bind_std.go, we'll be able to use
constants everywhere. So begin that simplification now.
Signed-off-by: Jason A. Donenfeld <[email protected]>
For some reason, this was omitted for response messages.
Reported-by: z <[email protected]>
Fixes: 8c34c4c ("First set of code review patches")
Signed-off-by: Jason A. Donenfeld <[email protected]>
We missed a function exit point. This was exacerbated by e3134bf
("device: defer state machine transitions until configuration is
complete"), but the bug existed prior. Minus provided the following
useful reproducer script:
#!/usr/bin/env bash
set -eux
make wireguard-go || exit 125
ip netns del test-ns || true
ip netns add test-ns
ip link add test-kernel type wireguard
wg set test-kernel listen-port 0 private-key <(echo "QMCfZcp1KU27kEkpcMCgASEjDnDZDYsfMLHPed7+538=") peer "eDPZJMdfnb8ZcA/VSUnLZvLB2k8HVH12ufCGa7Z7rHI=" allowed-ips 10.51.234.10/32
ip link set test-kernel netns test-ns up
ip -n test-ns addr add 10.51.234.1/24 dev test-kernel
port=$(ip netns exec test-ns wg show test-kernel listen-port)
ip link del test-go || true
./wireguard-go test-go
wg set test-go private-key <(echo "WBM7qimR3vFk1QtWNfH+F4ggy/hmO+5hfIHKxxI4nF4=") peer "+nj9Dkqpl4phsHo2dQliGm5aEiWJJgBtYKbh7XjeNjg=" allowed-ips 0.0.0.0/0 endpoint 127.0.0.1:$port
ip addr add 10.51.234.10/24 dev test-go
ip link set test-go up
ping -c2 -W1 10.51.234.1
Reported-by: minus <[email protected]>
Signed-off-by: Jason A. Donenfeld <[email protected]>
There are more places where we'll need to add it later, when Go 1.18
comes out with support for it in the "net" package. Also, allowedips
still uses slices internally, which might be suboptimal.
Signed-off-by: Jason A. Donenfeld <[email protected]>
A peer.endpoint never becomes nil after being not-nil, so creation is
the only time we actually need to set this. This prevents a race from
when the variable is actually used elsewhere, and allows us to avoid an
expensive atomic.
Signed-off-by: Jason A. Donenfeld <[email protected]>
Forgetting to seed the unsafe rng, the jitter before followed a fixed
pattern, which didn't help when a fleet of computers all boot at once.
Signed-off-by: Jason A. Donenfeld <[email protected]>
Otherwise recent WDK binaries fail on ARM64, where an exception handler
is used for trapping an illegal instruction when ARMv8.1 atomics are
being tested for functionality.
Signed-off-by: Jason A. Donenfeld <[email protected]>
The reason this was failing before is that dloadsup.h's
DloadObtainSection was doing a linear search of sections to find which
header corresponds with the IMAGE_DELAYLOAD_DESCRIPTOR section, and we
were stupidly overwriting the VirtualSize field, so the linear search
wound up matching the .text section, which then it found to not be
marked writable and failed with FAST_FAIL_DLOAD_PROTECTION_FAILURE.
Signed-off-by: Jason A. Donenfeld <[email protected]>
Probably a bad idea, but we don't currently support it, and those huge
windows.NewCallback trampolines make juicer targets anyway.
Signed-off-by: Jason A. Donenfeld <[email protected]>
Trying this for every peer winds up being very slow and precludes it
from acceptable runtime in the CI, so reduce this to 4.
Signed-off-by: Jason A. Donenfeld <[email protected]>
Now that we have parent pointers hooked up, we can simply go right to
the node and remove it in place, rather than having to recursively walk
the entire trie.
Signed-off-by: Jason A. Donenfeld <[email protected]>
This makes the insertion algorithm a bit more efficient, while also now
taking on the additional task of connecting up parent pointers. This
will be handy in the following commit.
Signed-off-by: Jason A. Donenfeld <[email protected]>
Heavier network extensions might require the wireguard-go component to
use less ram, so let users of this reduce these as needed.
At some point we'll put this behind a configuration method of sorts, but
for now, just expose the consts as vars.
Requested-by: Josh Bleecher Snyder <[email protected]>
Signed-off-by: Jason A. Donenfeld <[email protected]>
On Linux we can run `ip link del wg0`, in which case the fd becomes
stale, and we should exit. Since this is an intentional action, don't
treat it as an error.
Signed-off-by: Jason A. Donenfeld <[email protected]>
The -1 protection was removed and the wrong error was returned, causing
us to read from a bogus fd. As well, remove the useless closures that
aren't doing anything, since this is all synchronized anyway.
Fixes: 10533c3 ("all: make conn.Bind.Open return a slice of receive functions")
Signed-off-by: Jason A. Donenfeld <[email protected]>
When retrying, if count is not 0, we forget to dequeue another request,
and so the ring fills up and errors out.
Reported-by: Sascha Dierberg <[email protected]>
Signed-off-by: Jason A. Donenfeld <[email protected]>
In 097af6e ("tun: windows: protect reads from closing") we made sure no
functions are running when End() is called, to avoid a UaF. But we still
need to kick that event somehow, so that Read() is allowed to exit, in
order to release the lock. So this commit calls SetEvent, while moving
the closing boolean to be atomic so it can be modified without locks,
and then moves to a WaitGroup for the RCU-like pattern.
Signed-off-by: Jason A. Donenfeld <[email protected]>
The code previously used the old errors channel for checking, rather
than the simpler boolean, which caused issues on shutdown, since the
errors channel was meaningless. However, looking at this exposed a more
basic problem: Close() and all the other functions that check the closed
boolean can race. So protect with a basic RW lock, to ensure that
Close() waits for all pending operations to complete.
Reported-by: Joshua Sjoding <[email protected]>
Signed-off-by: Jason A. Donenfeld <[email protected]>
If we receive a large UDP packet, don't return an error to receive.go,
which then terminates the receive loop. Instead, simply retry.
Considering Winsock's general finickiness, we might consider other
places where an attacker on the wire can generate error conditions like
this.
Reported-by: Sascha Dierberg <[email protected]>
Signed-off-by: Jason A. Donenfeld <[email protected]>
When we pass the TUN FD to the child, we have to call TUNSIFPID;
otherwise when we close the device, we get a splat in dmesg.
Signed-off-by: Jason A. Donenfeld <[email protected]>
Jason A. Donenfeld