aboutsummaryrefslogtreecommitdiffstats
path: root/device (follow)
Commit message (Collapse)AuthorAgeFilesLines
...
* device: log all errors received by RoutineReceiveIncomingJosh Bleecher Snyder2021-05-061-1/+1
| | | | | | | | | | | When debugging, it's useful to know why a receive func exited. We were already logging that, but only in the "death spiral" case. Move the logging up, to capture it always. Reduce the verbosity, since it is not an error case any more. Put the receive func name in the log line. Signed-off-by: Josh Bleecher Snyder <josharian@gmail.com>
* device: don't defer unlocking from loopJason A. Donenfeld2021-04-121-1/+1
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* conn: reconstruct v4 vs v6 receive function based on symtabJason A. Donenfeld2021-04-121-2/+3
| | | | | | This is kind of gross but it's better than the alternatives. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* device: allocate new buffer in receive death spiralKristupas Antanavičius2021-04-121-0/+1
| | | | | | | | | | | | | | | | | | | | | | | | Note: this bug is "hidden" by avoiding "death spiral" code path by 6228659 ("device: handle broader range of errors in RoutineReceiveIncoming"). If the code reached "death spiral" mechanism, there would be multiple double frees happening. This results in a deadlock on iOS, because the pools are fixed size and goroutine might stop until somebody makes space in the pool. This was almost 100% repro on the new ARM Macbooks: - Build with 'ios' tag for Mac. This will enable bounded pools. - Somehow call device.IpcSet at least couple of times (update config) - device.BindUpdate() would be triggered - RoutineReceiveIncoming would enter "death spiral". - RoutineReceiveIncoming would stall on double free (pool is already full) - The stuck routine would deadlock 'device.closeBindLocked()' function on line 'netc.stopping.Wait()' Signed-off-by: Kristupas Antanavičius <kristupas.antanavicius@nordsec.com> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* all: make conn.Bind.Open return a slice of receive functionsJosh Bleecher Snyder2021-04-022-20/+12
| | | | | | | | | | | | | | | | | Instead of hard-coding exactly two sources from which to receive packets (an IPv4 source and an IPv6 source), allow the conn.Bind to specify a set of sources. Beneficial consequences: * If there's no IPv6 support on a system, conn.Bind.Open can choose not to return a receive function for it, which is simpler than tracking that state in the bind. This simplification removes existing data races from both conn.StdNetBind and bindtest.ChannelBind. * If there are more than two sources on a system, the conn.Bind no longer needs to add a separate muxing layer. Signed-off-by: Josh Bleecher Snyder <josharian@gmail.com>
* device: handle broader range of errors in RoutineReceiveIncomingJosh Bleecher Snyder2021-03-301-0/+3
| | | | | | | | | | | | | RoutineReceiveIncoming exits immediately on net.ErrClosed, but not on other errors. However, for errors that are known to be permanent, such as syscall.EAFNOSUPPORT, we may as well exit immediately instead of retrying. This considerably speeds up the package device tests right now, because the Bind sometimes (incorrectly) returns syscall.EAFNOSUPPORT instead of net.ErrClosed. Signed-off-by: Josh Bleecher Snyder <josharian@gmail.com>
* device: rename unsafeCloseBind to closeBindLockedJosh Bleecher Snyder2021-03-301-3/+5
| | | | | | | And document a bit. This name is more idiomatic. Signed-off-by: Josh Bleecher Snyder <josharian@gmail.com>
* device: signal to close device in separate routineJason A. Donenfeld2021-03-111-1/+1
| | | | | | Otherwise we wind up deadlocking. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* device: get rid of peers.empty boolean in timersActiveJason A. Donenfeld2021-03-063-10/+7
| | | | | | | | | | There's no way for len(peers)==0 when a current peer has isRunning==false. This requires some struct reshuffling so that the uint64 pointer is aligned. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* conn: implement RIO for fast Windows UDP socketsJason A. Donenfeld2021-02-252-1/+16
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* global: remove TODO name graffitiJason A. Donenfeld2021-02-232-2/+1
| | | | | | | | | | Googlers have a habit of graffiting their name in TODO items that then are never addressed, and other people won't go near those because they're marked territory of another animal. I've been gradually cleaning these up as I see them, but this commit just goes all the way and removes the remaining stragglers. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* device: test up/down using virtual connJason A. Donenfeld2021-02-231-23/+19
| | | | | | This prevents port clashing bugs. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* device: cleanup unused test componentsJason A. Donenfeld2021-02-233-80/+27
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* conn: make binds replacableJason A. Donenfeld2021-02-236-28/+19
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* device: disable waitpool testsJason A. Donenfeld2021-02-221-0/+1
| | | | | | | This code is stable, and the test is finicky, especially on high core count systems, so just disable it. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* global: stop using ioutilJason A. Donenfeld2021-02-171-2/+2
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* conn: bump to 1.16 and get rid of NetErrClosed hackJason A. Donenfeld2021-02-161-1/+1
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* device: remove old version fileJason A. Donenfeld2021-02-121-3/+0
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* device: use container/list instead of open coding itJason A. Donenfeld2021-02-102-37/+25
| | | | | | | This linked list implementation is awful, but maybe Go 2 will help eventually, and at least we're not open coding the hlist any more. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* device: retry Up() in up/down testJason A. Donenfeld2021-02-101-2/+13
| | | | | | | | We're loosing our ownership of the port when bringing the device down, which means another test process could reclaim it. Avoid this by retrying for 4 seconds. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* device: flush peer queues before starting deviceJason A. Donenfeld2021-02-102-24/+30
| | | | | | | In case some old packets snuck in there before, this flushes before starting afresh. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* device: create peer queues at peer creation timeJason A. Donenfeld2021-02-101-6/+3
| | | | | | | Rather than racing with Start(), since we're never destroying these queues, we just set the variables at creation time. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* device: return error from Up() and Down()Jason A. Donenfeld2021-02-102-18/+27
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* rwcancel: add an explicit close callJason A. Donenfeld2021-02-091-0/+1
| | | | | | This lets us collect FDs even if the GC doesn't do it for us. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* device: handshake routine writes into encryption queueJason A. Donenfeld2021-02-092-1/+5
| | | | | | | Since RoutineHandshake calls peer.SendKeepalive(), it potentially is a writer into the encryption queue, so we need to bump the wg count. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* device: make RoutineReadFromTUN keep encryption queue aliveJosh Bleecher Snyder2021-02-092-1/+3
| | | | | | | | | | | | | | | | | | | | | | RoutineReadFromTUN can trigger a call to SendStagedPackets. SendStagedPackets attempts to protect against sending on the encryption queue by checking peer.isRunning and device.isClosed. However, those are subject to TOCTOU bugs. If that happens, we get this: goroutine 1254 [running]: golang.zx2c4.com/wireguard/device.(*Peer).SendStagedPackets(0xc000798300) .../wireguard-go/device/send.go:321 +0x125 golang.zx2c4.com/wireguard/device.(*Device).RoutineReadFromTUN(0xc000014780) .../wireguard-go/device/send.go:271 +0x21c created by golang.zx2c4.com/wireguard/device.NewDevice .../wireguard-go/device/device.go:315 +0x298 Fix this with a simple, big hammer: Keep the encryption queue alive as long as it might be written to. Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
* device: only allocate peer queues onceJosh Bleecher Snyder2021-02-091-4/+4
| | | | | | | | | This serves two purposes. First, it makes repeatedly stopping then starting a peer cheaper. Second, it prevents a data race observed accessing the queues. Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
* device: clarify device.state.state docs (again)Josh Bleecher Snyder2021-02-091-2/+4
| | | | Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
* device: run fewer iterations in TestUpDownJosh Bleecher Snyder2021-02-091-2/+2
| | | | | | | | | | The high iteration count was useful when TestUpDown was the nexus of new bugs to investigate. Now that it has stabilized, that's less valuable. And it slows down running the tests and crowds out other tests. Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
* device: run fewer trials in TestWaitPool when race detector enabledJosh Bleecher Snyder2021-02-093-0/+24
| | | | | | | On a many-core machine with the race detector enabled, this test can take several minutes to complete. Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
* device: remove nil elem check in finalizersJosh Bleecher Snyder2021-02-091-6/+0
| | | | | | This is not necessary, and removing it speeds up detection of UAF bugs. Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
* device: rename unsafeRemovePeer to removePeerLockedJason A. Donenfeld2021-02-091-9/+5
| | | | | | This matches the new naming scheme of upLocked and downLocked. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* device: remove deviceStateNewJason A. Donenfeld2021-02-092-20/+8
| | | | | | | It's never used and we won't have a use for it. Also, move to go-running stringer, for those without GOPATHs. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* device: fix comment typo and shorten state.mu.Lock to state.LockJason A. Donenfeld2021-02-092-13/+12
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* device: fix typo in commentJason A. Donenfeld2021-02-091-1/+1
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* device: fix alignment on 32-bit machines and test for itJason A. Donenfeld2021-02-092-8/+2
| | | | | | | | | The test previously checked the offset within a substruct, not the offset within the allocated struct, so this adds the two together. It then fixes an alignment crash on 32-bit machines. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* device: do not log on idempotent device state changeJason A. Donenfeld2021-02-091-1/+0
| | | | | | | Part of being actually idempotent is that we shouldn't penalize code that takes advantage of this property with a log splat. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* device: do not attach finalizer to non-returned objectJason A. Donenfeld2021-02-095-20/+22
| | | | | | | | Before, the code attached a finalizer to an object that wasn't returned, resulting in immediate garbage collection. Instead return the actual pointer. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* device: lock elem in autodraining queue before freeingJason A. Donenfeld2021-02-091-0/+2
| | | | | | | Without this, we wind up freeing packets that the encryption/decryption queues still have, resulting in a UaF. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* device: remove listen port race in testsJason A. Donenfeld2021-02-091-63/+43
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* device: generate test keys on the flyJason A. Donenfeld2021-02-091-6/+21
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* device: remove mutex from Peer send/receiveJosh Bleecher Snyder2021-02-084-16/+80
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The immediate motivation for this change is an observed deadlock. 1. A goroutine calls peer.Stop. That calls peer.queue.Lock(). 2. Another goroutine is in RoutineSequentialReceiver. It receives an elem from peer.queue.inbound. 3. The peer.Stop goroutine calls close(peer.queue.inbound), close(peer.queue.outbound), and peer.stopping.Wait(). It blocks waiting for RoutineSequentialReceiver and RoutineSequentialSender to exit. 4. The RoutineSequentialReceiver goroutine calls peer.SendStagedPackets(). SendStagedPackets attempts peer.queue.RLock(). That blocks forever because the peer.Stop goroutine holds a write lock on that mutex. A background motivation for this change is that it can be expensive to have a mutex in the hot code path of RoutineSequential*. The mutex was necessary to avoid attempting to send elems on a closed channel. This commit removes that danger by never closing the channel. Instead, we send a sentinel nil value on the channel to indicate to the receiver that it should exit. The only problem with this is that if the receiver exits, we could write an elem into the channel which would never get received. If it never gets received, it cannot get returned to the device pools. To work around this, we use a finalizer. When the channel can be GC'd, the finalizer drains any remaining elements from the channel and restores them to the device pool. After that change, peer.queue.RWMutex no longer makes sense where it is. It is only used to prevent concurrent calls to Start and Stop. Move it to a more sensible location and make it a plain sync.Mutex. Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
* device: create channels.goJosh Bleecher Snyder2021-02-082-61/+69
| | | | | | | We have a bunch of stupid channel tricks, and I'm about to add more. Give them their own file. This commit is 100% code movement. Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
* device: print direction when ping transit failsJosh Bleecher Snyder2021-02-081-3/+9
| | | | Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
* device: separate timersInit from timersStartJosh Bleecher Snyder2021-02-082-5/+7
| | | | | | | | | | | | | | | timersInit sets up the timers. It need only be done once per peer. timersStart does the work to prepare the timers for a newly running peer. It needs to be done every time a peer starts. Separate the two and call them in the appropriate places. This prevents data races on the peer's timers fields when starting and stopping peers. Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
* device: don't track device interface state in RoutineTUNEventReaderJosh Bleecher Snyder2021-02-081-7/+4
| | | | | | | We already track this state elsewhere. No need to duplicate. The cost of calling changeState is negligible. Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
* device: improve MTU change handlingJosh Bleecher Snyder2021-02-081-8/+15
| | | | | | | | | | | The old code silently accepted negative MTUs. It also set MTUs above the maximum. It also had hard to follow deeply nested conditionals. Add more paranoid handling, and make the code more straight-line. Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
* device: remove device.state.stopping from RoutineTUNEventReaderJosh Bleecher Snyder2021-02-082-2/+1
| | | | | | | | | | The TUN event reader does three things: Change MTU, device up, and device down. Changing the MTU after the device is closed does no harm. Device up and device down don't make sense after the device is closed, but we can check that condition before proceeding with changeState. There's thus no reason to block device.Close on RoutineTUNEventReader exiting. Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
* device: overhaul device state managementJosh Bleecher Snyder2021-02-088-139/+188
| | | | | | | | | | | | | | | | | | | | | | | | | | This commit simplifies device state management. It creates a single unified state variable and documents its semantics. It also makes state changes more atomic. As an example of the sort of bug that occurred due to non-atomic state changes, the following sequence of events used to occur approximately every 2.5 million test runs: * RoutineTUNEventReader received an EventDown event. * It called device.Down, which called device.setUpDown. * That set device.state.changing, but did not yet attempt to lock device.state.Mutex. * Test completion called device.Close. * device.Close locked device.state.Mutex. * device.Close blocked on a call to device.state.stopping.Wait. * device.setUpDown then attempted to lock device.state.Mutex and blocked. Deadlock results. setUpDown cannot progress because device.state.Mutex is locked. Until setUpDown returns, RoutineTUNEventReader cannot call device.state.stopping.Done. Until device.state.stopping.Done gets called, device.state.stopping.Wait is blocked. As long as device.state.stopping.Wait is blocked, device.state.Mutex cannot be unlocked. This commit fixes that deadlock by holding device.state.mu when checking that the device is not closed. Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
* device: remove unnecessary zeroing in peer.SendKeepaliveJosh Bleecher Snyder2021-02-081-1/+0
| | | | | | elem.packet is always already nil. Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
Copyright © 1996 – 2023 Jason A. Donenfeld. All Rights Reverse Engineered.