From 7180b7604dac3a15550040f6e71084107f2b8558 Mon Sep 17 00:00:00 2001 From: "Brandon McGinty (chatgpt)" Date: Sun, 9 Aug 2026 18:45:57 -0400 Subject: [PATCH] Synchronize UDP decoding with client state --- fix.txt | 2 +- gumble/gumble/handlers.go | 4 ++++ gumble/gumble/udp15.go | 3 +++ gumble/gumble/udp_state_regression_test.go | 27 ++++++++++++++++++++++ 4 files changed, 35 insertions(+), 1 deletion(-) create mode 100644 gumble/gumble/udp_state_regression_test.go diff --git a/fix.txt b/fix.txt index 5197116..bd85b61 100644 --- a/fix.txt +++ b/fix.txt @@ -16,7 +16,7 @@ Priority 0: security and crashers voice traffic. Remove all secret material from logs. At most log lengths, setup success, and a non-secret connection identifier. -2. Native UDP races TCP state mutation +[x] 2. Native UDP races TCP state mutation Files: gumble/gumble/udp15.go, gumble/gumble/handlers.go, gumble/gumble/audiolisteners.go The UDP reader runs independently of the TCP read routine. It reads diff --git a/gumble/gumble/handlers.go b/gumble/gumble/handlers.go index 6db6906..720f0bc 100644 --- a/gumble/gumble/handlers.go +++ b/gumble/gumble/handlers.go @@ -85,6 +85,10 @@ func (c *Client) handleVersion(buffer []byte) error { } func (c *Client) handleUDPTunnel(buffer []byte) error { + // Native UDP and TCP tunnel packets can arrive concurrently. Keep the user + // map and its decoder/sequence state stable for the entire decode. + c.volatile.RLock() + defer c.volatile.RUnlock() if len(buffer) < 1 { log.Warn("handleUDPTunnel: empty buffer") return errInvalidProtobuf diff --git a/gumble/gumble/udp15.go b/gumble/gumble/udp15.go index 84da381..9a53dd0 100644 --- a/gumble/gumble/udp15.go +++ b/gumble/gumble/udp15.go @@ -704,6 +704,9 @@ func (c *Client) markUDPActive() { // dispatchOpus15 processes a decoded MumbleUDP.Audio frame and dispatches // the decoded PCM to audio listeners. func (c *Client) dispatchOpus15(pktNum uint64, session uint32, frameNum int64, opusData []byte, terminator bool, context uint32, position *[3]float32, volumeAdjustment float32) { + // This runs on the UDP reader independently of TCP state handlers. + c.volatile.RLock() + defer c.volatile.RUnlock() if len(opusData) == 0 && !terminator { log.Info("UDP15 #%d: no opus data (session=%d frame=%d), skipping", pktNum, session, frameNum) return diff --git a/gumble/gumble/udp_state_regression_test.go b/gumble/gumble/udp_state_regression_test.go new file mode 100644 index 0000000..9dfbeb7 --- /dev/null +++ b/gumble/gumble/udp_state_regression_test.go @@ -0,0 +1,27 @@ +package gumble + +import ( + "testing" + "time" +) + +// Regression: native UDP decoded Users and per-user decoder state while TCP +// handlers concurrently removed users or changed channels. UDP decoding must +// share the client state lock with those handlers. +func TestUDPTunnelWaitsForClientStateLock(t *testing.T) { + c := &Client{} + c.volatile.Lock() + done := make(chan struct{}) + go func() { _ = c.handleUDPTunnel([]byte{0}); close(done) }() + select { + case <-done: + t.Fatal("UDP handler bypassed client state lock") + case <-time.After(20 * time.Millisecond): + } + c.volatile.Unlock() + select { + case <-done: + case <-time.After(time.Second): + t.Fatal("UDP handler did not resume") + } +}