runc run: fix readonly path error for rootless + host pidns #2897

kolyshkin · 2021-04-08T00:00:59Z

Currently, runc fails like this when used from rootless podman
with host PID namespace:

$ podman --runtime=runc run --pid=host --rm -it busybox sh
WARN[0000] additional gid=10 is not present in the user namespace, skip setting it
Error: container_linux.go:380: starting container process caused:
process_linux.go:545: container init caused: readonly path /proc/asound:
operation not permitted: OCI permission denied

(Here /proc/asound is the first path from OCI spec's readonlyPaths).

The code uses MS_BIND|MS_REMOUNT flags that have a special meaning in
the kernel ("keep the flags like nodev, nosuid, noexec as is").
For some reason, this "special meaning" trick is not working for the
above use case (rootless + host PID namespace)~~, and I don't know~~
~~how to reproduce this without podman~~.

Instead of relying on the kernel feature, let's just get the current
mount flags using fstatfs(2) and add these. This fixes the issue observed.

Add a repro test case which fails like this (before the fix):

not ok 5 runc run [rootless with host pidns]
# (in test file tests/integration/start_hello.bats, line 78)
#   `[ "$status" -eq 0 ]' failed
# runc spec --rootless (status=0):
# 
# runc run test_hello (status=1):
# time="2021-04-14T17:12:35-07:00" level=warning msg="unable to get oom kill count" error="openat2 /sys/fs/cgroup/user.slice/user-1000.slice/[email protected]/test_hello/memory.events: no such file or directory"
# time="2021-04-14T17:12:35-07:00" level=warning msg="freezer not supported: openat2 /sys/fs/cgroup/user.slice/user-1000.slice/[email protected]/test_hello/cgroup.freeze: no such file or directory"
# time="2021-04-14T17:12:35-07:00" level=warning msg="lstat /sys/fs/cgroup/user.slice/user-1000.slice/[email protected]/test_hello: no such file or directory"
# time="2021-04-14T17:12:35-07:00" level=error msg="container_linux.go:380: starting container process caused: process_linux.go:545: container init caused

(the warnings are there because of rootless with no cgroup access)

Originally reported in https://bugzilla.redhat.com/show_bug.cgi?id=1947432

kolyshkin · 2021-04-08T00:02:24Z

The issue is easy to repro using any recent runc (I tried rc91 up to latest git) and podman >= 3

$ podman --runtime=runc run --pid=host --rm -it busybox sh

TomSweeneyRedHat · 2021-04-08T00:13:48Z

@mheon PTAL

mrunalp · 2021-04-08T00:46:03Z

@giuseppe ptal

cyphar · 2021-04-08T05:41:26Z

My guess as to why you can't reproduce this outside of podman is because I fixed this bug on the Docker side some time ago. moby/moby#35205 From memory there was a particular reason why I opted to solve this in Docker rather than runc -- I believe it's because I felt that it was unclear whether runc should be overriding the flags specified by the caller, but it's been ~4 years so my memory of this bug is quite foggy at this point.

cyphar · 2021-04-08T05:44:45Z

Though I guess this is being run in a slightly different scenario, it's in the "make everything read-only recursively" step not in the more general mount setup where we're just applying the configuration we were given. Maybe this does make sense in runc...

(As an aside, MS_REC is being ignored by the kernel here -- you cannot make things recursively readonly in one-shot, at least not until very recently with mount_setattr(2).)

giuseppe · 2021-04-08T07:36:51Z

@giuseppe ptal

I think the code is similar to what crun does: https://github.com/containers/crun/blob/master/src/libcrun/linux.c#L470-L497

It is a bit more articulated as it covers other cases as well, but for the MS_RDONLY I think it is equivalent, even if attempted only on failures: https://github.com/containers/crun/blob/master/src/libcrun/linux.c#L488-L492

chuanchang · 2021-04-08T13:09:11Z

The issue is easy to repro using any recent runc (I tried rc91 up to latest git) and podman >= 3

@kolyshkin hi, I gave a tests on RHEL-8.4 VM environment, but I got different result,
for more, please see the following details, thanks!

In my VM environment.

$ podman unshare cat /proc/self/uid_map
         0       1000          1
         1     100000      65536

$ rpm -q podman runc kernel
podman-3.0.1-6.module+el8.4.0+10487+af324045.x86_64
runc-1.0.0-70.rc92.module+el8.4.0+10487+af324045.x86_64
kernel-4.18.0-287.el8.dt4.x86_64

$ podman --runtime=runc run --pid=host --rm -it busybox sh

I got different error like below

[test@kvm-08-guest05 runc]$ git rev-parse HEAD
bb28c44f12bf24ea64590edfb4f23a4b4d2eaae8

[test@kvm-08-guest05 runc]$ podman --runtime=/home/test/go/src/github.com/opencontainers/runc/runc run --pid=host --rm -it quay.io/libpod/busybox sh
Error: time="2021-04-08T08:48:52-04:00" level=error msg="container_linux.go:380: starting container process caused: process_linux.go:545: container init caused: rootfs_linux.go:60: mounting \"sysfs\" to rootfs at \"/sys\" caused: operation not permitted": OCI permission denied

I also applied your PR2897, but I still got above error.

[test@kvm-08-guest05 runc]$ git rev-parse HEAD
3addcd4717d677653b58ae1cc86c952dbaf7581b

[test@kvm-08-guest05 runc]$ git log -1 | grep "rootless + host pidns"
    runc run: fix start for rootless + host pidns

[test@kvm-08-guest05 runc]$ make all
go build -trimpath "-buildmode=pie"  -tags "seccomp" -ldflags "-X main.gitCommit="3addcd4717d677653b58ae1cc86c952dbaf7581b" -X main.version=1.0.0-rc93+dev " -o runc .
go build -trimpath "-buildmode=pie"  -tags "seccomp" -ldflags "-X main.gitCommit="3addcd4717d677653b58ae1cc86c952dbaf7581b" -X main.version=1.0.0-rc93+dev " -o contrib/cmd/recvtty/recvtty ./contrib/cmd/recvtty

[test@kvm-08-guest05 runc]$ podman --runtime=/home/test/go/src/github.com/opencontainers/runc/runc run --pid=host --rm -it quay.io/libpod/busybox sh
Error: time="2021-04-08T08:54:54-04:00" level=error msg="container_linux.go:380: starting container process caused: process_linux.go:545: container init caused: rootfs_linux.go:60: mounting \"sysfs\" to rootfs at \"/sys\" caused: operation not permitted": OCI permission denied

mheon · 2021-04-08T13:18:21Z

Code LGTM

giuseppe · 2021-04-08T13:40:45Z

@kolyshkin hi, I gave a tests on RHEL-8.4 VM environment, but I got different result,
for more, please see the following details, thanks!

[test@kvm-08-guest05 runc]$ podman --runtime=/home/test/go/src/github.com/opencontainers/runc/runc run --pid=host --rm -it quay.io/libpod/busybox sh
Error: time="2021-04-08T08:48:52-04:00" level=error msg="container_linux.go:380: starting container process caused: process_linux.go:545: container init caused: rootfs_linux.go:60: mounting "sysfs" to rootfs at "/sys" caused: operation not permitted": OCI permission denied

that could be a regression in the RHEL 8.4 kernel, that is not caused by runc (https://bugzilla.redhat.com/show_bug.cgi?id=1903983), please make sure the kernel version is updated.

chuanchang · 2021-04-08T16:43:34Z

that could be a regression in the RHEL 8.4 kernel, that is not caused by runc (https://bugzilla.redhat.com/show_bug.cgi?id=1903983), please make sure the kernel version is updated.

Thank you @giuseppe, I can reproduce this issue after upgrading kernel to 4.18.0-293,
and this PR works for me, altough I got some warning, thanks!

[test@hpe-dl380pgen8-02-vm-3 runc]$ git rev-parse HEAD
3addcd4717d677653b58ae1cc86c952dbaf7581b

[test@hpe-dl380pgen8-02-vm-3 runc]$ podman --runtime=/home/test/go/src/github.com/opencontainers/runc/runc run --pid=host --rm -it quay.io/libpod/busybox sh
/ # ls
bin   dev   etc   home  proc  root  run   sys   tmp   usr   var
/ # exit
WARN[0000] cannot toggle freezer: cgroups not configured for container
WARN[0000] cannot toggle freezer: cgroups not configured for container
WARN[0000] lstat : no such file or directory

kolyshkin · 2021-04-08T19:42:16Z

libcontainer/rootfs_linux.go

-	return unix.Mount(path, path, "", unix.MS_BIND|unix.MS_REMOUNT|unix.MS_RDONLY|unix.MS_REC, "")
+
+	var s unix.Statfs_t
+	if err := unix.Statfs(path, &s); err != nil {


I was thinking about trying mount first and only going statfs route on EPERM, but this way the code path will rarely be hit (and thus tested), and a single statfs doesn't add too much overhead.

kolyshkin · 2021-04-14T17:15:07Z

I am still working on a test case for that, hope to finish this week.

kolyshkin · 2021-04-15T00:19:21Z

Added a test case, which fails like this (before the fix):

not ok 5 runc run [rootless with host pidns]
# (in test file tests/integration/start_hello.bats, line 78)
#   `[ "$status" -eq 0 ]' failed
# runc spec --rootless (status=0):
# 
# runc run test_hello (status=1):
# time="2021-04-14T17:12:35-07:00" level=warning msg="unable to get oom kill count" error="openat2 /sys/fs/cgroup/user.slice/user-1000.slice/[email protected]/test_hello/memory.events: no such file or directory"
# time="2021-04-14T17:12:35-07:00" level=warning msg="freezer not supported: openat2 /sys/fs/cgroup/user.slice/user-1000.slice/[email protected]/test_hello/cgroup.freeze: no such file or directory"
# time="2021-04-14T17:12:35-07:00" level=warning msg="lstat /sys/fs/cgroup/user.slice/user-1000.slice/[email protected]/test_hello: no such file or directory"
# time="2021-04-14T17:12:35-07:00" level=error msg="container_linux.go:380: starting container process caused: process_linux.go:545: container init caused: readonly path /proc/bus: operation not permitted"

NB: maybe we need to address those rootless warnings // cc @AkihiroSuda

Currently, runc fails like this when used from rootless podman with host PID namespace: > $ podman --runtime=runc run --pid=host --rm -it busybox sh > WARN[0000] additional gid=10 is not present in the user namespace, skip setting it > Error: container_linux.go:380: starting container process caused: > process_linux.go:545: container init caused: readonly path /proc/asound: > operation not permitted: OCI permission denied (Here /proc/asound is the first path from OCI spec's readonlyPaths). The code uses MS_BIND|MS_REMOUNT flags that have a special meaning in the kernel ("keep the flags like nodev, nosuid, noexec as is"). For some reason, this "special meaning" trick is not working for the above use case (rootless podman + no PID namespace), and I don't know how to reproduce this without podman. Instead of relying on the kernel feature, let's just get the current mount flags using fstatfs(2) and add those that needs to be preserved. While at it, wrap errors from unix.Mount into os.PathError to make errors a bit less cryptic. Signed-off-by: Kir Kolyshkin <[email protected]>

For the fix, see previous commit. Without the fix, this test case fails: > container_linux.go:380: starting container process caused: > process_linux.go:545: container init caused: readonly path /proc/bus: > operation not permitted Signed-off-by: Kir Kolyshkin <[email protected]>

kolyshkin · 2021-04-15T01:06:40Z

NB: maybe we need to address those rootless warnings // cc @AkihiroSuda

Opened #2910 about the OOM kill count warning, as it was recently introduced.

kolyshkin commented Apr 8, 2021

View reviewed changes

kolyshkin mentioned this pull request Apr 8, 2021

rc94 discussion (mid-April 2021?) #2790

Closed

kolyshkin force-pushed the fix-ro-paths branch from 3addcd4 to b0bdf5f Compare April 15, 2021 00:18

kolyshkin added 2 commits April 14, 2021 17:32

kolyshkin force-pushed the fix-ro-paths branch from b0bdf5f to 31dd1e4 Compare April 15, 2021 00:35

AkihiroSuda approved these changes Apr 15, 2021

View reviewed changes

kolyshkin added this to the 1.0.0-rc94 milestone Apr 15, 2021

mrunalp approved these changes Apr 19, 2021

View reviewed changes

mrunalp merged commit 3a20ccb into opencontainers:master Apr 19, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

runc run: fix readonly path error for rootless + host pidns #2897

runc run: fix readonly path error for rootless + host pidns #2897

kolyshkin commented Apr 8, 2021 •

edited

Loading

kolyshkin commented Apr 8, 2021

TomSweeneyRedHat commented Apr 8, 2021

mrunalp commented Apr 8, 2021

cyphar commented Apr 8, 2021 •

edited

Loading

cyphar commented Apr 8, 2021 •

edited

Loading

giuseppe commented Apr 8, 2021

chuanchang commented Apr 8, 2021

mheon commented Apr 8, 2021

giuseppe commented Apr 8, 2021

chuanchang commented Apr 8, 2021

kolyshkin Apr 8, 2021

kolyshkin commented Apr 14, 2021

kolyshkin commented Apr 15, 2021

kolyshkin commented Apr 15, 2021

runc run: fix readonly path error for rootless + host pidns #2897

runc run: fix readonly path error for rootless + host pidns #2897

Conversation

kolyshkin commented Apr 8, 2021 • edited Loading

kolyshkin commented Apr 8, 2021

TomSweeneyRedHat commented Apr 8, 2021

mrunalp commented Apr 8, 2021

cyphar commented Apr 8, 2021 • edited Loading

cyphar commented Apr 8, 2021 • edited Loading

giuseppe commented Apr 8, 2021

chuanchang commented Apr 8, 2021

mheon commented Apr 8, 2021

giuseppe commented Apr 8, 2021

chuanchang commented Apr 8, 2021

kolyshkin Apr 8, 2021

Choose a reason for hiding this comment

kolyshkin commented Apr 14, 2021

kolyshkin commented Apr 15, 2021

kolyshkin commented Apr 15, 2021

kolyshkin commented Apr 8, 2021 •

edited

Loading

cyphar commented Apr 8, 2021 •

edited

Loading

cyphar commented Apr 8, 2021 •

edited

Loading