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

Revert "rootfs: handle nested procfs mounts for MS_MOVE" + add a test #2654

Closed
wants to merge 2 commits into from

Conversation

AkihiroSuda
Copy link
Member

Reverts #2647 due to a security issue
Reopens #2639

This reverts commit b8bf572.

Revert #2647
Reopen #2339

Signed-off-by: Akihiro Suda <[email protected]>
@AkihiroSuda AkihiroSuda force-pushed the revert-2647-procfs-masking-nested branch from b626821 to 2aa00eb Compare October 20, 2020 13:58
@cyphar
Copy link
Member

cyphar commented Oct 20, 2020

This is actually a bug in github.com/moby/sys/mountinfo -- info.Root is always "". I've already submitted a fix (moby/sys#50).

mountinfos, err := mountinfo.GetMounts(func(info *mountinfo.Info) (skip, stop bool) {
// Collect every sysfs and procfs filesystem, except for those which
// are non-full mounts or are inside the rootfs of the container.
if info.Root != "/" ||
Copy link
Member

Choose a reason for hiding this comment

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

It's overkill to revert it -- a simpler patch is to just remove this info.Root != "/" clause which won't cause us to revert the fix (the ENOENT check later in the patch is sufficient to fix that bug).

Copy link
Member Author

Choose a reason for hiding this comment

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

Let's revert it first and then revendor moby/sys#50 when it is merged

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with @cyphar here -- for the reason of having a slightly less dramatic git history.

For preventing regression like #2647

Signed-off-by: Akihiro Suda <[email protected]>
@AkihiroSuda AkihiroSuda changed the title Revert "rootfs: handle nested procfs mounts for MS_MOVE" Revert "rootfs: handle nested procfs mounts for MS_MOVE" + add a test Oct 20, 2020
@AkihiroSuda
Copy link
Member Author

Added a new commit: test: add "runc run --no-pivot must not expose bare /proc"

@cyphar
Copy link
Member

cyphar commented Oct 21, 2020

#2655 removes just the info.Root check and cherry-picks 729e786.

@AkihiroSuda AkihiroSuda deleted the revert-2647-procfs-masking-nested branch February 4, 2021 06:01
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.

3 participants