Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[1.1] libct: fix mounting via wrong proc fd #3511

Merged
merged 1 commit into from
Jul 20, 2022

Conversation

kolyshkin
Copy link
Contributor

@kolyshkin kolyshkin commented Jun 15, 2022

Backport of #3510 to release-1.1 branch. The original description follows.


Due to a bug in commit 9c44407, when the user and mount namespaces
are used, and the bind mount is followed by the cgroup mount in the
spec, the cgroup is mounted using the bind mount's mount fd.

This can be reproduced with podman 4.1 (when configured to use runc):

$ podman run --uidmap 0:100:10000 quay.io/libpod/testimage:20210610 mount
Error: /home/kir/git/runc/runc: runc create failed: unable to start container process: error during container init: error mounting "cgroup" to rootfs at "/sys/fs/cgroup": mount /proc/self/fd/11:/sys/fs/cgroup/systemd (via /proc/self/fd/12), flags: 0x20502f: operation not permitted: OCI permission denied

or manually with the spec mounts containing something like this:

    {
      "destination": "/etc/resolv.conf",
      "type": "bind",
      "source": "/userdata/resolv.conf",
      "options": [
        "bind"
      ]
    },
    {
      "destination": "/sys/fs/cgroup",
      "type": "cgroup",
      "source": "cgroup",
      "options": [
        "rprivate",
        "nosuid",
        "noexec",
        "nodev",
        "relatime",
        "ro"
      ]
    }

The issue was not found earlier since it requires using userns, and even then
mount fd is ignored by mountToRootfs, except for bind mounts, and all the bind
mounts have mountfd set, except for the case of cgroup v1's /sys/fs/cgroup
which is internally transformed into a bunch of bind mounts.

This is a minimal fix for the issue, suitable for backporting.

Fixes: 9c44407 ("Open bind mount sources from the host userns")
Signed-off-by: Kir Kolyshkin [email protected]
(cherry picked from commit b3aa20a)
Signed-off-by: Kir Kolyshkin [email protected]

@kolyshkin kolyshkin added the backport/1.1-pr A backport PR to release-1.1 label Jun 15, 2022
@kolyshkin kolyshkin changed the title libct: fix mounting via wrong proc fd [1.1] libct: fix mounting via wrong proc fd Jun 15, 2022
@kolyshkin kolyshkin added this to the 1.1.4 milestone Jun 16, 2022
@AkihiroSuda
Copy link
Member

Marking as a draft as the main PR #3510 isn't merged yet

@AkihiroSuda AkihiroSuda marked this pull request as draft June 16, 2022 13:00
Due to a bug in commit 9c44407, when the user and mount namespaces
are used, and the bind mount is followed by the cgroup mount in the
spec, the cgroup is mounted using the bind mount's mount fd.

This can be reproduced with podman 4.1 (when configured to use runc):

$ podman run --uidmap 0:100:10000 quay.io/libpod/testimage:20210610 mount
Error: /home/kir/git/runc/runc: runc create failed: unable to start container process: error during container init: error mounting "cgroup" to rootfs at "/sys/fs/cgroup": mount /proc/self/fd/11:/sys/fs/cgroup/systemd (via /proc/self/fd/12), flags: 0x20502f: operation not permitted: OCI permission denied

or manually with the spec mounts containing something like this:

    {
      "destination": "/etc/resolv.conf",
      "type": "bind",
      "source": "/userdata/resolv.conf",
      "options": [
        "bind"
      ]
    },
    {
      "destination": "/sys/fs/cgroup",
      "type": "cgroup",
      "source": "cgroup",
      "options": [
        "rprivate",
        "nosuid",
        "noexec",
        "nodev",
        "relatime",
        "ro"
      ]
    }

The issue was not found earlier since it requires using userns, and even then
mount fd is ignored by mountToRootfs, except for bind mounts, and all the bind
mounts have mountfd set, except for the case of cgroup v1's /sys/fs/cgroup
which is internally transformed into a bunch of bind mounts.

This is a minimal fix for the issue, suitable for backporting.

A test case is added which reproduces the issue without the fix applied.

Fixes: 9c44407 ("Open bind mount sources from the host userns")
Signed-off-by: Kir Kolyshkin <[email protected]>
(cherry picked from commit d370e3c)
Signed-off-by: Kir Kolyshkin <[email protected]>
@AkihiroSuda AkihiroSuda marked this pull request as ready for review June 23, 2022 23:44
Copy link
Member

@rata rata left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@cyphar cyphar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (sorry for the delay, I was on leave).

@cyphar cyphar merged commit afda6b7 into opencontainers:release-1.1 Jul 20, 2022
@vinayakankugoyal
Copy link

with containerd 1.7.0 and runc 1.1.4 I am still seeing similar errors:

failed to create containerd task: failed to create shim task: OCI runtime create failed: runc create failed: unable to start container process: error during container init: error mounting \"/var/lib/containerd/io.containerd.grpc.v1.cri/sandboxes/ea42d3241f416305dcd9027f1a677090efc31439d414d0a764b2d9f207bda116/resolv.conf\" to rootfs at \"/etc/resolv.conf\": mount /proc/self/fd/7:/etc/resolv.conf (via /proc/self/fd/9), flags: 0x5021: operation not permitted: unknown

@rata
Copy link
Member

rata commented Mar 15, 2023

@vinayakankugoyal can you provide a reproducer for this?

A config.json that triggers this and all that is needed so we can do the runc run, hit this error and debug it. If you want to debug it and create a PR, that would be WAY better :)

@rata
Copy link
Member

rata commented Mar 15, 2023

@vinayakankugoyal also, please do so in a new issue, not here.

As we talked in slack, I've sent you a way to get the config.json, but probably you will have to prune it quite a lot to have a repro case that you can share.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.1-pr A backport PR to release-1.1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants