archive: fix chmod fallback for device nodes on nodev mounts

[thaJeztah]

• fixes Docker 29.7.0 failing to unpack images with devices on older Linux kernels #98
• fixes / addresses Docker 29.7.1 fails to pull all Oracle Linux images with "failed to register layer: openat dev/ptmx: no such file or directory" moby#53299
• fixes/ addresses failed to register layer after release 29.7 moby#53298
• updates archive: harden tar extraction against path traversal #45
• stacked on archive: add test for chmod fallback on nodev mounts (skipped) #102

Commit f43ca4d introduced an O_RDONLY-based chmod fallback for systems where
fchmodat with AT_SYMLINK_NOFOLLOW is unsupported. Opening a device node this
way fails with EACCES when the filesystem is mounted with nodev.

Use O_PATH to reference the entry without opening the underlying device, then
apply the mode through /proc/self/fd. Keep the existing fallback on non-Linux
Unix platforms.

Fix a regression introduced in v0.3.0 that caused archive extraction to fail
when applying permissions to device nodes, including nodes on `nodev`
filesystems and `dev/ptmx`. Device nodes are now referenced without opening
the underlying device before applying their mode.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 53.73134% with 31 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.25%. Comparing base (3daca2a) to head (e738eed).
⚠️ Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
chmod_linux.go	42.85%	9 Missing and 3 partials ⚠️
archive_other.go	0.00%	5 Missing ⚠️
archive_linux.go	60.00%	2 Missing and 2 partials ⚠️
chmod_unix_nolinux.go	50.00%	2 Missing and 2 partials ⚠️
chrootarchive/archive_linux.go	66.66%	2 Missing and 2 partials ⚠️
archive.go	80.00%	0 Missing and 1 partial ⚠️
archive_unix.go	80.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #103      +/-   ##
==========================================
+ Coverage   64.17%   65.25%   +1.07%     
==========================================
  Files          44       46       +2     
  Lines        3045     2377     -668     
==========================================
- Hits         1954     1551     -403     
+ Misses        877      606     -271     
- Partials      214      220       +6     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Copilot]

Pull request overview

This PR fixes a regression in the archive extraction permission-application fallback path on Linux when handling device nodes on nodev mounts. It splits the chmod fallback implementation into Linux vs non-Linux variants, using an O_PATH-based approach on Linux to avoid opening device nodes while still applying modes.

Changes:

• Add a Linux-specific chmodNoSymlinkFallback that opens entries with O_PATH and applies mode via /proc/self/fd/<fd>.
• Move the prior O_RDONLY-based fallback into a non-Linux Unix file to preserve behavior on those platforms.
• Enable the previously skipped Linux regression tests for device nodes on nodev mounts.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
chmod_unix_nolinux.go	Adds the non-Linux fallback implementation previously embedded in archive_unix.go.
chmod_linux.go	Introduces Linux-only fallback using O_PATH + /proc/self/fd to chmod device nodes on nodev mounts.
archive_unix.go	Removes the in-file fallback implementation and delegates to platform-specific files.
archive_linux_test.go	Unskips/updates regression tests so device-node chmod fallback is exercised on Linux.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[thaJeztah]

Heh, good catch. That's a pre-existing issue; fixing in a follow-up that I already had in draft;

• archive: set close-on-exec for chmod fallback descriptors #104

[vvoland]

Will this still work under chrootarchive though? IIRC we don't mount proc there?

[thaJeztah]

Ugh; good call.. nope probably won't work no; need to think how we can communicate that we're in a chroot already 🤔 (probably just checking if /proc/self/fd is missing ... would not be a good thing to do 😓)

[vvoland]

I think we could workaround this if the unshare package would actually open the /proc/self/fd/ dir, store the fd, so then we could just use openat(procSelfDirFd, ...)?

[thaJeztah]

Hm; yeah I was already looking if we could ... somehow pass some internal option. Either pass a "we're in a chroot" and different fallback or some ProcSelfFD int field.

Perhaps should be some internal structure so that others can't just set it;

type TarOptions struct {
	Internal *internal.Options
}

[thaJeztah]

This should be fixed in the second commit (789cef6)

[thaJeztah]

Cherry-picked #105, which should make this PR fail.

=== RUN   TestChmodNoSymlinkFallbackInChrootWithoutProc
    archive_linux_chrooted_test.go:75: assertion failed: error is not nil: chmod target-dir/target-file: via /proc/self/fd/7: no such file or directory
--- FAIL: TestChmodNoSymlinkFallbackInChrootWithoutProc (0.00s)

[thaJeztah]

So, we have the same issue for the non-chrootarchive path in Moby; moby/moby#53305

=== RUN   TestNoOverlayfsWarningsAboutUndefinedBehaviors/cp_to_container
overlayfs_linux_test.go:64: assertion failed: error is not nil: Error response from daemon: chmod new-file: via /proc/self/fd/47: no such file or directory
--- FAIL: TestNoOverlayfsWarningsAboutUndefinedBehaviors/cp_to_container (0.00s)
FAIL amd64.integration.container.TestNoOverlayfsWarningsAboutUndefinedBehaviors/cp_to_container (0.00s)

and

=== Failed
=== FAIL: amd64.integration.container TestCopyEmptyFile (0.03s)
copy_test.go:172: assertion failed: error is not nil: Error response from daemon: chmod empty-file.txt: via /proc/self/fd/30: no such file or directory
=== FAIL: amd64.integration.container TestCopyToContainerCopyUIDGID/image_default (1.02s)
copy_test.go:267: assertion failed: error is not nil: Error response from daemon: chmod empty-file.txt: via /proc/self/fd/36: no such file or directory
--- FAIL: TestCopyToContainerCopyUIDGID/image_default (1.02s)
=== FAIL: amd64.integration.container TestCopyToContainerCopyUIDGID/trailing_colon (1.35s)
copy_test.go:267: assertion failed: error is not nil: Error response from daemon: chmod empty-file.txt: via /proc/self/fd/36: no such file or directory
--- FAIL: TestCopyToContainerCopyUIDGID/trailing_colon (1.35s)
=== FAIL: amd64.integration.container TestCopyToContainerCopyUIDGID/leading_colon (1.35s)
copy_test.go:267: assertion failed: error is not nil: Error response from daemon: chmod empty-file.txt: via /proc/self/fd/36: no such file or directory
--- FAIL: TestCopyToContainerCopyUIDGID/leading_colon (1.35s)

On Oracle Linux 8 (kernel 5.15), we hit the fallback path, but docker cp extracts through containerFSView.RunInFS and calls archive.Untar directly.

PUT /containers/{id}/archive
→ putContainersArchive daemon/server/router/container/copy.go#L89-L101
→ ContainerExtractToDir daemon/archive.go#L63-L81
→ containerExtractToDir daemon/archive_unix.go#L101-L165
→ containerFSView.RunInFS daemon/containerfs_linux.go#L234-L242
→ archive.Untar

containerFSView.RunInFS isolates the extraction filesystem, so /proc/self/fd/<fd> is unavailable, but it bypasses the chrootarchive helper that pre-opens /proc/self/fd. As a result, ProcSelfFD is nil and the fallback uses the inaccessible absolute proc path.

So the fix needs to cover any extraction path that enters an isolated filesystem, not only chrootarchive.

[vvoland]

Or we could make it possible to pass the /proc/self/fd file description via archive.TarOptions?

[thaJeztah]

Oh! Yes, I was already working on an approach; ultimately, I think it may be cleanest if it would be through a constructor (archive.NewArchiver) that takes options, but I now moved, and exported the WithProcSelfFD. In moby we can call that before entering the RunInFS

[thaJeztah]

I'll squash the fixup commit if it all looks good.

edit: done

[vvoland]

Fun - Fable just taught me a thing!

Depending on whether caller forgets to defer closeOptions() after archive.WithProcSelfFD(opts), there's a possibility that the opts.ProcSelfFD object will be GC'd by the time we reach the fchmodat syscall.

Without defer which calls f.Close() the last operation that touches the ProcSelfFD object is: opts.ProcSelfFD.Fd(). After that, GC is free to collect the whole object and close the FD via the runtime finalizer:

		fd := int(opts.ProcSelfFD.Fd())
		// ^ was last usage of ProcSelfFD. may be GCed at this point
		err := unix.Fchmodat(fd, strconv.Itoa(fd), perm, 0)
		// unless we do: runtime.KeepAlive(opts.ProcSelfFD)

To avoid this, we should call runtime.KeepAlive so that Go compiler is aware that it needs to keep the ProcSelfFD alive at least until that point.

Suggested change

	if err := unix.Fchmodat(int(opts.ProcSelfFD.Fd()), strconv.Itoa(fd), perm, 0); err != nil {
	err := unix.Fchmodat(int(opts.ProcSelfFD.Fd()), strconv.Itoa(fd), perm, 0)
	runtime.KeepAlive(opts.ProcSelfFD)
	if err != nil {

[vvoland]

Some stdlib example:

https://github.com/golang/go/blob/d90b98e65320778f3b1f99a6951ab20f04d218b3/src/os/exec_posix.go#L50-L58

[vvoland]

LGTM