From df99522c6794cbf9ea77c4c314dadf9e9b5b2a54 Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Mon, 24 Dec 2018 06:55:24 -0500 Subject: 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 --- libpod/container_internal.go | 2 +- libpod/container_internal_linux.go | 5 +---- libpod/runtime_ctr.go | 3 ++- pkg/spec/spec.go | 2 ++ test/e2e/run_test.go | 7 +++++++ 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,"))) + }) }) -- cgit v1.2.3-54-g00ecf