archive.Extract Race

[ryanmoran]

Summary

The implementation of archive.Extract is not thread-safe, but is called from goroutine code, causing non-deterministic behavior. Specifically, archive.Extract clears and then resets the global umask value. This value cannot be modified safely from within a goroutine.

This was discovered while we were debugging a report in the Paketo go-dist buildpack. On subsequent builds, the buildpack appeared to produce a layer whose content changed when there were no changes to any of the inputs that might have caused a new layer to be created. Instead, when comparing layers between builds, we noticed that the permissions of the layer directories themselves were different. After some head scratching and tracing of our own code, we became convinced that this must be an issue with the lifecycle. That is when we discovered the archive.Extract code that mutates the global umask value and the concurrent code that invokes archive.Extract.

We've created a simplified reproduction program to isolate the issue.

---

Reproduction

Steps

1. Execute the reproduction program using go run main.go --order serial and witness that the umask values remain unchanged from beginning to end.

   ‣ go run main.go --order serial
   umask before: 22
   umask after: 22
   

2. Execute the reproduction program using go run main.go --order parallel and witness that the umask value is mutated (somewhat non-deterministically).

   ‣ go run main.go --order parallel
   umask before: 22
   umask after: 0
   

Current behavior

The lifecycle mutated the global value of umask leading to non-deterministic permissions when creating directories.

Expected

The lifecycle should set the value of umask back to its original state after modification in a thread-safe manner.

---

Context

lifecycle version

0.11.4

platform version(s)

‣ pack report
Pack:
  Version:  0.21.0+git-a9c294a.build-2814
  OS/Arch:  darwin/amd64

Default Lifecycle Version:  0.11.3

Supported Platform APIs:  0.3, 0.4, 0.5, 0.6

Config:
  default-builder-image = "[REDACTED]"

  [[trusted-builders]]
    name = "[REDACTED]"

‣ docker info
Client:
 Context:    default
 Debug Mode: false
 Plugins:
  buildx: Build with BuildKit (Docker Inc., v0.6.1-docker)
  compose: Docker Compose (Docker Inc., v2.0.0-rc.3)
  scan: Docker Scan (Docker Inc., v0.8.0)

Server:
 Containers: 0
  Running: 0
  Paused: 0
  Stopped: 0
 Images: 5
 Server Version: 20.10.8
 Storage Driver: overlay2
  Backing Filesystem: extfs
  Supports d_type: true
  Native Overlay Diff: true
  userxattr: false
 Logging Driver: json-file
 Cgroup Driver: cgroupfs
 Cgroup Version: 1
 Plugins:
  Volume: local
  Network: bridge host ipvlan macvlan null overlay
  Log: awslogs fluentd gcplogs gelf journald json-file local logentries splunk syslog
 Swarm: inactive
 Runtimes: io.containerd.runc.v2 io.containerd.runtime.v1.linux runc
 Default Runtime: runc
 Init Binary: docker-init
 containerd version: e25210fe30a0a703442421b0f60afac609f950a3
 runc version: v1.0.1-0-g4144b63
 init version: de40ad0
 Security Options:
  seccomp
   Profile: default
 Kernel Version: 5.10.47-linuxkit
 Operating System: Docker Desktop
 OSType: linux
 Architecture: x86_64
 CPUs: 4
 Total Memory: 11.7GiB
 Name: docker-desktop
 ID: T4XN:HJ2I:QI4X:GFX6:PW7J:XG36:TL4W:YQYY:XLMG:KA2U:7AZU:LZNA
 Docker Root Dir: /var/lib/docker
 Debug Mode: true
  File Descriptors: 43
  Goroutines: 45
  System Time: 2021-09-20T18:03:35.907829Z
  EventsListeners: 3
 HTTP Proxy: http.docker.internal:3128
 HTTPS Proxy: http.docker.internal:3128
 Registry: https://index.docker.io/v1/
 Labels:
 Experimental: true
 Insecure Registries:
  127.0.0.0/8
 Live Restore Enabled: false

[natalieparellano]

@ryanmoran thanks for reporting this. We'll try to get a fix out soon.

[natalieparellano]

@ryanmoran this should be fixed in 0.12.0 by #730. Could you let us know if the issue you were seeing is resolved?

[ryanmoran]

@natalieparellano I can confirm that I am now seeing stable file permissions. Thanks for the fix!