os: dup pidfd if caller sets PidFD manually

Fixes golang#68984
Change-Id: I16d25777cb38a337cd4204a8147eaf866c3df9e1
Reviewed-on: https://go-review.googlesource.com/c/go/+/607695
Reviewed-by: Kirill Kolyshkin <[email protected]>
Reviewed-by: Michael Pratt <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Commit-Queue: Ian Lance Taylor <[email protected]>
Reviewed-by: David Chase <[email protected]>
Auto-Submit: Ian Lance Taylor <[email protected]>

src/os/exec_posix.go

@@ -35,10 +35,11 @@ func startProcess(name string, argv []string, attr *ProcAttr) (p *Process, err e
 		}
 	}
 
+	attrSys, shouldDupPidfd := ensurePidfd(attr.Sys)
 	sysattr := &syscall.ProcAttr{
 		Dir: attr.Dir,
 		Env: attr.Env,
-		Sys: ensurePidfd(attr.Sys),
+		Sys: attrSys,
 	}
 	if sysattr.Env == nil {
 		sysattr.Env, err = execenv.Default(sysattr.Sys)
@@ -63,7 +64,7 @@ func startProcess(name string, argv []string, attr *ProcAttr) (p *Process, err e
 	// For Windows, syscall.StartProcess above already returned a process handle.
 	if runtime.GOOS != "windows" {
 		var ok bool
-		h, ok = getPidfd(sysattr.Sys)
+		h, ok = getPidfd(sysattr.Sys, shouldDupPidfd)
 		if !ok {
 			return newPIDProcess(pid), nil
 		}

src/os/pidfd_linux.go

@@ -19,33 +19,46 @@ import (
 	"unsafe"
 )
 
-func ensurePidfd(sysAttr *syscall.SysProcAttr) *syscall.SysProcAttr {
+// ensurePidfd initializes the PidFD field in sysAttr if it is not already set.
+// It returns the original or modified SysProcAttr struct and a flag indicating
+// whether the PidFD should be duplicated before using.
+func ensurePidfd(sysAttr *syscall.SysProcAttr) (*syscall.SysProcAttr, bool) {
 	if !pidfdWorks() {
-		return sysAttr
+		return sysAttr, false
 	}
 
 	var pidfd int
 
 	if sysAttr == nil {
 		return &syscall.SysProcAttr{
 			PidFD: &pidfd,
-		}
+		}, false
 	}
 	if sysAttr.PidFD == nil {
 		newSys := *sysAttr // copy
 		newSys.PidFD = &pidfd
-		return &newSys
+		return &newSys, false
 	}
 
-	return sysAttr
+	return sysAttr, true
 }
 
-func getPidfd(sysAttr *syscall.SysProcAttr) (uintptr, bool) {
+// getPidfd returns the value of sysAttr.PidFD (or its duplicate if needDup is
+// set) and a flag indicating whether the value can be used.
+func getPidfd(sysAttr *syscall.SysProcAttr, needDup bool) (uintptr, bool) {
 	if !pidfdWorks() {
 		return 0, false
 	}
 
-	return uintptr(*sysAttr.PidFD), true
+	h := *sysAttr.PidFD
+	if needDup {
+		dupH, e := unix.Fcntl(h, syscall.F_DUPFD_CLOEXEC, 0)
+		if e != nil {
+			return 0, false
+		}
+		h = dupH
+	}
+	return uintptr(h), true
 }
 
 func pidfdFind(pid int) (uintptr, error) {

src/os/pidfd_linux_test.go

@@ -6,6 +6,7 @@ package os_test
 
 import (
 	"errors"
+	"internal/syscall/unix"
 	"internal/testenv"
 	"os"
 	"syscall"
@@ -57,3 +58,34 @@ func TestFindProcessViaPidfd(t *testing.T) {
 		t.Fatalf("Release: got %v, want <nil>", err)
 	}
 }
+
+func TestStartProcessWithPidfd(t *testing.T) {
+	testenv.MustHaveGoBuild(t)
+	t.Parallel()
+
+	if err := os.CheckPidfdOnce(); err != nil {
+		// Non-pidfd code paths tested in exec_unix_test.go.
+		t.Skipf("skipping: pidfd not available: %v", err)
+	}
+
+	var pidfd int
+	p, err := os.StartProcess(testenv.GoToolPath(t), []string{"go"}, &os.ProcAttr{
+		Sys: &syscall.SysProcAttr{
+			PidFD: &pidfd,
+		},
+	})
+	if err != nil {
+		t.Fatalf("starting test process: %v", err)
+	}
+	defer syscall.Close(pidfd)
+
+	if _, err := p.Wait(); err != nil {
+		t.Fatalf("Wait: got %v, want <nil>", err)
+	}
+
+	// Check the pidfd is still valid
+	err = unix.PidFDSendSignal(uintptr(pidfd), syscall.Signal(0))
+	if !errors.Is(err, syscall.ESRCH) {
+		t.Errorf("SendSignal: got %v, want %v", err, syscall.ESRCH)
+	}
+}

src/os/pidfd_other.go

@@ -8,11 +8,11 @@ package os
 
 import "syscall"
 
-func ensurePidfd(sysAttr *syscall.SysProcAttr) *syscall.SysProcAttr {
-	return sysAttr
+func ensurePidfd(sysAttr *syscall.SysProcAttr) (*syscall.SysProcAttr, bool) {
+	return sysAttr, false
 }
 
-func getPidfd(_ *syscall.SysProcAttr) (uintptr, bool) {
+func getPidfd(_ *syscall.SysProcAttr, _ bool) (uintptr, bool) {
 	return 0, false
 }