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

linux: Keep MS_RDONLY when remounting bind mount of a read-only source #120

Conversation

debarshiray
Copy link
Member

On Silverblue, the host's /usr is mounted read-only:
$ findmnt -o TARGET,OPTIONS,PROPAGATION /usr
TARGET OPTIONS PROPAGATION
/usr ro,relatime,seclabel shared

On such a system, this throws an EPERM when using crun as the OCI
runtime:
$ podman run
--rm
--volume /usr:/run/host/usr:ro
registry.fedoraproject.org/fedora:31
/bin/true
Error: remount '... merged/run/host/usr': Operation not permitted: OCI
runtime permission denied error

The underlying system calls are:
mount("/usr",
"... merged/run/host/usr",
0x55d5dffb0bc0,
MS_NOSUID|MS_NODEV|MS_BIND|MS_REC|MS_SLAVE,
0x55d5dffaeef0) = 0
mount("/usr",
"... merged/run/host/usr",
0x55d5dffb0bc0,
MS_NOSUID|MS_NODEV|MS_REMOUNT|MS_BIND|MS_REC|MS_SLAVE,
NULL) = -1 EPERM (Operation not permitted)

Compare that to the system calls generated by runc when used as the
runtime with the same Podman invocation:
mount("/usr",
"... merged/run/host/usr",
0xc000170275,
MS_RDONLY|MS_NOSUID|MS_NODEV|MS_BIND|MS_REC,
NULL) = 0
mount("",
"... merged/run/host/usr",
0xc00017027b,
MS_REC|MS_SLAVE,
NULL) = 0
mount("/usr",
"... merged/run/host/usr",
0xc000170285,
MS_RDONLY|MS_NOSUID|MS_NODEV|MS_REMOUNT|MS_BIND|MS_REC,
NULL) = 0

What's happening here is that when crun tries to remount the newly
created bind mount to apply the rest of the flags unrelated to mount
propagation, it doesn't have the MS_RDONLY flag. This isn't allowed
because the source on the host (ie., /usr) is mounted read-only.

While it's true that the MS_RDONLY flag is ignored in the first
invocation of mount(2) that creates the mount-point, there's also no
harm in mentioning it because it will be silently ignored. Therefore,
it's easier to just keep the flags as they are instead of trying to
track which subset of flags are used by each mount(2) invocation.

Fixes: f658f17 ("linux: remount to honor flags like ...")

On Silverblue, the host's /usr is mounted read-only:
$ findmnt -o TARGET,OPTIONS,PROPAGATION /usr
TARGET OPTIONS              PROPAGATION
/usr   ro,relatime,seclabel shared

On such a system, this throws an EPERM when using crun as the OCI
runtime:
$ podman run \
      --rm \
      --volume /usr:/run/host/usr:ro \
      registry.fedoraproject.org/fedora:31 \
      /bin/true
Error: remount '... merged/run/host/usr': Operation not permitted: OCI
    runtime permission denied error

The underlying system calls are:
mount("/usr",
      "... merged/run/host/usr",
      0x55d5dffb0bc0,
      MS_NOSUID|MS_NODEV|MS_BIND|MS_REC|MS_SLAVE,
      0x55d5dffaeef0) = 0
mount("/usr",
      "... merged/run/host/usr",
      0x55d5dffb0bc0,
      MS_NOSUID|MS_NODEV|MS_REMOUNT|MS_BIND|MS_REC|MS_SLAVE,
      NULL) = -1 EPERM (Operation not permitted)

Compare that to the system calls generated by runc when used as the
runtime with the same Podman invocation:
mount("/usr",
      "... merged/run/host/usr",
      0xc000170275,
      MS_RDONLY|MS_NOSUID|MS_NODEV|MS_BIND|MS_REC,
      NULL) = 0
mount("",
      "... merged/run/host/usr",
      0xc00017027b,
      MS_REC|MS_SLAVE,
      NULL) = 0
mount("/usr",
      "... merged/run/host/usr",
      0xc000170285,
      MS_RDONLY|MS_NOSUID|MS_NODEV|MS_REMOUNT|MS_BIND|MS_REC,
      NULL) = 0

What's happening here is that when crun tries to remount the newly
created bind mount to apply the rest of the flags unrelated to mount
propagation, it doesn't have the MS_RDONLY flag. This isn't allowed
because the source on the host (ie., /usr) is mounted read-only.

While it's true that the MS_RDONLY flag is ignored in the first
invocation of mount(2) that creates the mount-point, there's also no
harm in mentioning it because it will be silently ignored. Therefore,
it's easier to just keep the flags as they are instead of trying to
track which subset of flags are used by each mount(2) invocation.

Fixes: f658f17 ("linux: remount to honor flags like ...")
@debarshiray
Copy link
Member Author

I am not 100% sure about the role of the remounts field in finalize_mounts() though.

@debarshiray
Copy link
Member Author

There's some related discussion in containers/podman#4061 but the issue there is something else.

@rhatdan
Copy link
Member

rhatdan commented Oct 3, 2019

Makes sense to me.
@giuseppe PTAL

@giuseppe
Copy link
Member

giuseppe commented Oct 3, 2019

thanks for the patch!

LGTM

@ghost
Copy link

ghost commented Oct 3, 2019

It is a bugfix. I can give it a test. Is this patch packaged in Fedora?

@juhp
Copy link

juhp commented Oct 4, 2019

Here is a test build for f31: https://koji.fedoraproject.org/koji/taskinfo?taskID=38044787
Now toolbox works for me in F31 Silverblue. :)
(Though I need to enter my toolbox 3 times initially - 3rd time it succeeds.)

@returntrip
Copy link

@juhp Thanks! I have quickly tested and it works also for me, I did not need to try to enter three times :)

@rhatdan
Copy link
Member

rhatdan commented Oct 4, 2019

Excellent.

@debarshiray debarshiray deleted the wip/rishi/keep-msrdonly-when-remounting-bind-mount-of-a-read-only-source branch October 4, 2019 12:35
@giuseppe
Copy link
Member

giuseppe commented Oct 4, 2019

new release here: https://bodhi.fedoraproject.org/updates/FEDORA-2019-845efd1d30

if it still works, please add karma :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants