daemon: fix docker cp with nested bind mounts

A container with two overlapping bind mounts -- one covering a
directory, and one whose destination is a path inside that directory --
fails docker cp in both directions:

    Error response from daemon: openat path/to/sock: no such device or
    address

openContainerFS replays the container's bind mounts parent-first, so by
the time it sets up the inner mount, that mount's destination resolves
to the contents of the outer mount rather than to the placeholder in the
container's own filesystem. The destination can therefore be any kind of
file, and both places where the mount setup opens it assume otherwise.

createIfNotExists opens the destination with O_CREATE|O_WRONLY and no
O_EXCL, so it opens a destination which already exists. Opening a socket
that way fails with ENXIO, opening a FIFO blocks until a reader shows up
-- wedging the unshared thread while the container lock is held -- and
opening a directory fails with EISDIR. Add O_EXCL and treat EEXIST as
success: the destination is either created fresh or left alone, and
never opened. Doing this atomically matters because the destination is
in the container's own filesystem, where a container process can
substitute a FIFO for it; a check followed by an open could be raced
into the blocking case.

The descriptor which pins the destination against a symlink swap is
opened with O_PATH, which works for any file type and never opens the
file for I/O. Only that descriptor's /proc/self/fd path is used, as a
mount target, so it does not need to be readable.

Signed-off-by: Cory Snider <csnider@mirantis.com>
Assisted-By: Claude Opus 5

daemon/containerfs_linux.go

@@ -143,7 +143,10 @@ func (daemon *Daemon) openContainerFS(ctr *container.Container) (_ *containerFSV
 				// file descriptor pinning the resolved inode. Using
 				// /proc/self/fd/<fd> as the mount target prevents any
 				// subsequent symlink swap from redirecting the mount.
-				targetFile, err := root.Open(relDest)
+				// O_PATH is required: the target is not necessarily a
+				// regular file or a directory, and opening a socket or a
+				// FIFO for I/O would fail or block.
+				targetFile, err := root.OpenFile(relDest, unix.O_PATH, 0)
 				if err != nil {
 					return fmt.Errorf("open mount target %q: %w", m.Destination, err)
 				}
@@ -314,7 +317,16 @@ func createIfNotExists(root *os.Root, unsafePath string, isDir bool) error {
 		}
 	}
 
-	f, err := root.OpenFile(unsafePath, os.O_CREATE|os.O_WRONLY, 0o755)
+	// O_EXCL leaves an existing destination alone whatever its type, and does
+	// so atomically. Checking for the destination and then opening it would
+	// let a container process substitute a FIFO in between, and opening a
+	// FIFO for writing blocks until a reader shows up -- wedging the thread
+	// while the container lock is held. Opening an existing socket would fail
+	// with ENXIO, and an existing directory with EISDIR.
+	f, err := root.OpenFile(unsafePath, os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0o755)
+	if errors.Is(err, os.ErrExist) {
+		return nil
+	}
 	if err != nil {
 		return err
 	}

daemon/containerfs_linux_test.go

@@ -1,10 +1,13 @@
 package daemon
 
 import (
+	"net"
 	"os"
 	"path/filepath"
 	"testing"
+	"time"
 
+	"golang.org/x/sys/unix"
 	"gotest.tools/v3/assert"
 )
 
@@ -42,4 +45,67 @@ func TestCreateIfNotExists(t *testing.T) {
 		err = createIfNotExists(root, "file/to/create", false)
 		assert.NilError(t, err, "Should not fail if already exists")
 	})
+
+	// A mount destination which already exists is left alone whatever its
+	// type. Nested bind mounts make the destination of the inner mount
+	// resolve to the outer mount's contents, so it can be any kind of file.
+	t.Run("existing socket", func(t *testing.T) {
+		dir := t.TempDir()
+		l, err := net.Listen("unix", filepath.Join(dir, "sock"))
+		assert.NilError(t, err)
+		defer l.Close()
+
+		root, err := os.OpenRoot(dir)
+		assert.NilError(t, err)
+		defer root.Close()
+
+		err = createIfNotExists(root, "sock", false)
+		assert.NilError(t, err, "Should not fail if already exists as a socket")
+	})
+	t.Run("existing fifo", func(t *testing.T) {
+		dir := t.TempDir()
+		assert.NilError(t, unix.Mkfifo(filepath.Join(dir, "fifo"), 0o600))
+
+		root, err := os.OpenRoot(dir)
+		assert.NilError(t, err)
+		defer root.Close()
+
+		// Opening a FIFO blocks until the other end is opened, so guard
+		// against the test hanging for the whole -timeout if it regresses.
+		done := make(chan error, 1)
+		go func() { done <- createIfNotExists(root, "fifo", false) }()
+		select {
+		case err := <-done:
+			assert.NilError(t, err, "Should not fail if already exists as a FIFO")
+		case <-time.After(10 * time.Second):
+			t.Fatal("createIfNotExists blocked on an existing FIFO")
+		}
+	})
+	t.Run("existing socket when a directory is expected", func(t *testing.T) {
+		dir := t.TempDir()
+		l, err := net.Listen("unix", filepath.Join(dir, "sock"))
+		assert.NilError(t, err)
+		defer l.Close()
+
+		root, err := os.OpenRoot(dir)
+		assert.NilError(t, err)
+		defer root.Close()
+
+		// Nothing can be bind-mounted onto it -- the kernel rejects a
+		// directory source with ENOTDIR -- so it is reported rather than
+		// left for mount(2) to fail on.
+		err = createIfNotExists(root, "sock", true)
+		assert.ErrorIs(t, err, os.ErrExist)
+	})
+	t.Run("existing directory when a file is expected", func(t *testing.T) {
+		dir := t.TempDir()
+		assert.NilError(t, os.Mkdir(filepath.Join(dir, "dir"), 0o755))
+
+		root, err := os.OpenRoot(dir)
+		assert.NilError(t, err)
+		defer root.Close()
+
+		err = createIfNotExists(root, "dir", false)
+		assert.NilError(t, err, "Should not fail if already exists as a directory")
+	})
 }

integration/container/copy_linux_test.go

@@ -2,6 +2,8 @@ package container
 
 import (
 	"bytes"
+	"io"
+	"net"
 	"os"
 	"path/filepath"
 	"testing"
@@ -64,3 +66,52 @@ RUN touch /root/nil && ln -s /root /sockets
 	})
 	assert.NilError(t, err)
 }
+
+// TestCopyWithNestedBindMountedSocket is a regression test for a container
+// with two overlapping bind mounts: one covering a directory, and one whose
+// destination is a path inside that directory.
+//
+// openContainerFS replays the container's bind mounts parent-first, so by the
+// time the inner mount is set up its destination resolves to the contents of
+// the outer mount, and may be any kind of file. The security fix in
+// GHSA-vp62-88p7-qqf5 started unconditionally opening that destination -- to
+// create it, and again to pin its inode -- which fails with ENXIO when it is a
+// socket, breaking docker cp in both directions for the whole container.
+func TestCopyWithNestedBindMountedSocket(t *testing.T) {
+	skip.If(t, testEnv.DaemonInfo.OSType != "linux")
+	ctx := setupTest(t)
+	apiClient := testEnv.APIClient()
+
+	// Use testutil.TempDir so the rootless daemon can access the bind-mount
+	// source: t.TempDir() creates a 0700 parent that the fake-root user
+	// cannot stat.
+	srcDir := testutil.TempDir(t)
+	l, err := net.Listen("unix", filepath.Join(srcDir, "sock"))
+	assert.NilError(t, err)
+	defer l.Close()
+
+	cid := container.Create(ctx, t, apiClient,
+		container.WithMount(mounttypes.Mount{
+			Type:   mounttypes.TypeBind,
+			Source: srcDir,
+			Target: "/mountpoint",
+		}),
+		container.WithMount(mounttypes.Mount{
+			Type:   mounttypes.TypeBind,
+			Source: filepath.Join(srcDir, "sock"),
+			Target: "/mountpoint/sock",
+		}),
+	)
+
+	res, err := apiClient.CopyFromContainer(ctx, cid, client.CopyFromContainerOptions{SourcePath: "/mountpoint"})
+	assert.NilError(t, err)
+	defer res.Content.Close()
+	_, err = io.Copy(io.Discard, res.Content)
+	assert.NilError(t, err)
+
+	_, err = apiClient.CopyToContainer(ctx, cid, client.CopyToContainerOptions{
+		DestinationPath: "/mountpoint/",
+		Content:         bytes.NewReader(nil),
+	})
+	assert.NilError(t, err)
+}