linux: fix bind mount flags - #2247
Merged
Merged
Conversation
Commit f53aa37 made finalize_mounts use mount_setattr for read-only remounts, which is done for e.g. a "bind,ro" mount. Unlike do_remount it used before, it only sets the requested flags, not clearing the rest. As a result, a "bind,ro" mount of a nosuid,nodev,noexec source keeps all those flags, while it should only have the requested one (ro), as it was before, and as it is in runc. Clear the flags which are not set, like do_remount does. Found by the mounts_sshfs.bats runc integration tests. Fixes: f53aa37 ("linux: use mount_setattr for readonly remounts in finalize_mounts") Signed-off-by: Kir Kolyshkin <[email protected]>
A bind mount source is opened (cloned) in advance, and after it is moved into place, its flags are only changed if there are some flags to set. Options which only clear flags (e.g. "dev", "suid", "exec", "rw") do not set any, so they are silently ignored, and the flags of the source are kept. For example, a "bind,dev,suid,exec" mount of a ro,nosuid,nodev,noexec source is left with all these flags. Do the remount if there are any mount flag options, so the resulting flags are exactly the requested ones, like it is done for a bind mount with some flags to set (e.g. "bind,nosuid"), and like runc does. A bind mount with no such options (e.g. "bind", "rbind,rprivate") still keeps the flags of the source. Add a test case, covering the previous commit, too. Found by the mounts_sshfs.bats runc integration tests. Signed-off-by: Kir Kolyshkin <[email protected]>
When remounting to apply the requested flags, the flags which are not requested are cleared, except for nosymfollow, which is inherited from the source mount. E.g., a "bind,ro" mount of a nosymfollow source is left with nosymfollow. Add MS_NOSYMFOLLOW to the flags which are cleared, like runc does. Note it is not a locked flag, so it can be cleared in a user namespace, too. Found by the mounts_sshfs.bats runc integration tests. Signed-off-by: Kir Kolyshkin <[email protected]>
When remounting with mount_setattr, the atime part of the mount flags
is only reset if the requested flags contain a non-zero atime value.
Yet, relatime is zero, and nodiratime is not a part of the atime value,
so, e.g., a "bind,relatime" mount of a noatime source is left with
noatime, and so is a "bind,nodiratime" one.
Do what mount(2) does: if any atime flag is set, set all the atime ones
anew (relatime is the default, noatime and strictatime override it, and
nodiratime is independent); otherwise, keep the atime flags as is. This
is also what runc does, as it uses mount(2).
Add test cases.
Found by the "runc run [bind mount {no,rel,strict}atime semantics]"
runc integration test.
Signed-off-by: Kir Kolyshkin <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The flags of a bind mount with some mount flag options should be exactly the requested ones (with the unrequested ones cleared), while a bind mount with no such options (e.g. just
bind) should keep the flags of the source. This is how runc works, and mostly how crun works, too, except for these cases:linux: clear unrequested flags on read-only remount
Commit f53aa37 made
finalize_mountsusemount_setattrfor read-only remounts (e.g. for abind,romount), which only sets the requested flags, not clearing the rest (unlikedo_remountused before). As a result, abind,romount of anosuid,nodev,noexecsource keeps all those flags. Clear the flags which are not set, likedo_remountdoes.Fixes: f53aa37 ("linux: use mount_setattr for readonly remounts in finalize_mounts")
linux: honor clearing-only options for bind mounts
A bind mount source is cloned in advance, and after it is moved into place, its flags are only changed if there are some flags to set. Options which only clear flags (e.g.
dev,suid,exec,rw) do not set any, so they are silently ignored. For example, abind,dev,suid,execmount of aro,nosuid,nodev,noexecsource is left with all these flags. Do the remount if there are any mount flag options. Add a test case (covering the previous commit, too).linux: clear nosymfollow on remount unless requested
Add
MS_NOSYMFOLLOWto the flags which are cleared when not requested, like runc does. It is not a locked flag, so it can be cleared in a user namespace, too.linux: set atime flags like mount(2) does
When remounting with mount_setattr, the atime part of the flags is only reset if the requested flags contain a non-zero atime value. Yet, relatime is zero, and nodiratime is not a part of the atime value, so e.g. a
bind,relatimemount of anoatimesource is left withnoatime, and so is abind,nodiratimeone. Do what mount(2) (and so runc) does: if any atime flag is set, set all the atime ones anew; otherwise, keep them as is. Add test cases.Checked against runc with a set of source flags / bind options combinations (the ones from runc's
mounts_sshfs.bats), now giving the same results (without a user namespace).Found by the
mounts_sshfs.batsrunc integration tests (see #2238).