Repository navigation
x/sync/semaphore: Release and Acquire with negative weights bypass safeguards and corrupt internal state #80183
Description
Go version
go version go1.26.4 windows/amd64
Output of go env in your module/workspace:
set AR=ar
set CC=gcc
set CGO_CFLAGS=-O2 -g
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-O2 -g
set CGO_ENABLED=0
set CGO_FFLAGS=-O2 -g
set CGO_LDFLAGS=-O2 -g
set CXX=g++
set GCCGO=gccgo
set GO111MODULE=
set GOAMD64=v1
set GOARCH=amd64
set GOAUTH=netrc
set GOBIN=
set GOCACHE=C:\Users\USER\AppData\Local\go-build
set GOCACHEPROG=
set GODEBUG=
set GOENV=C:\Users\USER\AppData\Roaming\go\env
set GOEXE=.exe
set GOEXPERIMENT=
set GOFIPS140=off
set GOFLAGS=
set GOGCCFLAGS=-m64 -fno-caret-diagnostics -Qunused-arguments -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=C:\Users\USER\AppData\Local\Temp\go-build833783488=/tmp/go-build -gno-record-gcc-switches
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GOMOD=NUL
set GOMODCACHE=C:\Users\USER\go\pkg\mod
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=C:\Users\USER\go
set GOPRIVATE=
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=C:\Program Files\Go
set GOSUMDB=sum.golang.org
set GOTELEMETRY=local
set GOTELEMETRYDIR=C:\Users\USER\AppData\Roaming\go\telemetry
set GOTMPDIR=
set GOTOOLCHAIN=auto
set GOTOOLDIR=C:\Program Files\Go\pkg\tool\windows_amd64
set GOVCS=
set GOVERSION=go1.26.4
set GOWORK=
set PKG_CONFIG=pkg-configWhat did you do?
The semaphore.Weighted API accepts int64 for weights in Acquire, TryAcquire, and Release. If a negative weight is passed due to a logic error or unvalidated input, it mathematically corrupts the internal state tracker (s.cur) and bypasses the package's existing safety checks.
Specifically, in Release, the safeguard intended to prevent over-releasing relies on checking if s.cur < 0 after subtraction:
s.cur -= n
if s.cur < 0 {
s.mu.Unlock()
panic("semaphore: released more than held")
}
If n is negative (e.g., -10), s.cur -= (-10) effectively adds 10 to the utilization tracker. Because the resulting s.cur is highly positive, the s.cur < 0 safeguard is bypassed.
This artificially inflates s.cur beyond s.size. Consequently, notifyWaiters() silently fails because s.size - s.cur >= w.n evaluates to false, permanently starving the waiter queue.
Proof of Concept:
Go
package main
import (
"fmt"
"golang.org/x/sync/semaphore"
)
func main() {
sem := semaphore.NewWeighted(10)
// Releasing a negative weight bypasses the s.cur < 0 panic check
// and inflates s.cur to 100.
sem.Release(-100)
// Legitimate operations now permanently fail.
success := sem.TryAcquire(1)
if !success {
fmt.Println("BUG: TryAcquire failed due to corrupted state.")
}
}What did you see happen?
The negative input is silently accepted, the s.cur < 0 safeguard is bypassed, and the semaphore is permanently deadlocked for all future legitimate acquisitions.
Suggested Fix:
Add strict boundary validation at the beginning of the core API methods (NewWeighted, Acquire, TryAcquire, Release) to trap invalid weights immediately:
if n <= 0 { // or n < 0 if zero-weight is intended to be a no-op
panic("semaphore: weight must be positive")
}
### What did you expect to see?
In accordance with standard library idioms (e.g., `sync.WaitGroup.Add` panicking on negative counters), the API should fail-fast on state corruption.
I expected the API to panic immediately when a negative weight is provided, trapping the logic error before the semaphore's internal state machine is permanently broken.
Activity
gabyhelp commentedon Jun 28, 2026
Related Issues
(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)
adonovan commentedon Jun 29, 2026
In general we don't promise to assert all preconditions, but I agree it makes sense to do it here where it is simple and cheap and turns a delayed corruption into an immediate panic.
ZainYoussef commentedon Jun 30, 2026
Thanks for reviewing this Alan!
I'd be happy to put together a CL adding the boundary checks to turn this into an immediate panic. Before I open it, should
n == 0also panic, or should a zero-weight operation be treated as a valid no-op?Let me know your preference and I will get the patch submitted.
adonovan commentedon Jun 30, 2026
n=0 seems like a valid case that monotonically continues the pattern: if there are no waiters, it will run immediately, and if there are waiters, it will wait for its turn and then run immediately.
semaphore: panic on negative weights
gopherbot commentedon Jul 1, 2026
Change https://go.dev/cl/796080 mentions this issue:
semaphore: panic on negative weightssemaphore: panic on negative weights
semaphore: panic on negative weights
semaphore: panic on negative weights
semaphore: panic on negative weights
gopherbot commentedon Aug 1, 2026
Change https://go.dev/cl/808920 mentions this issue:
semaphore: panic on negative capacitysemaphore: panic on negative capacity