Mark fds with cloexec, plus test

Author: tstuefe

src/java.base/unix/native/libjava/childproc.c

@@ -51,6 +51,21 @@ closeSafely(int fd)
     return (fd == -1) ? 0 : close(fd);
 }
 
+int
+markCloseOnExec(int fd)
+{
+    const int flags = fcntl(fd, F_GETFD);
+    if (flags < 0) {
+        return -1;
+    }
+    if ((flags & FD_CLOEXEC) == 0) {
+        if (fcntl(fd, F_SETFD, flags | FD_CLOEXEC) < 0) {
+            return -1;
+        }
+    }
+    return 0;
+}
+
 static int
 isAsciiDigit(char c)
 {
@@ -67,21 +82,10 @@ isAsciiDigit(char c)
 #endif
 
 static int
-closeDescriptors(void)
+markDescriptorsCloseOnExec(void)
 {
     DIR *dp;
     struct dirent *dirp;
-    int from_fd = FAIL_FILENO + 1;
-
-    /* We're trying to close all file descriptors, but opendir() might
-     * itself be implemented using a file descriptor, and we certainly
-     * don't want to close that while it's in use.  We assume that if
-     * opendir() is implemented using a file descriptor, then it uses
-     * the lowest numbered file descriptor, just like open().  So we
-     * close a couple explicitly.  */
-
-    close(from_fd);          /* for possible use by opendir() */
-    close(from_fd + 1);      /* another one for good luck */
 
 #if defined(_AIX)
     /* AIX does not understand '/proc/self' - it requires the real process ID */
@@ -90,18 +94,21 @@ closeDescriptors(void)
 #endif
 
     if ((dp = opendir(FD_DIR)) == NULL)
-        return 0;
+        return -1;
 
     while ((dirp = readdir(dp)) != NULL) {
         int fd;
         if (isAsciiDigit(dirp->d_name[0]) &&
-            (fd = strtol(dirp->d_name, NULL, 10)) >= from_fd + 2)
-            close(fd);
+            (fd = strtol(dirp->d_name, NULL, 10)) > STDERR_FILENO) {
+            if (markCloseOnExec(fd) == -1) {
+                return -1;
+            }
+        }
     }
 
     closedir(dp);
 
-    return 1;
+    return 0;
 }
 
 static int
@@ -393,11 +400,11 @@ childProcess(void *arg)
     fail_pipe_fd = FAIL_FILENO;
 
     /* close everything */
-    if (closeDescriptors() == 0) { /* failed,  close the old way */
+    if (markDescriptorsCloseOnExec() == -1) { /* failed,  close the old way */
         int max_fd = (int)sysconf(_SC_OPEN_MAX);
         int fd;
         for (fd = FAIL_FILENO + 1; fd < max_fd; fd++)
-            if (close(fd) == -1 && errno != EBADF)
+            if (markCloseOnExec(fd) == -1 && errno != EBADF)
                 goto WhyCantJohnnyExec;
     }
 

test/jdk/java/lang/ProcessBuilder/FDLeakTest.java

@@ -0,0 +1,50 @@
+/*
+ * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+
+/**
+ * @test
+ * @summary Check that we don't leak FDs
+ * @library /test/lib
+ * @run main/othervm/native -agentlib:FDLeaker FDLeakTest
+ */
+
+import jdk.test.lib.process.ProcessTools;
+public class FDLeakTest {
+    // This test has two native parts:
+    // - a library invoked with -agentlib that opens a native file without setting FD_CLOEXEC (libFDLeaker.c). This is
+    //   necessary because there is no way to do this from Java (if Java functions correctly, all files the user could
+    //   open via its APIs should be marked with FD_CLOEXEC).
+    // - a small native executable that tests - without using /proc - whether any file descriptors other than
+    //   stdin/out/err are open.
+    //
+    public static void main(String[] args) throws Exception {
+        ProcessBuilder pb = ProcessTools.createNativeTestProcessBuilder("FDLeakTester");
+        pb.inheritIO();
+        Process p = pb.start();
+        p.waitFor();
+        if (p.exitValue() != 0) {
+            throw new RuntimeException("Failed");
+        }
+    }
+}

test/jdk/java/lang/ProcessBuilder/exeFDLeakTester.c

@@ -0,0 +1,51 @@
+/*
+ * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+#include <fcntl.h>
+#include <stdio.h>
+#include <string.h>
+#include <unistd.h>
+
+/* Check if any fd past stderr is valid; if true, print warning on stderr and return -1
+ * Note: check without accessing /proc or similar techniques since those depend on OS and
+ * may cause temp. file descriptors to open */
+int main(int argc, char** argv) {
+    int max_fd = (int)sysconf(_SC_OPEN_MAX);
+    if (max_fd == -1) {
+        max_fd = 10000;
+    }
+    // We start past stderr fd
+    const int max_error = 10;
+    int errors = 0;
+    int rc = 0;
+    for (int fd = 3; fd < max_fd; fd++) {
+        if (fcntl(fd, F_GETFD, 0) >= 0) {
+            // Error: found valid file descriptor
+            errors++;
+            char buf[128];
+            snprintf(buf, sizeof(buf), "*** Parent leaked filedescriptor %d ***\n", fd);
+            rc = write(2, buf, strlen(buf));
+        }
+    }
+    return errors > 0 ? -1 : 0;
+}

test/jdk/java/lang/ProcessBuilder/libFDLeaker.c

@@ -0,0 +1,36 @@
+/*
+ * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+#include <stdio.h>
+#include "jvmti.h"
+
+JNIEXPORT jint JNICALL
+Agent_OnLoad(JavaVM *jvm, char *options, void *reserved) {
+  const char* filename = "./testfile_FDLeaker.txt";
+  FILE* f = fopen(filename, "w");
+  if (f == NULL) {
+    return JNI_ERR;
+  }
+  printf("Opened and leaked %s (%d)", filename, fileno(f));
+  return JNI_OK;
+}