From 1861abdc4a31efad202a5c3d89a895bb7a62799a Mon Sep 17 00:00:00 2001
From: Kir Kolyshkin <kolyshkin@gmail.com>
Date: Fri, 10 Nov 2017 15:43:57 -0800
Subject: [PATCH] Fix "duplicate mount point" when --tmpfs /dev/shm is used

This is a fix to the following issue:

$ docker run --tmpfs /dev/shm busybox sh
docker: Error response from daemon: linux mounts: Duplicate mount point '/dev/shm'.

In current code (daemon.createSpec()), tmpfs mount from --tmpfs is added
to list of mounts (`ms`), when the mount from IpcMounts() is added.
While IpcMounts() is checking for existing mounts first, it does that
by using container.HasMountFor() function which only checks container.Mounts
but not container.Tmpfs.

Ultimately, the solution is to get rid of container.Tmpfs (moving its
data to container.Mounts). Current workaround is to add checking
of container.Tmpfs into container.HasMountFor().

A unit test case is included.

Unfortunately we can't call daemon.createSpec() from a unit test,
as the code relies a lot on various daemon structures to be initialized
properly, and it is hard to achieve. Therefore, we minimally mimick
the code flow of daemon.createSpec() -- barely enough to reproduce
the issue.

https://github.com/moby/moby/issues/35455

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
---
 container/container_unix.go | 13 +++++++++-
 daemon/oci_linux_test.go    | 50 +++++++++++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+), 1 deletion(-)
 create mode 100644 daemon/oci_linux_test.go

diff --git a/container/container_unix.go b/container/container_unix.go
index 2e1b95f404..28a3722c79 100644
--- a/container/container_unix.go
+++ b/container/container_unix.go
@@ -157,7 +157,18 @@ func (container *Container) ShmResourcePath() (string, error) {
 // HasMountFor checks if path is a mountpoint
 func (container *Container) HasMountFor(path string) bool {
 	_, exists := container.MountPoints[path]
-	return exists
+	if exists {
+		return true
+	}
+
+	// Also search among the tmpfs mounts
+	for dest := range container.HostConfig.Tmpfs {
+		if dest == path {
+			return true
+		}
+	}
+
+	return false
 }
 
 // UnmountIpcMount uses the provided unmount function to unmount shm if it was mounted
diff --git a/daemon/oci_linux_test.go b/daemon/oci_linux_test.go
new file mode 100644
index 0000000000..f2f455f9c6
--- /dev/null
+++ b/daemon/oci_linux_test.go
@@ -0,0 +1,50 @@
+package daemon
+
+import (
+	"testing"
+
+	containertypes "github.com/docker/docker/api/types/container"
+	"github.com/docker/docker/container"
+	"github.com/docker/docker/daemon/config"
+	"github.com/docker/docker/oci"
+	"github.com/docker/docker/pkg/idtools"
+
+	"github.com/stretchr/testify/assert"
+)
+
+// TestTmpfsDevShmNoDupMount checks that a user-specified /dev/shm tmpfs
+// mount (as in "docker run --tmpfs /dev/shm:rw,size=NNN") does not result
+// in "Duplicate mount point" error from the engine.
+// https://github.com/moby/moby/issues/35455
+func TestTmpfsDevShmNoDupMount(t *testing.T) {
+	d := Daemon{
+		// some empty structs to avoid getting a panic
+		// caused by a null pointer dereference
+		idMappings:  &idtools.IDMappings{},
+		configStore: &config.Config{},
+	}
+	c := &container.Container{
+		ShmPath: "foobar", // non-empty, for c.IpcMounts() to work
+		HostConfig: &containertypes.HostConfig{
+			IpcMode: containertypes.IpcMode("shareable"), // default mode
+			// --tmpfs /dev/shm:rw,exec,size=NNN
+			Tmpfs: map[string]string{
+				"/dev/shm": "rw,exec,size=1g",
+			},
+		},
+	}
+
+	// Mimick the code flow of daemon.createSpec(), enough to reproduce the issue
+	ms, err := d.setupMounts(c)
+	assert.NoError(t, err)
+
+	ms = append(ms, c.IpcMounts()...)
+
+	tmpfsMounts, err := c.TmpfsMounts()
+	assert.NoError(t, err)
+	ms = append(ms, tmpfsMounts...)
+
+	s := oci.DefaultSpec()
+	err = setMounts(&d, &s, c, ms)
+	assert.NoError(t, err)
+}