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

specconv: emit an error when using MS_PRIVATE with --no-pivot #1606

Merged
merged 1 commit into from
Oct 18, 2017
Merged

specconv: emit an error when using MS_PRIVATE with --no-pivot #1606

merged 1 commit into from
Oct 18, 2017

Conversation

cyphar
Copy link
Member

@cyphar cyphar commented Oct 8, 2017

Due to the semantics of chroot(2) when it comes to mount namespaces, it
is not generally safe to use MS_PRIVATE as a mount propgation when using
chroot(2). The reason for this is that this effectively results in a set
of mount references being held by the chroot'd namespace which the
namespace cannot free. pivot_root(2) does not have this issue because
the @old_root can be unmounted by the process.

Ultimately, --no-pivot is not really necessary anymore as a commonly
used option since f8e6b5a ("rootfs: make pivot_root not use a
temporary directory") resolved the read-only issue. But if someone
really needs to use it, MS_PRIVATE is never a good idea.

Fixes #1602
Signed-off-by: Aleksa Sarai [email protected]

@cyphar
Copy link
Member Author

cyphar commented Oct 8, 2017

I'm not sure if it's better that we have an error here or if we have some sort of warning message. I didn't want to add a warning message here initially because it would be the first logging we did as part of specconv (which doesn't really feel right).

Due to the semantics of chroot(2) when it comes to mount namespaces, it
is not generally safe to use MS_PRIVATE as a mount propgation when using
chroot(2). The reason for this is that this effectively results in a set
of mount references being held by the chroot'd namespace which the
namespace cannot free. pivot_root(2) does not have this issue because
the @old_root can be unmounted by the process.

Ultimately, --no-pivot is not really necessary anymore as a commonly
used option since f8e6b5a ("rootfs: make pivot_root not use a
temporary directory") resolved the read-only issue. But if someone
really needs to use it, MS_PRIVATE is never a good idea.

Signed-off-by: Aleksa Sarai <[email protected]>
@crosbymichael
Copy link
Member

What if we moved this into runc so that it happens before setup and clone?

@cyphar
Copy link
Member Author

cyphar commented Oct 15, 2017

@crosbymichael You mean in /utils.go? Maybe, though I think that some specconv users might actually want to know this as well. But I can switch this to a warning in /utils.go if you prefer.

@crosbymichael
Copy link
Member

crosbymichael commented Oct 16, 2017

Oh, sorry. I didn't read the filename correctly.

LGTM

Approved with PullApprove

@hqhq
Copy link
Contributor

hqhq commented Oct 18, 2017

LGTM

Approved with PullApprove

@hqhq hqhq merged commit 3409d5c into opencontainers:master Oct 18, 2017
@cyphar cyphar deleted the rootfs-propagation-no-pivot branch October 18, 2017 16:22
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.

runc with --no-pivot retains global mount context
3 participants