fix rootfs propagation mode to shared / unbindable #4724

saku3 · 2025-04-13T01:32:13Z

This PR adds support for applying mount propagation settings (MS_SHARED or MS_UNBINDABLE) to the container root based on the value of config.RootPropagation.
We apply mount propagation after executing pivot_root and rootfsParentMountPrivate

Fixes #1755

Related:
#1815
youki-dev/youki#3141

Signed-off-by: Yusuke Sakurai yusuke.sakurai@3-shake.com

AkihiroSuda · 2025-04-16T06:44:17Z

tests/integration/mounts_propagation.bats

+        # Set rootfsPropagation to shared
+        update_config ' .linux.rootfsPropagation = "shared" '
+
+        update_config ' .process.args = ["sh", "-c", "findmnt --noheadings -o PROPAGATION /"] '


No need to wrap findmnt in sh

AkihiroSuda · 2025-04-16T06:44:44Z

tests/integration/mounts_propagation.bats

+}
+
+@test "runc run [rootfsPropagation shared]" {
+        # Set rootfsPropagation to shared


This is obvious from the code and does not need to be explained in a code comment line

AkihiroSuda · 2025-04-16T06:45:16Z

tests/integration/mounts_propagation.bats

+        # Run the container and capture the output
+        runc run test_shared_rootfs
+        [ "$status" -eq 0 ]
+        [[ "$output" == *"shared"* ]]


Suggested change

[[ "$output" == *"shared"* ]]

[ "$output" == "shared" ]

strictly speaking this should be

[ "$output" = "shared" ]

(i.e. single =), since test (aka [) only support = for string comparison (unlike [[ which supports ==).

AkihiroSuda · 2025-04-16T06:49:44Z

libcontainer/rootfs_linux.go

@@ -232,6 +236,27 @@ func prepareRootfs(pipe *syncSocket, iConfig *initConfig) (err error) {
 	return nil
 }

+// adjustRootMountPropagation applies mount propagation flags such as MS_SHARED or MS_UNBINDABLE
+// to the root mount after the pivot_root or chroot has taken place.


Needs a comment to explain why this cannot be done in prepareRoot:

runc/libcontainer/rootfs_linux.go

Lines 1012 to 1027 in 3d8a278

func prepareRoot(config *configs.Config) error {

flag := unix.MS_SLAVE | unix.MS_REC

if config.RootPropagation != 0 {

flag = config.RootPropagation

}

if err := mount("", "/", "", uintptr(flag), ""); err != nil {

return err

}

if err := rootfsParentMountPrivate(config.Rootfs); err != nil {

return err

}

return mount(config.Rootfs, config.Rootfs, "bind", unix.MS_BIND|unix.MS_REC, "")

}

I have added a comment.

rata · 2025-04-16T10:22:48Z

This issue has a lot of history, I wonder if @cyphar or @kolyshkin would like to take a look

AkihiroSuda · 2025-04-16T14:26:24Z

Please squash and sign off the commits

rata · 2025-05-05T09:38:00Z

This sounds reasonable to me. Do we know what crun is doing?

Also, @cyphar , do you want to take a look?

saku3 · 2025-05-09T00:58:35Z

The behavior of shared mount propagation, as it relates to both crun and runc, can be demonstrated using the following steps, as outlined in the reference below.
opencontainers/runc#1815

Runc

$ podman info --format {{.Host.OCIRuntime.Version}}
runc version 1.3.0-rc.1+dev  
commit: bb317a03 
spec: 1.2.1  
go: go1.23.0  
libseccomp: 2.5.5

$ mkdir ~/hoge && mount -t tmpfs tmpfs ~/hoge
$ podman run -itd --privileged --name testcon --volume ~/hoge:/hoge:z,shared fedora-minimal:42 /bin/bash
babefd2d44a9d237a236482ddf593b578e8d56225c3f42220c582e484a779620
$ cat /var/lib/containers/storage/overlay-containers/babefd2d44a9d237a236482ddf593b578e8d56225c3f42220c582e484a779620/userdata/config.json | jq .linux.rootfsPropagation
"shared"
$ podman exec testcon mkdir /test
$ podman exec testcon mount -t tmpfs tmpfs /test
$ podman exec testcon findmnt -o "TARGET,PROPAGATION"
TARGET               PROPAGATION  
/                    shared  
|-/test              shared  
|-/hoge              shared  
|-/proc              private  
|-/dev               private  
| |-/dev/console     private  
| |-/dev/pts         private  
| |-/dev/mqueue      private  
| `-/dev/shm         private  
|-/sys               private  
| `-/sys/fs/cgroup   private  
|-/etc/resolv.conf   private  
|-/etc/hosts         private  
|-/run/.containerenv private  
`-/etc/hostname      private

With crun

$ podman info --format {{.Host.OCIRuntime.Version}}
crun version 1.14.1  
commit: de537a7965bfbe9992e2cfae0baeb56a08128171  
rundir: /run/crun  
spec: 1.0.0  
+SYSTEMD +SELINUX +APPARMOR +CAP +SECCOMP +EBPF +WASM:wasmedge +YAJL

$ podman run -itd --privileged --name testcon --volume ~/hoge:/hoge:z,shared fedora-minimal:42 /bin/bash
90a94c7a21000c8a83389ae085c4846a505da2e2891ab25f5b0e325f79de1f62
$ cat /var/lib/containers/storage/overlay-containers/90a94c7a21000c8a83389ae085c4846a505da2e2891ab25f5b0e325f79de1f62/userdata/config.json | jq .linux.rootfsPropagation
"shared"
$ podman exec testcon mkdir /test
$ podman exec testcon mount -t tmpfs tmpfs /test
$ podman exec testcon findmnt -o "TARGET,PROPAGATION"
TARGET               PROPAGATION  
/                    shared  
|-/test              shared  
|-/hoge              shared  
|-/proc              private  
|-/dev               private  
| |-/dev/console     private  
| |-/dev/pts         private  
| |-/dev/mqueue      private  
| `-/dev/shm         private  
|-/sys               private  
| `-/sys/fs/cgroup   private  
|-/run/.containerenv private  
|-/etc/hostname      private  
|-/etc/resolv.conf   private  
`-/etc/hosts         private

As shown above, the result of findmnt is identical between crun and runc (with the changes from this PR), confirming consistent behavior regarding shared mount propagation.

kolyshkin

LGTM, thanks!

lifubang · 2025-05-15T01:56:26Z

libcontainer/rootfs_linux.go

+		return nil
+	}
+
+	if err := unix.Mount("", "/", "", flag, ""); err != nil {


Why not use rootPropagation directly?

kolyshkin · 2025-05-15T23:26:50Z

libcontainer/rootfs_linux.go

+	if err := adjustRootMountPropagation(uintptr(config.RootPropagation)); err != nil {
+		return err
+	}
+


Yeah, I think that we can use config.RootPropagation directly, and not use a separate function.

Something like

// Apply root mount propagation flags. if config.RootPropagation != 0 { if err := mount("", "/", "", uintptr(flag), ""); err != nil { return fmt.Errorf("unable to apply root propagation flags: %w", err ) } }

@kolyshkin @lifubang
Thank you for the review. I've fixed.

lifubang · 2025-05-19T06:04:06Z

libcontainer/rootfs_linux.go

+	// to the current root ("/"), and not to the old rootfs before it becomes "/". Applying the
+	// flag in prepareRoot would affect the host mount namespace if the container's
+	// root mount is shared.
+	if config.RootPropagation != 0 {


I think if we add a unix.MS_PRIVATE comparison, it will reduce a mount syscall.

if config.RootPropagation != 0 && config.RootPropagation & unix.MS_PRIVATE == 0 {

Signed-off-by: Yusuke Sakurai <yusuke.sakurai@3-shake.com>

AkihiroSuda reviewed Apr 16, 2025

View reviewed changes

saku3 force-pushed the fix-rootfspropagation branch 3 times, most recently from 24745a2 to 897ae54 Compare April 18, 2025 03:53

saku3 force-pushed the fix-rootfspropagation branch from 897ae54 to bb317a0 Compare May 1, 2025 22:03

AkihiroSuda approved these changes May 9, 2025

View reviewed changes

kolyshkin approved these changes May 14, 2025

View reviewed changes

lifubang reviewed May 15, 2025

View reviewed changes

kolyshkin self-requested a review May 15, 2025 23:13

kolyshkin reviewed May 15, 2025

View reviewed changes

saku3 force-pushed the fix-rootfspropagation branch from bb317a0 to 1b28a8a Compare May 16, 2025 23:33

lifubang reviewed May 19, 2025

View reviewed changes

saku3 force-pushed the fix-rootfspropagation branch from 1b28a8a to ecec73d Compare May 19, 2025 12:34

fix rootfs propagation mode

04be81b

Signed-off-by: Yusuke Sakurai <yusuke.sakurai@3-shake.com>

saku3 force-pushed the fix-rootfspropagation branch from ecec73d to 04be81b Compare May 19, 2025 12:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix rootfs propagation mode to shared / unbindable #4724

fix rootfs propagation mode to shared / unbindable #4724

saku3 commented Apr 13, 2025

AkihiroSuda Apr 16, 2025

AkihiroSuda Apr 16, 2025

AkihiroSuda Apr 16, 2025

kolyshkin Apr 17, 2025

AkihiroSuda Apr 16, 2025

saku3 Apr 16, 2025

rata commented Apr 16, 2025

AkihiroSuda commented Apr 16, 2025

rata commented May 5, 2025

saku3 commented May 9, 2025

kolyshkin left a comment

lifubang May 15, 2025

kolyshkin May 15, 2025

saku3 May 16, 2025

lifubang May 19, 2025

saku3 May 19, 2025

	func prepareRoot(config *configs.Config) error {
	flag := unix.MS_SLAVE \| unix.MS_REC
	if config.RootPropagation != 0 {
	flag = config.RootPropagation
	}
	if err := mount("", "/", "", uintptr(flag), ""); err != nil {
	return err
	}

	if err := rootfsParentMountPrivate(config.Rootfs); err != nil {
	return err
	}

	return mount(config.Rootfs, config.Rootfs, "bind", unix.MS_BIND\|unix.MS_REC, "")
	}

fix rootfs propagation mode to shared / unbindable #4724

Are you sure you want to change the base?

fix rootfs propagation mode to shared / unbindable #4724

Conversation

saku3 commented Apr 13, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rata commented Apr 16, 2025

AkihiroSuda commented Apr 16, 2025

rata commented May 5, 2025

saku3 commented May 9, 2025

Runc

With crun

kolyshkin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment