Hacktoberfest 2026: le issue che i maintainer hanno segnato per ottobre, aperte e adatte ai principianti. Sfoglia le issue Hacktoberfest

bug(orchestrator): Cleanup.Add has TOCTOU race — cleanup functions silently dropped after Run()

Aperta
#3,557 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub

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

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

Apri in Codespaces

Avvia il container di sviluppo del progetto nel browser, con il tuo account GitHub.

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di e2b-dev/runtime

Tutte le issue di e2b-dev/runtime

Issue simili

Altre issue su Go

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.