Skip to content

Commit

Permalink
Fix race in dowloader.Cached()
Browse files Browse the repository at this point in the history
Checking if an image is cached races with parallel downloads. Take the
lock when validating the digest or the data file to ensure that we
validate the cached when it is in consistent state.

If an image is being downloaded, the check will block until the download
completes.

Signed-off-by: Nir Soffer <[email protected]>
  • Loading branch information
nirs committed Nov 14, 2024
1 parent c1b4d72 commit 5071535
Showing 1 changed file with 22 additions and 7 deletions.
29 changes: 22 additions & 7 deletions pkg/downloader/downloader.go
Original file line number Diff line number Diff line change
Expand Up @@ -361,18 +361,33 @@ func Cached(remote string, opts ...Opt) (*Result, error) {
if err != nil {
return nil, err
}

// Checking if data file exists is safe without locking.
if _, err := os.Stat(shadData); err != nil {
return nil, err
}
if _, err := os.Stat(shadDigest); err != nil {
if err := validateCachedDigest(shadDigest, o.expectedDigest); err != nil {
return nil, err
}
} else {
if err := validateLocalFileDigest(shadData, o.expectedDigest); err != nil {
return nil, err

// But validating the digest or the data file must take the lock to avoid races
// with parallel downloads.
if err := os.MkdirAll(shad, 0o700); err != nil {
return nil, err
}
err = lockutil.WithDirLock(shad, func() error {
if _, err := os.Stat(shadDigest); err != nil {
if err := validateCachedDigest(shadDigest, o.expectedDigest); err != nil {
return err
}
} else {
if err := validateLocalFileDigest(shadData, o.expectedDigest); err != nil {
return err
}
}
return nil
})
if err != nil {
return nil, err
}

res := &Result{
Status: StatusUsedCache,
CachePath: shadData,
Expand Down

0 comments on commit 5071535

Please sign in to comment.