Fix #1: avoid restoring root net namespace, instead retire the joining thread

The problem is that setns() on /proc/self/ns/net might be priviledged
operation if we don't own the namespace. Therefore, avoid. This hack
calls runtime.LockOSThread, and does _not_ call UnlockOSThread, forcing
golang to retire the os thread joining the unpriviledged namespace.
Good enough.

Author: majek

go.mod

@@ -6,6 +6,7 @@ require (
 	github.com/cenkalti/backoff v2.2.1+incompatible // indirect
 	github.com/golang/protobuf v1.3.2 // indirect
 	github.com/opencontainers/runtime-spec v0.1.2-0.20171211145439-b2d941ef6a78
+	github.com/wadey/gocovmerge v0.0.0-20160331181800-b5bfa59ec0ad // indirect
 	golang.org/x/sys v0.0.0-20200121082415-34d275377bf9
 	golang.org/x/time v0.0.0-20191024005414-555d28b269f0 // indirect
 	gvisor.dev/gvisor v0.0.0-20200118174625-b75a6be0ea24

netns_utils.go

@@ -8,18 +8,26 @@ import (
 	"gvisor.dev/gvisor/runsc/specutils"
 )
 
-func joinNetNS(nsPath string) (func(), error) {
-	runtime.LockOSThread()
-	restoreNS, err := specutils.ApplyNS(specs.LinuxNamespace{
-		Type: specs.NetworkNamespace,
-		Path: nsPath,
-	})
-	if err != nil {
-		runtime.UnlockOSThread()
-		return nil, fmt.Errorf("joining net namespace %q: %v", nsPath, err)
-	}
-	return func() {
-		restoreNS()
-		runtime.UnlockOSThread()
-	}, nil
+func joinNetNS(nsPath string, run func()) error {
+	ch := make(chan error, 2)
+	go func() {
+		runtime.LockOSThread()
+		_, err := specutils.ApplyNS(specs.LinuxNamespace{
+			Type: specs.NetworkNamespace,
+			Path: nsPath,
+		})
+		if err != nil {
+			runtime.UnlockOSThread()
+			ch <- fmt.Errorf("joining net namespace %q: %v", nsPath, err)
+			return
+		}
+		run()
+		ch <- nil
+	}()
+	// Here is a big hack. Avoid restoring netns. Allow golang to
+	// reap the thread, by not calling runtime.UnlockOSThread().
+	// This will avoid any errors from restoreNS().
+
+	err := <-ch
+	return err
 }

stack.go

@@ -26,43 +26,54 @@ import (
 
 func GetTunTap(netNsPath string, ifName string) (int, bool, uint32, error) {
 	var (
-		restore func()
-		err     error
+		err error
 	)
-	if netNsPath != "" {
-		fmt.Fprintf(os.Stderr, "[.] Joininig netns %s\n", netNsPath)
-		restore, err = joinNetNS(netNsPath)
-		if err != nil {
-			fmt.Fprintf(os.Stderr, "[!] Can't join netns %s: %s\n", netNsPath, err)
-			return 0, false, 0, err
-		}
-	}
 
-	fmt.Fprintf(os.Stderr, "[.] Opening tun interface %s\n", ifName)
-	mtu, err := rawfile.GetMTU(ifName)
-	if err != nil {
-		fmt.Fprintf(os.Stderr, "[!] GetMTU(%s) = %s\n", ifName, err)
-		return 0, false, 0, err
+	type tunState struct {
+		fd      int
+		tapMode bool
+		mtu     uint32
+		err     error
 	}
 
-	tapMode := false
+	ch := make(chan tunState, 2)
+	run := func() {
+		fmt.Fprintf(os.Stderr, "[.] Opening tun interface %s\n", ifName)
+		mtu, err := rawfile.GetMTU(ifName)
+		if err != nil {
+			fmt.Fprintf(os.Stderr, "[!] GetMTU(%s) = %s\n", ifName, err)
+			ch <- tunState{err: err}
+			return
+		}
 
-	fd, err := tun.Open(ifName)
-	if err != nil {
-		tapMode = true
-		fd, err = tun.OpenTAP(ifName)
+		tapMode := false
+
+		fd, err := tun.Open(ifName)
 		if err != nil {
-			fmt.Fprintf(os.Stderr, "[!] open(%s) = %s\n", ifName, err)
-			return 0, false, 0, err
+			tapMode = true
+			fd, err = tun.OpenTAP(ifName)
+			if err != nil {
+				fmt.Fprintf(os.Stderr, "[!] open(%s) = %s\n", ifName, err)
+				ch <- tunState{err: err}
+				return
+			}
 		}
+		ch <- tunState{fd, tapMode, mtu, nil}
 	}
 
 	if netNsPath != "" {
-		fmt.Fprintf(os.Stderr, "[.] Restoring root netns\n")
-		restore()
+		fmt.Fprintf(os.Stderr, "[.] Joininig netns %s\n", netNsPath)
+		err = joinNetNS(netNsPath, run)
+		if err != nil {
+			fmt.Fprintf(os.Stderr, "[!] Can't join netns %s: %s\n", netNsPath, err)
+			return 0, false, 0, err
+		}
+	} else {
+		run()
 	}
 
-	return fd, tapMode, mtu, nil
+	s := <-ch
+	return s.fd, s.tapMode, s.mtu, s.err
 }
 
 func NewStack() *stack.Stack {
@@ -206,7 +217,7 @@ func GonetDialTCP(s *stack.Stack, laddr, raddr *tcpip.FullAddress, network tcpip
 }
 
 type GonetTCPConn struct {
-	net.Conn
+	*gonet.Conn
 	ep tcpip.Endpoint
 }
 

tests/base.py

@@ -226,7 +226,6 @@ def assertTcpRefusedError(self, ip="127.0.0.1", port=0):
     def assertStartSync(self, p):
         self.assertIn("[.] Join", p.stderr_line())
         self.assertIn("[.] Opening tun", p.stderr_line())
-        self.assertIn("[.] Restoring roo", p.stderr_line())
         self.assertIn("Started", p.stderr_line())
 
     def assertListenLine(self, p, in_pattern):

tests/test_basic.py

@@ -37,7 +37,6 @@ def test_local_fwd_error_tcp(self):
         p = self.prun("-L %s -L %s" % (port, port))
         self.assertIn("[.] Join", p.stderr_line())
         self.assertIn("[.] Opening tun", p.stderr_line())
-        self.assertIn("[.] Restoring roo", p.stderr_line())
         xport = self.assertListenLine(p, "local-fwd Local listen tcp://127.0.0.1")
         self.assertEqual(port, xport)
         # [!] Failed to listen on tcp://127.0.0.1:45295
@@ -49,7 +48,6 @@ def test_local_fwd_error_udp(self):
         p = self.prun("-L udp://%s -L udp://%s" % (port, port))
         self.assertIn("[.] Join", p.stderr_line())
         self.assertIn("[.] Opening tun", p.stderr_line())
-        self.assertIn("[.] Restoring roo", p.stderr_line())
         xport = self.assertListenLine(p, "local-fwd Local listen udp://127.0.0.1")
         self.assertEqual(port, xport)
         # [!] Failed to listen on udp://127.0.0.1:45295