From: Kir Kolyshkin <kolyshkin@gmail.com>
Date: Thu, 16 Nov 2023 08:42:04 +0000 (-0800)
Subject: syscall: fix getting pidfd when using CLONE_NEWUSER
X-Git-Tag: go1.22rc1~206
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=f7b2779086683bf00570427ce08bebfb54c53b76;p=gostls13.git

syscall: fix getting pidfd when using CLONE_NEWUSER

While working on CL 528798, I found out that sys.PidFD field (added
in CL 520266) is not filled in when CLONE_NEWUSER is used.

This happens because the code assumed that the parent and the child
run in the same memory space. This assumption is right only when
CLONE_VM is used for clone syscall, and the code only sets CLONE_VM
when CLONE_NEWUSER is not used.

Fix this, and add a test case (which fails before the fix).

Updates #51246.

Change-Id: I805203c1369cadd63d769568b132a9ffd92cc184
Reviewed-on: https://go-review.googlesource.com/c/go/+/542698
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
---

diff --git a/src/syscall/exec_linux.go b/src/syscall/exec_linux.go
index e1c71b5a34..e6d6343ed8 100644
--- a/src/syscall/exec_linux.go
+++ b/src/syscall/exec_linux.go
@@ -133,7 +133,7 @@ func runtime_AfterForkInChild()
 func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr *ProcAttr, sys *SysProcAttr, pipe int) (pid int, err Errno) {
 	// Set up and fork. This returns immediately in the parent or
 	// if there's an error.
-	upid, err, mapPipe, locked := forkAndExecInChild1(argv0, argv, envv, chroot, dir, attr, sys, pipe)
+	upid, pidfd, err, mapPipe, locked := forkAndExecInChild1(argv0, argv, envv, chroot, dir, attr, sys, pipe)
 	if locked {
 		runtime_AfterFork()
 	}
@@ -143,6 +143,9 @@ func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr
 
 	// parent; return PID
 	pid = int(upid)
+	if sys.PidFD != nil {
+		*sys.PidFD = int(pidfd)
+	}
 
 	if sys.UidMappings != nil || sys.GidMappings != nil {
 		Close(mapPipe[0])
@@ -210,7 +213,7 @@ type cloneArgs struct {
 //go:noinline
 //go:norace
 //go:nocheckptr
-func forkAndExecInChild1(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr *ProcAttr, sys *SysProcAttr, pipe int) (pid uintptr, err1 Errno, mapPipe [2]int, locked bool) {
+func forkAndExecInChild1(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr *ProcAttr, sys *SysProcAttr, pipe int) (pid uintptr, pidfd int32, err1 Errno, mapPipe [2]int, locked bool) {
 	// Defined in linux/prctl.h starting with Linux 4.3.
 	const (
 		PR_CAP_AMBIENT       = 0x2f
@@ -241,12 +244,12 @@ func forkAndExecInChild1(argv0 *byte, argv, envv []*byte, chroot, dir *byte, att
 		uidmap, setgroups, gidmap []byte
 		clone3                    *cloneArgs
 		pgrp                      int32
-		pidfd                     _C_int = -1
 		dirfd                     int
 		cred                      *Credential
 		ngroups, groups           uintptr
 		c                         uintptr
 	)
+	pidfd = -1
 
 	rlim := origRlimitNofile.Load()
 
@@ -341,10 +344,6 @@ func forkAndExecInChild1(argv0 *byte, argv, envv []*byte, chroot, dir *byte, att
 
 	// Fork succeeded, now in child.
 
-	if sys.PidFD != nil {
-		*sys.PidFD = int(pidfd)
-	}
-
 	// Enable the "keep capabilities" flag to set ambient capabilities later.
 	if len(sys.AmbientCaps) > 0 {
 		_, _, err1 = RawSyscall6(SYS_PRCTL, PR_SET_KEEPCAPS, 1, 0, 0, 0, 0)
diff --git a/src/syscall/exec_linux_test.go b/src/syscall/exec_linux_test.go
index f255930aa8..a7af00d2c0 100644
--- a/src/syscall/exec_linux_test.go
+++ b/src/syscall/exec_linux_test.go
@@ -522,7 +522,7 @@ func TestCloneTimeNamespace(t *testing.T) {
 	}
 }
 
-func testPidFD(t *testing.T) error {
+func testPidFD(t *testing.T, userns bool) error {
 	testenv.MustHaveExec(t)
 
 	if os.Getenv("GO_WANT_HELPER_PROCESS") == "1" {
@@ -541,6 +541,9 @@ func testPidFD(t *testing.T) error {
 	cmd.SysProcAttr = &syscall.SysProcAttr{
 		PidFD: &pidfd,
 	}
+	if userns {
+		cmd.SysProcAttr.Cloneflags = syscall.CLONE_NEWUSER
+	}
 	if err := cmd.Start(); err != nil {
 		return err
 	}
@@ -572,7 +575,13 @@ func testPidFD(t *testing.T) error {
 }
 
 func TestPidFD(t *testing.T) {
-	if err := testPidFD(t); err != nil {
+	if err := testPidFD(t, false); err != nil {
+		t.Fatal("can't start a process:", err)
+	}
+}
+
+func TestPidFDWithUserNS(t *testing.T) {
+	if err := testPidFD(t, true); err != nil {
 		t.Fatal("can't start a process:", err)
 	}
 }
@@ -581,7 +590,7 @@ func TestPidFDClone3(t *testing.T) {
 	*syscall.ForceClone3 = true
 	defer func() { *syscall.ForceClone3 = false }()
 
-	if err := testPidFD(t); err != nil {
+	if err := testPidFD(t, false); err != nil {
 		if testenv.SyscallIsNotSupported(err) {
 			t.Skip("clone3 not supported:", err)
 		}