flightcontrol: nil pointer dereference in (*progressState).run when a waiter is cancelled as the call finishes
I maintainer di solito rispondono entro 1 giorno
Una pull request collegata è già stata integrata.
- #7256 di @crazy-max — integrata
Valutazione
- Difficoltà
- 2/5
- Tempo stimato
- 1-3 ore
- Idoneità per principianti
- 90/100
- Tipo di issue
- Bug
- Chiarezza
- Specificata chiaramente
- Stato di attività
- Attiva
- Stack tecnologico
- go
- Ambito
- build-system
Direzione di ricerca
La data race si trova in util/flightcontrol/flightcontrol.go. Esamina (*progressState).run intorno alle righe 303-308, dove i writer vengono chiusi senza mantenere il lock, e progressState.close intorno alla riga 356, dove slices.Delete modifica ps.writers. Applica la correzione suggerita: acquisisci uno snapshot di ps.writers sotto il lock, poi chiudi lo snapshot al di fuori della sezione critica. Esegui il test riproduttore fornito per confermare che il panic non si verifica più.
Scritto dal modello di indicizzazione a partire dal testo della issue.
Descrizione
Summary
dockerd crashes with a nil pointer dereference in util/flightcontrol.(*progressState).run. The cause is a data race between run and progressState.close.
When run reads io.EOF, it sets ps.done under ps.mu, releases the lock, and then ranges over ps.writers and calls Close on each one without the lock (flightcontrol.go:303-308 at v0.33.0). Meanwhile, a waiter whose context is cancelled while other waiters still hold the call reaches progressState.close. That function removes its writer under the lock with ps.writers = slices.Delete(ps.writers, i, i+1) (flightcontrol.go:356).
Since Go 1.22, slices.Delete zeroes the vacated tail element. The unlocked range in run still uses the old length, so it reads a nil rawProgressWriter and calls Close on it. addr=0x18 is the offset of the first method in the itab, and Close is that method.
The unlocked read itself was reported as a data race in #3923, which was closed without a code change. It became a crash with b5286f8dcb ("apply x/tools/modernize fixes", 2025-03-07). That commit replaced the append-based removal with slices.Delete. It first shipped in v0.21.0 (Docker 28.1.0), and the code is byte-identical in v0.33.0, v0.33.1 and master as of 2026-10-06.
Observed
Docker 29.8.0, vendoring BuildKit v0.33.0, as a rootless daemon with the containerd snapshotter, running several concurrent docker buildx bake sessions (docker driver) whose targets share identical stages:
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x...]
goroutine ... [running]:
github.com/moby/moby/v2/vendor/github.com/moby/buildkit/util/flightcontrol.(*progressState).run(...)
.../vendor/github.com/moby/buildkit/util/flightcontrol/flightcontrol.go:307 +0x201
created by github.com/moby/moby/v2/vendor/github.com/moby/buildkit/util/flightcontrol.newCall[...] in goroutine ...
.../vendor/github.com/moby/buildkit/util/flightcontrol/flightcontrol.go:113 +0x245
The daemon exits, and every running container and build on it dies with it. On this host it happened once in about 5.5 days of continuous build load.
Reproducer
This is a standalone test against github.com/moby/buildkit at v0.33.0. It uses one shared call, one waiter that stays, and 32 waiters whose contexts are cancelled at about the moment the shared function returns:
package repro
import (
"context"
"math/rand"
"sync"
"testing"
"time"
"github.com/moby/buildkit/util/flightcontrol"
"github.com/moby/buildkit/util/progress"
)
func TestProgressStateCloseRace(t *testing.T) {
const iterations = 20000
const cancelled = 32
for i := 0; i < iterations; i++ {
var g flightcontrol.Group[int]
release := make(chan struct{})
fn := func(ctx context.Context) (int, error) {
<-release
return 1, nil
}
var wg sync.WaitGroup
_, keepCtx, keepClose := progress.NewContext(context.Background())
wg.Add(1)
go func() { defer wg.Done(); _, _ = g.Do(keepCtx, "k", fn) }()
cancels := make([]context.CancelFunc, 0, cancelled)
for j := 0; j < cancelled; j++ {
_, pctx, _ := progress.NewContext(context.Background())
ctx, cancel := context.WithCancel(pctx)
cancels = append(cancels, cancel)
wg.Add(1)
go func() { defer wg.Done(); _, _ = g.Do(ctx, "k", fn) }()
}
time.Sleep(200 * time.Microsecond)
go func() {
time.Sleep(time.Duration(rand.Intn(40)) * time.Microsecond)
close(release)
}()
for _, c := range cancels {
c()
}
wg.Wait()
keepClose(nil)
}
}
Results with Go 1.26:
- v0.33.0 panics within a fraction of a second with the same trace (flightcontrol.go:307 and :113,
addr=0x18). Under-raceit reports a DATA RACE betweenslices.DeleteincloseandCloseinrun. - v0.21.0 panicked in 3 of 3 runs.
- v0.20.2, which uses append-based removal, passed in 3 of 3 runs.
Suggested fix
Snapshot the writers while holding the lock, then close the snapshot:
if errors.Is(err, io.EOF) {
ps.mu.Lock()
ps.done = true
writers := slices.Clone(ps.writers)
ps.mu.Unlock()
for _, w := range writers {
w.Close()
}
}
With this change, v0.33.0 passed the reproducer in 3 of 3 runs of 20,000 iterations. Closing the writers while holding the lock would also work, but cloning keeps Close outside the critical section.
- Lingua principale
- Go
- Stelle
- 10.3k
- Fork
- 1.5k
- Merge medio
- 2g 2h
- PR unite (30g)
- 68
Preparare l'ambiente
- Include un Dockerfile o un file Docker Compose
- Nessun modello di pull request
- Leggi la guida per i contributori
Come iniziare
- Leggi tutta la issue e poi la guida ai contributi del progetto.
- Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
- Fai un fork del repository e lavora su un branch.
- Apri una pull request che faccia riferimento al numero della issue.
Altre issue di moby/buildkit
-
docs: rootless.md should recommend a per-binary AppArmor profile over `apparmor_restrict_unprivileged_userns=0Forse già presa @shashankvarma499 l’ha presa 24 giorni fa. Aperta
Difficoltà 2/5 1-3 ore Idoneità per principianti 84/100
I maintainer di solito rispondono entro 1 giorno
-
area/dockerfile
Difficoltà 2/5 1-3 ore Idoneità per principianti 65/100
I maintainer di solito rispondono entro 1 giorno
-
area/source needs/maintainer-decision
Difficoltà 5/5 Più di una settimana Idoneità per principianti 30/100
I maintainer di solito rispondono entro 1 giorno
-
Avoid pulling whole Nydus layers to merge cached imagesForse già presa @shayonj l’ha presa 9 giorni fa. Aperta
Difficoltà 4/5 3-5 giorni Idoneità per principianti 48/100
I maintainer di solito rispondono entro 1 giorno
-
Difficoltà 4/5 3-5 giorni Idoneità per principianti 52/100
I maintainer di solito rispondono entro 1 giorno
Tutte le issue di moby/buildkit
Issue simili
-
bug from-studio
Difficoltà 2/5 1-3 ore Idoneità per principianti 63/100
esengine/DeepSeek-Reasonix#12355 ·
I maintainer di solito rispondono entro 1 giorno
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 85/100
I maintainer di solito rispondono entro 1 giorno
-
needs-triage
Difficoltà 2/5 1-3 ore Idoneità per principianti 76/100
gke-labs/kube-agents#2612 ·
I maintainer di solito rispondono entro 1 giorno
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 66/100
bluesky-social/indigo#1496 ·
I maintainer di solito rispondono entro 2 giorni
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 86/100
Gentleman-Programming/gentle-ai#5371 ·
I maintainer di solito rispondono entro 1 giorno