Fixes to handle /dev/shm correctly.

We had two problems with /dev/shm, first, you mount the
container read/only then /dev/shm was mounted read/only.
This is a bug a tmpfs directory should be read/write within
a read-only container.

The second problem is we were ignoring users mounted /dev/shm
from the host.

If user specified

podman run -d -v /dev/shm:/dev/shm ...

We were dropping this mount and still using the internal mount.

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>

5 files changed, 13 insertions, 6 deletions

diff --git a/libpod/container_internal.go b/libpod/container_internal.go
index 3217b2421..928be52ae 100644
--- a/libpod/container_internal.go
+++ b/libpod/container_internal.go
@@ -851,7 +851,7 @@ func (c *Container) mountStorage() (string, error) {
 		return "", errors.Wrapf(err, "unable to determine if %q is mounted", c.config.ShmDir)
 	}
 
-	if !mounted {
+	if !mounted && !MountExists(c.config.Spec.Mounts, "/dev/shm") {
 		shmOptions := fmt.Sprintf("mode=1777,size=%d", c.config.ShmSize)
 		if err := c.mountSHM(shmOptions); err != nil {
 			return "", err
diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go
index 93d20491e..0745b7732 100644
--- a/libpod/container_internal_linux.go
+++ b/libpod/container_internal_linux.go
@@ -210,9 +210,6 @@ func (c *Container) generateSpec(ctx context.Context) (*spec.Spec, error) {
 
 	g.SetProcessSelinuxLabel(c.ProcessLabel())
 	g.SetLinuxMountLabel(c.MountLabel())
-	// Remove the default /dev/shm mount to ensure we overwrite it
-	g.RemoveMount("/dev/shm")
-
 	// Add bind mounts to container
 	for dstPath, srcPath := range c.state.BindMounts {
 		newMount := spec.Mount{
@@ -221,7 +218,7 @@ func (c *Container) generateSpec(ctx context.Context) (*spec.Spec, error) {
 			Destination: dstPath,
 			Options:     []string{"bind", "private"},
 		}
-		if c.IsReadOnly() {
+		if c.IsReadOnly() && dstPath != "/dev/shm" {
 			newMount.Options = append(newMount.Options, "ro")
 		}
 		if !MountExists(g.Mounts(), dstPath) {
diff --git a/libpod/runtime_ctr.go b/libpod/runtime_ctr.go
index c448f8cb1..a9095c6d1 100644
--- a/libpod/runtime_ctr.go
+++ b/libpod/runtime_ctr.go
@@ -180,7 +180,8 @@ func (r *Runtime) newContainer(ctx context.Context, rSpec *spec.Spec, options ..
 	if ctr.config.LogPath == "" {
 		ctr.config.LogPath = filepath.Join(ctr.config.StaticDir, "ctr.log")
 	}
-	if ctr.config.ShmDir == "" {
+
+	if !MountExists(ctr.config.Spec.Mounts, "/dev/shm") && ctr.config.ShmDir == "" {
 		if ctr.state.UserNSRoot == "" {
 			ctr.config.ShmDir = filepath.Join(ctr.bundlePath(), "shm")
 		} else {
diff --git a/pkg/spec/spec.go b/pkg/spec/spec.go
index c90f16f7c..3d6603364 100644
--- a/pkg/spec/spec.go
+++ b/pkg/spec/spec.go
@@ -52,6 +52,8 @@ func CreateConfigToOCISpec(config *CreateConfig) (*spec.Spec, error) { //nolint
 	if err != nil {
 		return nil, err
 	}
+	// Remove the default /dev/shm mount to ensure we overwrite it
+	g.RemoveMount("/dev/shm")
 	g.HostSpecific = true
 	addCgroup := true
 	canMountSys := true
diff --git a/test/e2e/run_test.go b/test/e2e/run_test.go
index d73d8662f..cf366b197 100644
--- a/test/e2e/run_test.go
+++ b/test/e2e/run_test.go
@@ -669,4 +669,11 @@ USER mail`
 		numContainers := podmanTest.NumberOfContainers()
 		Expect(numContainers).To(Equal(1))
 	})
+	It("podman run readonly container should NOT mount /dev/shm read/only", func() {
+		session := podmanTest.Podman([]string{"run", "--read-only", ALPINE, "mount"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+
+		Expect(session.OutputToString()).To(Not(ContainSubstring("/dev/shm type tmpfs (ro,")))
+	})
 })