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.

Sign up for GitHub

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

More fix to nsexec.c's comments #1168

Merged
merged 1 commit into from
Nov 7, 2016
Merged

More fix to nsexec.c's comments #1168

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 14 additions & 6 deletions libcontainer/nsenter/nsexec.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -598,11 +598,11 @@ void nsexec(void)

/*
* Stage 1: We're in the first child process. Our job is to join any
* provided user namespaces in the netlink payload. If we've been
* asked to CLONE_NEWUSER, we will unshare the user namespace and
* ask our parent (stage 0) to set up our user mappings for us.
* Then, we unshare the rest of the requested namespaces and
* create a new child (stage 2: JUMP_INIT). We then send the
* provided namespaces in the netlink payload and unshare all
* of the requested namespaces. If we've been asked to
* CLONE_NEWUSER, we will ask our parent (stage 0) to set up
* our user mappings for us. Then, we create a new child
* (stage 2: JUMP_INIT) for PID namespace. We then send the
* child's PID to our parent (stage 0).
*/
case JUMP_CHILD: {
Expand Down Expand Up @@ -660,7 +660,15 @@ void nsexec(void)
bail("failed to sync with parent: SYNC_USERMAP_ACK: got %u", s);
}

/* TODO: What about non-namespace clone flags that we're dropping here? */
/*
* TODO: What about non-namespace clone flags that we're dropping here?
*
* We fork again because of PID namespace, setns(2) or unshare(2) don't
* change the PID namespace of the calling process, because doing so
* would change the caller's idea of its own PID (as reported by getpid()),
* which would break many applications and libraries, so we must fork
* to actually enter the new PID namespace.
Copy link
Member

Choose a reason for hiding this comment

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

You don't really need to explain why the PID namespace is so funky. You can just say that we have to fork again to enter the PID namespace (even though we unshared it) because the PID namespace affects children only.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I still got questions with comments like you said in out old implementation, so I think we can explain it clearly. It's not redundance for most of people.

Copy link
Member

Choose a reason for hiding this comment

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

Fair enough.

*/
child = clone_parent(&env, JUMP_INIT);
if (child < 0)
bail("unable to fork: init_func");
Expand Down