Merge pull request #3277 from kolyshkin/1.0-fix-ro-dev

[1.0] Fix failure with read-only /dev in spec

libcontainer/rootfs_linux.go

@@ -22,7 +22,6 @@ import (
 	"github.com/opencontainers/runc/libcontainer/devices"
 	"github.com/opencontainers/runc/libcontainer/userns"
 	"github.com/opencontainers/runc/libcontainer/utils"
-	libcontainerUtils "github.com/opencontainers/runc/libcontainer/utils"
 	"github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/opencontainers/selinux/go-selinux/label"
 	"github.com/sirupsen/logrus"
@@ -42,7 +41,7 @@ type mountConfig struct {
 // needsSetupDev returns true if /dev needs to be set up.
 func needsSetupDev(config *configs.Config) bool {
 	for _, m := range config.Mounts {
-		if m.Device == "bind" && libcontainerUtils.CleanPath(m.Destination) == "/dev" {
+		if m.Device == "bind" && utils.CleanPath(m.Destination) == "/dev" {
 			return false
 		}
 	}
@@ -154,15 +153,16 @@ func prepareRootfs(pipe io.ReadWriter, iConfig *initConfig) (err error) {
 // finalizeRootfs sets anything to ro if necessary. You must call
 // prepareRootfs first.
 func finalizeRootfs(config *configs.Config) (err error) {
-	// remount dev as ro if specified
+	// All tmpfs mounts and /dev were previously mounted as rw
+	// by mountPropagate. Remount them read-only as requested.
 	for _, m := range config.Mounts {
-		if libcontainerUtils.CleanPath(m.Destination) == "/dev" {
-			if m.Flags&unix.MS_RDONLY == unix.MS_RDONLY {
-				if err := remountReadonly(m); err != nil {
-					return newSystemErrorWithCausef(err, "remounting %q as readonly", m.Destination)
-				}
+		if m.Flags&unix.MS_RDONLY != unix.MS_RDONLY {
+			continue
+		}
+		if m.Device == "tmpfs" || utils.CleanPath(m.Destination) == "/dev" {
+			if err := remountReadonly(m); err != nil {
+				return newSystemErrorWithCausef(err, "remounting %q as readonly", m.Destination)
 			}
-			break
 		}
 	}
 
@@ -432,12 +432,6 @@ func mountToRootfs(m *configs.Mount, c *mountConfig) error {
 				return err
 			}
 		}
-		// Initially mounted rw in mountPropagate, remount to ro if flag set.
-		if m.Flags&unix.MS_RDONLY != 0 {
-			if err := remount(m, rootfs); err != nil {
-				return err
-			}
-		}
 		return nil
 	case "bind":
 		if err := prepareBindMount(m, rootfs); err != nil {
@@ -1062,10 +1056,10 @@ func mountPropagate(m *configs.Mount, rootfs string, mountLabel string) error {
 		flags = m.Flags
 	)
 	// Delay mounting the filesystem read-only if we need to do further
-	// operations on it. We need to set up files in "/dev" and tmpfs mounts may
-	// need to be chmod-ed after mounting. The mount will be remounted ro later
-	// in finalizeRootfs() if necessary.
-	if libcontainerUtils.CleanPath(m.Destination) == "/dev" || m.Device == "tmpfs" {
+	// operations on it. We need to set up files in "/dev", and other tmpfs
+	// mounts may need to be chmod-ed after mounting. These mounts will be
+	// remounted ro later in finalizeRootfs(), if necessary.
+	if m.Device == "tmpfs" || utils.CleanPath(m.Destination) == "/dev" {
 		flags &= ^unix.MS_RDONLY
 	}
 

tests/integration/mounts.bats

@@ -23,6 +23,7 @@ function teardown() {
 	[[ "${lines[0]}" == *'/tmp/bind/config.json'* ]]
 }
 
+# https://github.com/opencontainers/runc/issues/2246
 @test "runc run [ro tmpfs mount]" {
 	update_config '	  .mounts += [{
 					source: "tmpfs",
@@ -37,6 +38,16 @@ function teardown() {
 	[[ "${lines[0]}" == *'ro,'* ]]
 }
 
+# https://github.com/opencontainers/runc/issues/3248
+@test "runc run [ro /dev mount]" {
+	update_config '   .mounts |= map((select(.destination == "/dev") | .options += ["ro"]) // .)
+			| .process.args |= ["grep", "^tmpfs /dev", "/proc/mounts"]'
+
+	runc run test_busybox
+	[ "$status" -eq 0 ]
+	[[ "${lines[0]}" == *'ro,'* ]]
+}
+
 # https://github.com/opencontainers/runc/issues/2683
 @test "runc run [tmpfs mount with absolute symlink]" {
 	# in container, /conf -> /real/conf