From: Ian Lance Taylor <iant@golang.org>
Date: Fri, 6 Sep 2024 19:19:01 +0000 (-0700)
Subject: [release-branch.go1.23] syscall: on exec failure, close pidfd
X-Git-Tag: go1.23.2~3
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=c8c6f9abfbbd2f949285a527036a3d0dbd991e74;p=gostls13.git

[release-branch.go1.23] syscall: on exec failure, close pidfd

For #69284
Fixes #69402

Change-Id: I6350209302778ba5e44fa03d0b9e680d2b4ec192
Reviewed-on: https://go-review.googlesource.com/c/go/+/611495
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: roger peppe <rogpeppe@gmail.com>
Reviewed-by: Tim King <taking@google.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
(cherry picked from commit 8926ca9c5ec3ea0b51e413e87f737aeb1422ea48)
Reviewed-on: https://go-review.googlesource.com/c/go/+/613616
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
---

diff --git a/src/os/pidfd_linux_test.go b/src/os/pidfd_linux_test.go
index fa0877037b..c1f41d02d6 100644
--- a/src/os/pidfd_linux_test.go
+++ b/src/os/pidfd_linux_test.go
@@ -9,6 +9,7 @@ import (
 	"internal/syscall/unix"
 	"internal/testenv"
 	"os"
+	"os/exec"
 	"syscall"
 	"testing"
 )
@@ -89,3 +90,62 @@ func TestStartProcessWithPidfd(t *testing.T) {
 		t.Errorf("SendSignal: got %v, want %v", err, syscall.ESRCH)
 	}
 }
+
+// Issue #69284
+func TestPidfdLeak(t *testing.T) {
+	testenv.MustHaveExec(t)
+	exe, err := os.Executable()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Find the next 10 descriptors.
+	// We need to get more than one descriptor in practice;
+	// the pidfd winds up not being the next descriptor.
+	const count = 10
+	want := make([]int, count)
+	for i := range count {
+		var err error
+		want[i], err = syscall.Open(exe, syscall.O_RDONLY, 0)
+		if err != nil {
+			t.Fatal(err)
+		}
+	}
+
+	// Close the descriptors.
+	for _, d := range want {
+		syscall.Close(d)
+	}
+
+	// Start a process 10 times.
+	for range 10 {
+		// For testing purposes this has to be an absolute path.
+		// Otherwise we will fail finding the executable
+		// and won't start a process at all.
+		cmd := exec.Command("/noSuchExecutable")
+		cmd.Run()
+	}
+
+	// Open the next 10 descriptors again.
+	got := make([]int, count)
+	for i := range count {
+		var err error
+		got[i], err = syscall.Open(exe, syscall.O_RDONLY, 0)
+		if err != nil {
+			t.Fatal(err)
+		}
+	}
+
+	// Close the descriptors
+	for _, d := range got {
+		syscall.Close(d)
+	}
+
+	t.Logf("got %v", got)
+	t.Logf("want %v", want)
+
+	// Allow some slack for runtime epoll descriptors and the like.
+	if got[count-1] > want[count-1]+5 {
+		t.Errorf("got descriptor %d, want %d", got[count-1], want[count-1])
+	}
+}
diff --git a/src/syscall/exec_bsd.go b/src/syscall/exec_bsd.go
index 149cc2f11c..bbdab46de4 100644
--- a/src/syscall/exec_bsd.go
+++ b/src/syscall/exec_bsd.go
@@ -293,3 +293,8 @@ childerror:
 		RawSyscall(SYS_EXIT, 253, 0, 0)
 	}
 }
+
+// forkAndExecFailureCleanup cleans up after an exec failure.
+func forkAndExecFailureCleanup(attr *ProcAttr, sys *SysProcAttr) {
+	// Nothing to do.
+}
diff --git a/src/syscall/exec_freebsd.go b/src/syscall/exec_freebsd.go
index 3226cb88cd..686fd23bef 100644
--- a/src/syscall/exec_freebsd.go
+++ b/src/syscall/exec_freebsd.go
@@ -317,3 +317,8 @@ childerror:
 		RawSyscall(SYS_EXIT, 253, 0, 0)
 	}
 }
+
+// forkAndExecFailureCleanup cleans up after an exec failure.
+func forkAndExecFailureCleanup(attr *ProcAttr, sys *SysProcAttr) {
+	// Nothing to do.
+}
diff --git a/src/syscall/exec_libc.go b/src/syscall/exec_libc.go
index 768e8c131c..0e88650873 100644
--- a/src/syscall/exec_libc.go
+++ b/src/syscall/exec_libc.go
@@ -314,6 +314,11 @@ childerror:
 	}
 }
 
+// forkAndExecFailureCleanup cleans up after an exec failure.
+func forkAndExecFailureCleanup(attr *ProcAttr, sys *SysProcAttr) {
+	// Nothing to do.
+}
+
 func ioctlPtr(fd, req uintptr, arg unsafe.Pointer) (err Errno) {
 	return ioctl(fd, req, uintptr(arg))
 }
diff --git a/src/syscall/exec_libc2.go b/src/syscall/exec_libc2.go
index 7a67500844..a0579627a3 100644
--- a/src/syscall/exec_libc2.go
+++ b/src/syscall/exec_libc2.go
@@ -289,3 +289,8 @@ childerror:
 		rawSyscall(abi.FuncPCABI0(libc_exit_trampoline), 253, 0, 0)
 	}
 }
+
+// forkAndExecFailureCleanup cleans up after an exec failure.
+func forkAndExecFailureCleanup(attr *ProcAttr, sys *SysProcAttr) {
+	// Nothing to do.
+}
diff --git a/src/syscall/exec_linux.go b/src/syscall/exec_linux.go
index e4b9ce1bf4..2684412191 100644
--- a/src/syscall/exec_linux.go
+++ b/src/syscall/exec_linux.go
@@ -735,3 +735,11 @@ func writeUidGidMappings(pid int, sys *SysProcAttr) error {
 
 	return nil
 }
+
+// forkAndExecFailureCleanup cleans up after an exec failure.
+func forkAndExecFailureCleanup(attr *ProcAttr, sys *SysProcAttr) {
+	if sys.PidFD != nil && *sys.PidFD != -1 {
+		Close(*sys.PidFD)
+		*sys.PidFD = -1
+	}
+}
diff --git a/src/syscall/exec_unix.go b/src/syscall/exec_unix.go
index 1b90aa7e72..4747fa0758 100644
--- a/src/syscall/exec_unix.go
+++ b/src/syscall/exec_unix.go
@@ -237,6 +237,10 @@ func forkExec(argv0 string, argv []string, attr *ProcAttr) (pid int, err error)
 		for err1 == EINTR {
 			_, err1 = Wait4(pid, &wstatus, 0, nil)
 		}
+
+		// OS-specific cleanup on failure.
+		forkAndExecFailureCleanup(attr, sys)
+
 		return 0, err
 	}