From 67399c6279d7c2777a1a83103644f853d87533ba Mon Sep 17 00:00:00 2001 From: "Ronald G. Minnich" Date: Mon, 24 Apr 2017 16:09:24 -0700 Subject: [PATCH] syscall: fix ordering of Unshare and chroot on Linux When unshare specifies a new namespace, the syscall package changes / to make namespace changes private. If a chroot is specified, the unshare must be done first. If the chroot is done first then the unshare will not specify the correct /. A new test is included which test combining chroot and CLONE_NEWNS; it fails without the patch and works with it. Fixes #20103 Change-Id: I86022803c784bd418a30383321f3d64103d95c62 Reviewed-on: https://go-review.googlesource.com/41626 Reviewed-by: Ian Lance Taylor Run-TryBot: Ian Lance Taylor TryBot-Result: Gobot Gobot --- src/syscall/exec_linux.go | 16 ++++----- src/syscall/exec_linux_test.go | 64 +++++++++++++++++++++++++++++++++- 2 files changed, 71 insertions(+), 9 deletions(-) diff --git a/src/syscall/exec_linux.go b/src/syscall/exec_linux.go index e35ac25580..66fb0356ba 100644 --- a/src/syscall/exec_linux.go +++ b/src/syscall/exec_linux.go @@ -195,14 +195,6 @@ func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr } } - // Chroot - if chroot != nil { - _, _, err1 = RawSyscall(SYS_CHROOT, uintptr(unsafe.Pointer(chroot)), 0, 0) - if err1 != 0 { - goto childerror - } - } - // Unshare if sys.Unshareflags != 0 { _, _, err1 = RawSyscall(SYS_UNSHARE, sys.Unshareflags, 0, 0) @@ -224,6 +216,14 @@ func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr } } + // Chroot + if chroot != nil { + _, _, err1 = RawSyscall(SYS_CHROOT, uintptr(unsafe.Pointer(chroot)), 0, 0) + if err1 != 0 { + goto childerror + } + } + // User and groups if cred := sys.Credential; cred != nil { ngroups := uintptr(len(cred.Groups)) diff --git a/src/syscall/exec_linux_test.go b/src/syscall/exec_linux_test.go index 83cde96b1f..7a88968b49 100644 --- a/src/syscall/exec_linux_test.go +++ b/src/syscall/exec_linux_test.go @@ -12,6 +12,7 @@ import ( "io/ioutil" "os" "os/exec" + "path/filepath" "strings" "syscall" "testing" @@ -265,7 +266,6 @@ func TestUnshareMountNameSpaceHelper(*testing.T) { return } defer os.Exit(0) - if err := syscall.Mount("none", flag.Args()[0], "proc", 0, ""); err != nil { fmt.Fprintf(os.Stderr, "unshare: mount %v failed: %v", os.Args, err) os.Exit(2) @@ -320,3 +320,65 @@ func TestUnshareMountNameSpace(t *testing.T) { } } } + +// Test for Issue 20103: unshare fails when chroot is used +func TestUnshareMountNameSpaceChroot(t *testing.T) { + // Make sure we are running as root so we have permissions to use unshare + // and create a network namespace. + if os.Getuid() != 0 { + t.Skip("kernel prohibits unshare in unprivileged process, unless using user namespace") + } + + // When running under the Go continuous build, skip tests for + // now when under Kubernetes. (where things are root but not quite) + // Both of these are our own environment variables. + // See Issue 12815. + if os.Getenv("GO_BUILDER_NAME") != "" && os.Getenv("IN_KUBERNETES") == "1" { + t.Skip("skipping test on Kubernetes-based builders; see Issue 12815") + } + + d, err := ioutil.TempDir("", "unshare") + if err != nil { + t.Fatalf("tempdir: %v", err) + } + + // Since we are doing a chroot, we need the binary there, + // and it must be statically linked. + x := filepath.Join(d, "syscall.test") + cmd := exec.Command("go", "test", "-c", "-o", x, "syscall") + cmd.Env = append(os.Environ(), "CGO_ENABLED=0") + if o, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("Build of syscall in chroot failed, output %v, err %v", o, err) + } + + cmd = exec.Command("/syscall.test", "-test.run=TestUnshareMountNameSpaceHelper", "/") + cmd.Env = []string{"GO_WANT_HELPER_PROCESS=1"} + cmd.SysProcAttr = &syscall.SysProcAttr{Chroot: d, Unshareflags: syscall.CLONE_NEWNS} + + o, err := cmd.CombinedOutput() + if err != nil { + if strings.Contains(err.Error(), ": permission denied") { + t.Skipf("Skipping test (golang.org/issue/19698); unshare failed due to permissions: %s, %v", o, err) + } + t.Fatalf("unshare failed: %s, %v", o, err) + } + + // How do we tell if the namespace was really unshared? It turns out + // to be simple: just try to remove the executable. If it's still mounted + // on, the rm will fail. Then we have some cleanup to do: + // we must force unmount it, then try to remove it again. + + if err := os.Remove(x); err != nil { + t.Errorf("rm failed on %v: %v", x, err) + if err := syscall.Unmount(d, syscall.MNT_FORCE); err != nil { + t.Fatalf("Can't unmount %v: %v", d, err) + } + if err := os.Remove(x); err != nil { + t.Fatalf("rm failed on %v: %v", x, err) + } + } + + if err := os.Remove(d); err != nil { + t.Errorf("rmdir failed on %v: %v", d, err) + } +} -- 2.48.1