Changes to os/exec_unix.go and issue 1954.

[Dave Cheney]

Hello,

I appear to have kicked over a hornets nest in attempting to close
issue 1954, please accept my appologies for not bringing a propsal for
the changes to golang-dev before submitting a CL.

To respond to the comments raised so far

* Regarding the size of the patch: yes, it's huge because of the
requirement that forkAndExecInChild doesn't acquire locks or new stack
segments. I don't see a way around duplicating the common code.
However, from my point of view I would prefer to see a variant of
forkAndExecInChild for each operating system, rather than a
consolidated one with conditional checks all the way through. This
also has implications for the SysProcAttr struct.

* Ownership of the initial patch: greping the CONTRIBUTORS file I
can't see an entry for jdn...@qwe.cc so I can only assume there is no
CLA on file for that person. As their original patch added new
functionality, I can't see that it can be applied without a CLA.

What I would like to propose is I submit a new CL to split
exec_unix.go into two flavors with the names exec_unix_linux.go and
exec_unix_bsd.go using the method Russ proposed. In this way it
suggests, for example, exec_unix_linux.go is related to exec_unix.go,
in the same way syscall_linux_amd64.go is to syscall_linux.go. No new
functionality would be added, simply forkAndExecInChild() and the
SysProcAttr struct would be moved out of exec_unix.go and duplicated
in the other two files. Future CL's could address adding unshare(2),
prctl or possibly removing the darwin specific workaround from the
linux variant. There may also be opportunities to consolidate some of
exec_plan9.go, but I haven't looked into that at all.

Alternatively, I can just drop the whole thing. If that is the case,
issue 1954 should probably closed as can't/wont fix.

Cheers

Dave

[Albert Strasheim]

Hello all

I see the original CL for this has been abandoned:

http://codereview.appspot.com/5246052/

What's the feeling about how we want to handle this? I'd really like
to contribute the prctl support, but we need to decide on a strategy
first.

Regards

Albert

[Albert Strasheim]

Hello

On Thu, Oct 20, 2011 at 4:51 PM, Paul Borman <bor...@google.com> wrote:
> The unshare system call does nearly nothing, at least according to the
> documentation for unshare(2) that I read.  For real unshare support the code
> would need to be changed to use the clone system call and would be very
> Linux specific.  Looking at the documentation I see that unshare only
> supports:

I'm not particularly interested in unshare per se. What is more
interesting is how the core developers think we should deal with
unshare, prctl and other system-specific exec stuff.

I'd like to contribute some prctl bits, but I'm going to run into the
same issue as the unshare CL: do we want an exec_linux.go? If not,
what approach should we take?

Regards

Albert

[Paul Borman]

So that particular problem is one that Russ is pondering.  Without pre-processing the files, and with the "no memory allocations and hence no function calls" restriction it is pretty hard to make conditional code and this code is not code you want to fork and two copies.  For unshare the solution I saw, that did not require forking of code, was to make a dummy definition for SYS_UNSHARE and then at runtime check to see if it can make the unshare call.  A structure could be defined per OS that contains the various bool's for the various flags and for non-linux the structure would be empty (or you could do bit fields).

As for unshare, couldn't this be done as effectively as a stand-alone system call at the start of a program? 

The barrier to forking the exec code is really high because the fork/exec code is delicate.

[Paul Borman]

Sorry, I sent my reply to just Albert.  Also, I have updated information from what I sent.

What I said was:

The unshare system call does nearly nothing, at least according to the documentation for unshare(2) that I read.  For real unshare support the code would need to be changed to use the clone system call and would be very Linux specific.  Looking at the documentation I see that unshare only supports:

       CLONE_FILES

              Reverse  the  effect  of the clone(2) CLONE_FILES flag.  Unshare

              the file descriptor table, so that the calling process no longer

              shares its file descriptors with any other process.

       CLONE_FS

              Reverse  the effect of the clone(2) CLONE_FS flag.  Unshare file

              system attributes, so that the calling process no longer  shares

              its  root directory, current directory, or umask attributes with

              any other process.  chroot(2), chdir(2), or umask(2)

       CLONE_NEWNS

              This flag has the same effect as the clone(2) CLONE_NEWNS  flag.

              Unshare  the  mount namespace, so that the calling process has a

              private copy of its namespace which is not shared with any other

              process.  Specifying this flag automatically implies CLONE_FS as

              well.

This would indicate only CLONE_NEWNS is of value and that is marginal.  With the current state of Go there are not many options to easily share different code.  This is one of the few cases where I think a #ifdef would be useful.

If unshare(2) can do more than the manual page indicates then it might warrant looking at just using unshare(2) and not trying to switch to clone(2).

The updated information is that I just wrote an Unshare function:

func Unshare(f int) os.Error {

_, _, err1 := syscall.RawSyscall(syscall.SYS_UNSHARE, uintptr(f), 0, 0)

if err1 != 0 {

return os.NewSyscallError("unshare", int(err1))

}

return nil

}

And then tried it with passing in CLONE_NEWNET.  That process did indeed get a new network namespace, so it appears that you can do some real work with unshare outside of the fork/exec code in syscall.  If you are trying to write a program similar to unshare(1) then this is sub-optimal.  If you want to write a go program that unshares itself, this should work.

    -Paul