-
Notifications
You must be signed in to change notification settings - Fork 36
Closed
Description
Description
The StartDockerImageDownload function in pkg/cli/docker_images.go has a time-of-check-time-of-use (TOCTOU) race condition. The availability check happens outside the lock, creating a race window where multiple threads could initiate duplicate downloads or miss externally-downloaded images.
Problem
func StartDockerImageDownload(image string) bool {
// First check if already available (NO LOCK)
if IsDockerImageAvailable(image) {
return false
}
// Check if already downloading (LOCK ACQUIRED HERE)
pullState.mu.Lock()
if pullState.downloading[image] {
pullState.mu.Unlock()
return false
}
pullState.downloading[image] = true
pullState.mu.Unlock()
go func() { /* download logic */ }()
return true
}Impact
- Severity: Critical
- Risk: Duplicate goroutines, resource waste, potential deadlocks
- Files Affected:
pkg/cli/docker_images.go:68-83
Suggested Fix
Move the availability check inside the lock to make it atomic:
func StartDockerImageDownload(image string) bool {
pullState.mu.Lock()
defer pullState.mu.Unlock()
// Check if already available (inside lock for atomicity)
if IsDockerImageAvailable(image) {
dockerImagesLog.Printf("Image %s is already available", image)
return false
}
// Check if already downloading
if pullState.downloading[image] {
dockerImagesLog.Printf("Image %s is already downloading", image)
return false
}
pullState.downloading[image] = true
go func() { /* download logic */ }()
return true
}Success Criteria
- Availability check moved inside mutex lock
- All tests in
pkg/cli/docker_images_test.gopass - Add concurrency test with multiple goroutines
- Verify no performance regression
- Check for deadlocks if
IsDockerImageAvailablealso takes locks
Source
Extracted from Sergo Concurrency Analysis discussion #10665
Priority
Critical - Could cause resource leaks and duplicate downloads in production
AI generated by Discussion Task Miner - Code Quality Improvement Agent
- expires on Feb 2, 2026, 2:08 PM UTC
Copilot