bug(orchestrator): Cleanup.Add has TOCTOU race — cleanup functions silently dropped after Run()
I maintainer di solito rispondono entro 1 giorno
Nessuno ha ancora preso questa issue.
Valutazione
- Difficoltà
- 3/5
- Tempo stimato
- 1-2 giorni
- Idoneità per principianti
- 72/100
- Tipo di issue
- Bug
- Chiarezza
- Specificata chiaramente
- Stato di attività
- Tranquilla
- Stack tecnologico
- go
- Ambito
- infrastructure
Direzione di ricerca
Esamina Cleanup.Add, Cleanup.AddPriority e Cleanup.run in packages/orchestrator/pkg/sandbox/cleanup.go, concentrandoti sull’ordine di lock e hasRun. Conferma che la modifica chiuda la finestra TOCTOU e che le funzioni di pulizia registrate concorrentemente con Run() non vengano più scartate silenziosamente, quindi esegui i test esistenti del package con il race detector.
Scritto dal modello di indicizzazione a partire dal testo della issue.
Descrizione
Summary
Cleanup.Add and Cleanup.AddPriority have a TOCTOU (time-of-check/time-of-use) race against Cleanup.run that silently discards cleanup functions without executing them, causing permanent resource leaks.
Root Cause
In packages/orchestrator/pkg/sandbox/cleanup.go:
run() sets hasRun before acquiring the lock (line 80 vs 82):
func (c *Cleanup) run(ctx context.Context) {
c.hasRun.Store(true) // ← written outside the lock
c.mu.Lock()
defer c.mu.Unlock()
// ... drains c.cleanup and c.priorityCleanup
}
Add() reads hasRun before acquiring the lock (line 40 vs 49):
func (c *Cleanup) Add(ctx context.Context, f func(ctx context.Context) error) {
if c.hasRun.Load() == true { // ← read outside the lock
// run f immediately
return
}
c.mu.Lock()
defer c.mu.Unlock()
c.cleanup = append(c.cleanup, f) // ← appended after run() may have drained
}
Race Window
Goroutine A (Add): hasRun.Load() == false → [not yet locked]
Goroutine B (run): hasRun.Store(true) → Lock() → drain all cleanup → Unlock()
Goroutine A (Add): Lock() → append(f) ← f is now in a drained slice, never executed
sync.Once in Run() prevents run() from being called a second time, so f is permanently lost — no log, no error, no signal.
Impact
The Cleanup type is the central resource-release mechanism for Firecracker sandbox lifecycle. The package has 28 call sites across sandbox startup (sandbox.go, resume.go, reboot.go, etc.) registering operations such as:
- Closing overlay filesystems
- Releasing network slots
- Removing Firecracker and UFFD socket files
- Stopping the Firecracker process
Any one of these silently dropped during a concurrent error-path teardown leaves a leaked resource on the host until the orchestrator restarts.
Fix
Move hasRun.Store(true) inside the lock in run(), and add a double-check in Add() / AddPriority() after acquiring mu:
func (c *Cleanup) Add(ctx context.Context, f func(ctx context.Context) error) {
// Optimistic fast path.
if c.hasRun.Load() {
err := f(context.WithoutCancel(ctx))
if err != nil {
logger.L().Error(ctx, "failed to run function after cleanup has run", zap.Error(err))
}
return
}
c.mu.Lock()
// Double-check: run() may have completed between the Load above and here.
if c.hasRun.Load() {
c.mu.Unlock()
err := f(context.WithoutCancel(ctx))
if err != nil {
logger.L().Error(ctx, "failed to run function after cleanup has run", zap.Error(err))
}
return
}
c.cleanup = append(c.cleanup, f)
c.mu.Unlock()
}
func (c *Cleanup) run(ctx context.Context) {
c.mu.Lock()
defer c.mu.Unlock()
c.hasRun.Store(true) // now set under the lock — closes the race window
// ... rest unchanged
}
The same double-check applies to AddPriority.
- Lingua principale
- Go
- Stelle
- 1.6k
- Fork
- 448
- Metriche di merge delle PR
- Nessuna PR unita negli ultimi 30g
Preparare l'ambiente
Avvia il container di sviluppo del progetto nel browser, con il tuo account GitHub.
- Nessun Dockerfile né 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 e2b-dev/runtime
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 88/100
I maintainer di solito rispondono entro 1 giorno
-
sandbox cache: StartRemoving state transition not broadcast, all allocations see stale Running stateAperta
Difficoltà 2/5 1-3 ore Idoneità per principianti 86/100
I maintainer di solito rispondono entro 1 giorno
-
Difficoltà 1/5 Meno di un'ora Idoneità per principianti 86/100
I maintainer di solito rispondono entro 1 giorno
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 86/100
I maintainer di solito rispondono entro 1 giorno
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 88/100
I maintainer di solito rispondono entro 1 giorno
Tutte le issue di e2b-dev/runtime
Issue simili
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 78/100
siderolabs/terraform-provider-talos#414 ·
I maintainer di solito rispondono entro 1 giorno
-
bug
Difficoltà 2/5 1-3 ore Idoneità per principianti 78/100
I maintainer di solito rispondono entro 1 giorno
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 70/100
JuliaComputing/jh#63 · 1 commento ·
-
area/proxy kind/bug priority/backlog triage/accepted
Difficoltà 2/5 1-3 ore Idoneità per principianti 68/100
lexfrei/cloudflare-tunnel-gateway-controller#840 ·
I maintainer di solito rispondono entro 1 giorno
-
Priority: Normal Type: Bug
Difficoltà 2/5 1-3 ore Idoneità per principianti 78/100
cloudflare/cloudflared#1747 ·