merge branch 'pr-3276'

Kir Kolyshkin (2):
  runc run: fix ro /dev
  test/int/mount.bats: refer to github issue

LGTMs: thaJeztah cyphar
Closes #3276

Author: cyphar

libcontainer/rootfs_linux.go

@@ -164,15 +164,16 @@ func prepareRootfs(pipe io.ReadWriter, iConfig *initConfig, mountFds []int) (err
 // 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 utils.CleanPath(m.Destination) == "/dev" {
-			if m.Flags&unix.MS_RDONLY == unix.MS_RDONLY {
-				if err := remountReadonly(m); err != nil {
-					return err
-				}
+		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 err
 			}
-			break
 		}
 	}
 
@@ -452,12 +453,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, mountFd); err != nil {
-				return err
-			}
-		}
 		return nil
 	case "bind":
 		if err := prepareBindMount(m, rootfs, mountFd); err != nil {
@@ -1092,10 +1087,10 @@ func mountPropagate(m *configs.Mount, rootfs string, mountLabel string, mountFd
 		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 utils.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