[GH-ISSUE #155] v1.0.4: Device.Close races with other devices via package-global protocol types #77

New issue

Open

opened 2026-10-13 21:03:21 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-13 21:03:21 +00:00

Owner

Originally created by @autopeasant on GitHub (Jul 23, 2026).
Original GitHub issue: https://github.com/amnezia-vpn/amneziawg-go/issues/155

Affected version

github.com/amnezia-vpn/amneziawg-go v1.0.4 (latest stable Go module release)
Go 1.26.4
Linux amd64

Minimal reproduction

Add this test under package device and run go test -race ./device -run TestResetProtocolRace -count=1:

func TestResetProtocolRace(t *testing.T) {
    var reader, closer Device
    done := make(chan struct{})
    var wg sync.WaitGroup
    wg.Add(1)
    go func() {
        defer wg.Done()
        for {
            select {
            case <-done:
                return
            default:
                reader.awg.ASecMux.RLock()
                _ = MessageTransportType
                reader.awg.ASecMux.RUnlock()
            }
        }
    }()

    for i := 0; i < 10_000; i++ {
        closer.resetProtocol()
    }
    close(done)
    wg.Wait()
}

A production-shaped consumer reproduction also trips reliably when two plain Device instances run in one process: closing one instance executes Device.Close -> resetProtocol, while the other instance's RoutineReceiveIncoming reads the package globals. In 10 iterations, the race detector reported the race once.

Observed behavior

The race detector reports a write/read race between:

WRITE: device.(*Device).resetProtocol
  device/device.go:590
  device.(*Device).Close

READ: device.(*Device).RoutineReceiveIncoming
  device/receive.go:174

resetProtocol writes the package-level MessageInitiationType, MessageResponseType, MessageCookieReplyType, and MessageTransportType. Receive/send/handshake paths read the same globals. device.awg.ASecMux does not synchronize the accesses because each Device owns a different mutex.

handlePostConfig also writes these package globals, plus the package-level packetSizeToMsgType and msgTypeToJunkSize maps, so multiple configured devices can both race and overwrite each other's protocol state.

Expected behavior

Distinct Device instances should not share mutable protocol-type or packet-shape state. Closing or configuring one device should neither race with nor alter another live device.

Moving the four message types and the two lookup maps into Device state would remove both the data race and cross-device configuration interference. Cookie-reply generation also needs the owning device's cookie message type rather than the package global.

Additional context

Current master appears to have moved header state onto each Device as part of the unreleased concealment/refactoring work, but the latest stable module release remains v1.0.4 and still contains resetProtocol plus the mutable package globals. A focused backport or a stable release containing the per-device state would address consumers pinned to stable versions.

Originally created by @autopeasant on GitHub (Jul 23, 2026). Original GitHub issue: https://github.com/amnezia-vpn/amneziawg-go/issues/155 ## Affected version - `github.com/amnezia-vpn/amneziawg-go v1.0.4` (latest stable Go module release) - Go `1.26.4` - Linux amd64 ## Minimal reproduction Add this test under package `device` and run `go test -race ./device -run TestResetProtocolRace -count=1`: ```go func TestResetProtocolRace(t *testing.T) { var reader, closer Device done := make(chan struct{}) var wg sync.WaitGroup wg.Add(1) go func() { defer wg.Done() for { select { case <-done: return default: reader.awg.ASecMux.RLock() _ = MessageTransportType reader.awg.ASecMux.RUnlock() } } }() for i := 0; i < 10_000; i++ { closer.resetProtocol() } close(done) wg.Wait() } ``` A production-shaped consumer reproduction also trips reliably when two plain `Device` instances run in one process: closing one instance executes `Device.Close -> resetProtocol`, while the other instance's `RoutineReceiveIncoming` reads the package globals. In 10 iterations, the race detector reported the race once. ## Observed behavior The race detector reports a write/read race between: ```text WRITE: device.(*Device).resetProtocol device/device.go:590 device.(*Device).Close READ: device.(*Device).RoutineReceiveIncoming device/receive.go:174 ``` `resetProtocol` writes the package-level `MessageInitiationType`, `MessageResponseType`, `MessageCookieReplyType`, and `MessageTransportType`. Receive/send/handshake paths read the same globals. `device.awg.ASecMux` does not synchronize the accesses because each `Device` owns a different mutex. `handlePostConfig` also writes these package globals, plus the package-level `packetSizeToMsgType` and `msgTypeToJunkSize` maps, so multiple configured devices can both race and overwrite each other's protocol state. ## Expected behavior Distinct `Device` instances should not share mutable protocol-type or packet-shape state. Closing or configuring one device should neither race with nor alter another live device. Moving the four message types and the two lookup maps into `Device` state would remove both the data race and cross-device configuration interference. Cookie-reply generation also needs the owning device's cookie message type rather than the package global. ## Additional context Current `master` appears to have moved header state onto each `Device` as part of the unreleased concealment/refactoring work, but the latest stable module release remains `v1.0.4` and still contains `resetProtocol` plus the mutable package globals. A focused backport or a stable release containing the per-device state would address consumers pinned to stable versions.

bsakaguchi referenced this issue

2026-10-13 21:03:53 +00:00

[PR #78] [MERGED] Sync with Major Upstream changes #130