--read-write-tmpfs=false should set /dev/* tmpfs to readonly

Change the --read-only-tmpfs option to --read-write-tmpfs since
this makes sense and the old name was doing the exact opposite.

Fixes: #12937

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

cmd/podman/common/create.go

@@ -381,8 +381,8 @@ func DefineCreateFlags(cmd *cobra.Command, cf *entities.ContainerCreateOptions,
 			"Make containers root filesystem read-only",
 		)
 		createFlags.BoolVar(
-			&cf.ReadOnlyTmpFS,
-			"read-only-tmpfs", cf.ReadOnlyTmpFS,
+			&cf.ReadWriteTmpFS,
+			"read-write-tmpfs", cf.ReadWriteTmpFS,
 			"When running containers in read-only mode mount a read-write tmpfs on /run, /tmp and /var/tmp",
 		)
 		requiresFlagName := "requires"

cmd/podman/common/create_opts.go

@@ -83,7 +83,7 @@ func DefineCreateDefaults(opts *entities.ContainerCreateOptions) {
 	opts.MemorySwappiness = -1
 	opts.ImageVolume = podmanConfig.ContainersConfDefaultsRO.Engine.ImageVolumeMode
 	opts.Pull = policy()
-	opts.ReadOnlyTmpFS = true
+	opts.ReadWriteTmpFS = true
 	opts.SdNotifyMode = define.SdNotifyModeContainer
 	opts.StopTimeout = podmanConfig.ContainersConfDefaultsRO.Engine.StopTimeout
 	opts.Systemd = "true"

cmd/podman/containers/create.go

@@ -114,6 +114,11 @@ func commonFlags(cmd *cobra.Command) error {
 	if cmd.Flags().Changed("image-volume") {
 		cliVals.ImageVolume = cmd.Flag("image-volume").Value.String()
 	}
+
+	if cmd.Flags().Changed("read-write-tmpfs") && !cliVals.ReadOnly {
+		return errors.New("--read-write-tmpfs is only used if --read-only=true")
+	}
+
 	return nil
 }
 

cmd/podman/utils/alias.go

@@ -33,6 +33,8 @@ func AliasFlags(f *pflag.FlagSet, name string) pflag.NormalizedName {
 		name = "os"
 	case "override-variant":
 		name = "variant"
+	case "read-only-tmpfs":
+		name = "read-write-tmpfs"
 	}
 	return pflag.NormalizedName(name)
 }

docs/source/markdown/options/read-write-tmpfs.md

@@ -0,0 +1,8 @@
+####> This option file is used in:
+####>   podman create, run
+####> If you edit this file, make sure your changes
+####> are applicable to all of those.
+#### **--read-write-tmpfs**
+
+If container is running in --read-only mode, then mount a read-write tmpfs on /run, /tmp, and /var/tmp. When false, Podman the entire container has no tmpfs that are read/write unless specified on the command line.  Podman mounts /dev, /dev/mqueue, /dev/pts, /dev/shm as read only. The default is *true*
+

docs/source/markdown/podman-create.1.md.in

@@ -293,7 +293,7 @@ Suppress output information when pulling images
 
 @@option read-only
 
-@@option read-only-tmpfs
+@@option read-write-tmpfs
 
 @@option replace
 

docs/source/markdown/podman-run.1.md.in

@@ -317,7 +317,7 @@ Suppress output information when pulling images
 
 @@option read-only
 
-@@option read-only-tmpfs
+@@option read-write-tmpfs
 
 @@option replace
 
@@ -477,7 +477,7 @@ tmpfs directories on _/run_ and _/tmp_.
 ```
 $ podman run --read-only -i -t fedora /bin/bash
 
-$ podman run --read-only --read-only-tmpfs=false --tmpfs /run -i -t fedora /bin/bash
+$ podman run --read-only --read-write-tmpfs=false --tmpfs /run -i -t fedora /bin/bash
 ```
 
 ### Exposing log messages from the container to the host's log

pkg/api/handlers/compat/containers_create.go

@@ -399,7 +399,7 @@ func cliOpts(cc handlers.CreateContainerConfig, rtc *config.Config) (*entities.C
 		PublishAll:        cc.HostConfig.PublishAllPorts,
 		Quiet:             false,
 		ReadOnly:          cc.HostConfig.ReadonlyRootfs,
-		ReadOnlyTmpFS:     true, // podman default
+		ReadWriteTmpFS:    true, // podman default
 		Rm:                cc.HostConfig.AutoRemove,
 		SecurityOpt:       cc.HostConfig.SecurityOpt,
 		StopSignal:        cc.Config.StopSignal,

pkg/domain/entities/pods.go

@@ -249,7 +249,7 @@ type ContainerCreateOptions struct {
 	Pull              string
 	Quiet             bool
 	ReadOnly          bool
-	ReadOnlyTmpFS     bool
+	ReadWriteTmpFS    bool
 	Restart           string
 	Replace           bool
 	Requires          []string
@@ -303,6 +303,7 @@ func NewInfraContainerCreateOptions() ContainerCreateOptions {
 		IsInfra:          true,
 		ImageVolume:      "bind",
 		MemorySwappiness: -1,
+		ReadWriteTmpFS:   true,
 	}
 	return options
 }

pkg/specgen/specgen.go

@@ -384,6 +384,9 @@ type ContainerSecurityConfig struct {
 	// ReadOnlyFilesystem indicates that everything will be mounted
 	// as read-only
 	ReadOnlyFilesystem bool `json:"read_only_filesystem,omitempty"`
+	// ReadWriteTmpfs indicates that tmpfs will be mounted
+	// as read-only
+	ReadWriteTmpFS bool `json:"read_write_tmpfs,omitempty"`
 	// Umask is the umask the init process of the container will be run with.
 	Umask string `json:"umask,omitempty"`
 	// ProcOpts are the options used for the proc mount.

pkg/specgenutil/specgen.go

@@ -566,20 +566,17 @@ func FillOutSpecGen(s *specgen.SpecGenerator, c *entities.ContainerCreateOptions
 	if !s.ReadOnlyFilesystem {
 		s.ReadOnlyFilesystem = c.ReadOnly
 	}
+	s.ReadWriteTmpFS = c.ReadWriteTmpFS
 	if len(s.ConmonPidFile) == 0 || len(c.ConmonPIDFile) != 0 {
 		s.ConmonPidFile = c.ConmonPIDFile
 	}
-
 	if len(s.DependencyContainers) == 0 || len(c.Requires) != 0 {
 		s.DependencyContainers = c.Requires
 	}
 
 	// TODO
 	// outside of specgen and oci though
 	// defaults to true, check spec/storage
-	// s.readonly = c.ReadOnlyTmpFS
-	//  TODO convert to map?
-	// check if key=value and convert
 	sysmap := make(map[string]string)
 	for _, ctl := range c.Sysctl {
 		splitCtl := strings.SplitN(ctl, "=", 2)
@@ -657,7 +654,7 @@ func FillOutSpecGen(s *specgen.SpecGenerator, c *entities.ContainerCreateOptions
 
 	// Only add read-only tmpfs mounts in case that we are read-only and the
 	// read-only tmpfs flag has been set.
-	mounts, volumes, overlayVolumes, imageVolumes, err := parseVolumes(c.Volume, c.Mount, c.TmpFS, c.ReadOnlyTmpFS && c.ReadOnly)
+	mounts, volumes, overlayVolumes, imageVolumes, err := parseVolumes(c.Volume, c.Mount, c.TmpFS, c.ReadWriteTmpFS && c.ReadOnly)
 	if err != nil {
 		return err
 	}

pkg/specgenutil/volumes.go

@@ -26,7 +26,7 @@ var (
 // Does not handle image volumes, init, and --volumes-from flags.
 // Can also add tmpfs mounts from read-only tmpfs.
 // TODO: handle options parsing/processing via containers/storage/pkg/mount
-func parseVolumes(volumeFlag, mountFlag, tmpfsFlag []string, addReadOnlyTmpfs bool) ([]spec.Mount, []*specgen.NamedVolume, []*specgen.OverlayVolume, []*specgen.ImageVolume, error) {
+func parseVolumes(volumeFlag, mountFlag, tmpfsFlag []string, addReadWriteTmpfs bool) ([]spec.Mount, []*specgen.NamedVolume, []*specgen.OverlayVolume, []*specgen.ImageVolume, error) {
 	// Get mounts from the --mounts flag.
 	unifiedMounts, unifiedVolumes, unifiedImageVolumes, err := Mounts(mountFlag)
 	if err != nil {
@@ -79,10 +79,10 @@ func parseVolumes(volumeFlag, mountFlag, tmpfsFlag []string, addReadOnlyTmpfs bo
 	}
 
 	// If requested, add tmpfs filesystems for read-only containers.
-	if addReadOnlyTmpfs {
-		readonlyTmpfs := []string{"/tmp", "/var/tmp", "/run"}
+	if addReadWriteTmpfs {
+		tmpfs := []string{"/tmp", "/var/tmp", "/run"}
 		options := []string{"rw", "rprivate", "nosuid", "nodev", "tmpcopyup"}
-		for _, dest := range readonlyTmpfs {
+		for _, dest := range tmpfs {
 			if _, ok := unifiedMounts[dest]; ok {
 				continue
 			}

test/e2e/container_clone_test.go

@@ -2,6 +2,7 @@ package integration
 
 import (
 	"os"
+	"strings"
 
 	. "github.com/containers/podman/v4/test/utils"
 	. "github.com/onsi/ginkgo"
@@ -51,6 +52,11 @@ var _ = Describe("Podman container clone", func() {
 		Expect(ctrInspect).To(Exit(0))
 		Expect(ctrInspect.InspectContainerToJSON()[0].Name).To(ContainSubstring("-clone1"))
 
+		// FIXME when new crun reaches Ubuntu
+		if podmanTest.Host.Distribution == "ubuntu" &&
+			strings.Contains(podmanTest.OCIRuntime, "crun") {
+			Skip("The version of crun shipped by ubuntu is too old. Needs crun-1.4.2 or greater")
+		}
 		ctrStart := podmanTest.Podman([]string{"container", "start", clone.OutputToString()})
 		ctrStart.WaitWithDefaultTimeout()
 		Expect(ctrStart).To(Exit(0))
@@ -164,12 +170,18 @@ var _ = Describe("Podman container clone", func() {
 
 	It("podman container clone in a pod", func() {
 		SkipIfRootlessCgroupsV1("starting a container with the memory limits not supported")
+		// FIXME when new crun reaches Ubuntu
+		if podmanTest.Host.Distribution == "ubuntu" &&
+			strings.Contains(podmanTest.OCIRuntime, "crun") {
+			Skip("The version of crun shipped by ubuntu is too old. Needs crun-1.4.2 or greater")
+		}
 		run := podmanTest.Podman([]string{"run", "-dt", "--pod", "new:1234", ALPINE, "sleep", "20"})
 		run.WaitWithDefaultTimeout()
 		Expect(run).To(Exit(0))
 		clone := podmanTest.Podman([]string{"container", "clone", run.OutputToString()})
 		clone.WaitWithDefaultTimeout()
 		Expect(clone).To(Exit(0))
+
 		ctrStart := podmanTest.Podman([]string{"container", "start", clone.OutputToString()})
 		ctrStart.WaitWithDefaultTimeout()
 		Expect(ctrStart).To(Exit(0))

test/system/030-run.bats

@@ -832,6 +832,25 @@ EOF
     is "${lines[0]}" ".*${HOST}.*"
 }
 
+@test "podman run --read-write-tmpfs" {
+    # FIXME once ubuntu gets a newer crun
+    if is_ubuntu; then
+        if [ $(podman_runtime) == "crun" ]; then
+	   skip "The version of crun shipped by ubuntu is too old. Needs crun-1.4.2 or greater"
+	fi
+    fi
+    HOST=$(random_string 25)
+    run_podman 1 run --rm --read-only $IMAGE touch /foo
+    is "$output" "touch: /foo: Read-only file system" "Should fail with read only file system"
+
+    for rwdir in /run /tmp /var/tmp /dev /dev/shm; do
+        run_podman run --rm --read-only $IMAGE touch ${rwdir}/foo
+        run_podman run --rm --read-only --read-write-tmpfs=true $IMAGE touch ${rwdir}/foo
+	run_podman 1 run --rm --read-only --read-write-tmpfs=false $IMAGE touch ${rwdir}/foo
+	is "$output" "touch: ${rwdir}/foo: Read-only file system" "Should fail with ${rwdir}/foo read only file system"
+     done
+}
+
 @test "podman run doesn't override oom-score-adj" {
     current_oom_score_adj=$(cat /proc/self/oom_score_adj)
     run_podman run --rm $IMAGE cat /proc/self/oom_score_adj