Comment 1:

Please use the normal contribution process if you'd like feedback.

Status changed to Accepted.

Comment 2 by mrunalp:

Sure, I will follow the contribution process.

Comment 3:

Labels changed: added repo-main, release-none.

Comment 4:

This bug should have more context and references to the interfaces involved.
It seems this is a new Linux-only interface as of Linux 3.5:
I dug up this description of /proc/nnn/uid_map and gid_map, etc:
https://lists.linux-foundation.org/pipermail/containers/2013-January/031504.html
It's probably committed somewhere.
Copying Ian, who loves fork issues.

Comment 5 by mrunalp:

Yes, this is a linux only feature. It is described here --
http://lwn.net/Articles/532593/

Comment 6:

OK, that is some crazy stuff.  But, it's not clear to me why this needs to go into the
syscall package at all.  Why can't the child process do it when it starts up?
That is, I can see that it would sometimes be convenient to have this in SysProcAttr,
but so far everything in SysProcAttr is there because it can't reasonably be done
elsewhere.  It seems like this mapping could be written by a helper process.

Comment 7 by mrunalp:

The child process is limited in what mappings it could write. One approach we tried was
writing the mappings after the child is exec'ed. In that case, child does become root
(if the parent writes the correct mappings), but it has lost capabilities on exec
(unless the binary was a setuid), so it is very limited in what it could do. It can't
even switch to the docker user since CAP_SETUID/CAP_SETGID are lost. This led us to the
path of pursuing this patch.

Comment 8 by crosby.michael:

The mappings have to be written after the fork and before the exec of the new process or
they will not be successfully applied.  That is why it has to go into the syscall
package because we have no reliable way to insert code in-between the fork and exec.
Like the comment above using a helper process to write the mappings does work but
capabilities are lost when the fork'd process tries to update it's own UID/GID mappings.

Comment 9 by mrunalp:

Here are more details about what happens in exec_linux.go when CLONE_NEWUSER is passed
as a flag --
1. child uid is overflowuid i.e 65534.
2. If UID/GID are passed setuid/setgid in child will fail since
   linux returns invalid arguments for setuid/setgid for any unmapped values.
   (With patch and mappings written, this succeeds as the mappings have been written.)
3. child execs any binary (nsinit in case of docker which isn't setuid) and since the
child isn't root it loses capabilities. With the patch we ensure that it is root, so it
has all capabilities and is able to do further setup before exec'ing into user supplied
command.

Comment 10:

OK.  We'll probably need to figure out how to implement this in the new go.sys package.

Comment 11 by brandon@ifup.co:

Ian- Does this mean that the current code review should be closed and something new
should be proposed against go.sys? https://golang.org/cl/126190043
This issue blocks a pretty major feature of Docker and libcontainer so it would be great
to get it on the right path for go 1.4.

Comment 12:

I think that I was wrong about go.sys.  Code that has to run between fork and exec more
or less has to run in the syscall support, otherwise the os and os/exec packages can't
use it.
I'm not sure I understand that CL, though.  It looks like that CL was never even mailed
out.  Is it ready for review?  Why does it use a pipe rather than simply opening the
file in the child?

Labels changed: added release-go1.4, removed release-none.

Comment 13 by mrunalp:

Ian, it wasn't mailed out after this discussion as I assumed that it won't be used in
favor of a go.sys based solution. The pipe is used as a synchronization mechanism to
make the child wait for the parent to write the mappings. BTW, I can mail this out to
the appropriate mailing list if we are ready to make the change in fork/exec code.

Comment 14 by crosby.michael:

Also, the parent process must write the mappings because the child is not allowed to
remap UID/GIDs that are not it's own, which is basically only 0.  That is why the pipe
is used and the parent writes the mappings for the child process.

Comment 15:

If you want something in Go 1.4 we need to start on that immediately if not sooner, as
we are already in the feature freeze.

Comment 16 by mrunalp:

I used the publish + mail comments option on the codereview, since I don't have access
to the system where I initiated the codereview from right now. Hopefully that sent out
an email to initiate a formal review. If that didn't work, I will resend it from my
other system in the evening.

Comment 17:

https://golang.org/cl/126190043/ mentions this issue.

Comment 18:

This issue was closed by revision f9d7e13.

Status changed to Fixed.

Comment 19 by teabee89:

Would this make it into Go 1.4 ?

Comment 20:

Yes, this will be in Go 1.4.