[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 fa0877037baadc0fefbd39f14b4b44c461d98de1..c1f41d02d66c735f2b231672b30c7fbf1d6fd47f 100644 (file)
--- 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 149cc2f11c128c5c08a19af107a3428c43b4ceb2..bbdab46de48c03113d8a9ad4cc28f417c61da8e4 100644 (file)
--- 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 3226cb88cd999aabfd9efa8149309dff06b62ac6..686fd23bef435dea6ff4047c9a0d33be915b9eec 100644 (file)
--- 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 768e8c131c13231311629cb3c85e73ec9c1ddfe9..0e886508737d1e7310143e9a735d0111747f1a66 100644 (file)
--- 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 7a6750084486cf7566120a2b08edd78a15112cc7..a0579627a300bf5f04432d47317963d829de1ccb 100644 (file)
--- 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 e4b9ce1bf47da34845348577a8e1aaba442c24a5..26844121910e8fd4634875cfc7532eaed769f05d 100644 (file)
--- 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 1b90aa7e72e0ed839fc3f6fc8c22c180c209db18..4747fa075834af6337343427df36ba299c451dd9 100644 (file)
--- 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
        }