diff --git a/make/test/JtregNativeJdk.gmk b/make/test/JtregNativeJdk.gmk index 60a88ca1c9a..f5970527648 100644 --- a/make/test/JtregNativeJdk.gmk +++ b/make/test/JtregNativeJdk.gmk @@ -62,7 +62,7 @@ BUILD_JDK_JTREG_LIBRARIES_JDK_LIBS_libGetXSpace := java.base:libjava ifeq ($(call isTargetOs, windows), true) BUILD_JDK_JTREG_EXCLUDE += libDirectIO.c libInheritedChannel.c \ libExplicitAttach.c libImplicitAttach.c \ - exelauncher.c + exelauncher.c libFDLeaker.c exeFDLeakTester.c BUILD_JDK_JTREG_EXECUTABLES_LIBS_exeNullCallerTest := $(LIBCXX) BUILD_JDK_JTREG_EXECUTABLES_LIBS_exerevokeall := advapi32.lib diff --git a/src/java.base/unix/native/libjava/childproc.c b/src/java.base/unix/native/libjava/childproc.c index 7a21b86565f..0dc0788879e 100644 --- a/src/java.base/unix/native/libjava/childproc.c +++ b/src/java.base/unix/native/libjava/childproc.c @@ -52,6 +52,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) { @@ -68,21 +83,15 @@ 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 */ + /* This function marks all file descriptors beyond stderr as CLOEXEC. + * That includes the file descriptor used for the fail pipe: we want that + * one to stay open up until the execve, but it should be closed with the + * execve. */ + const int fd_from = STDERR_FILENO + 1; #if defined(_AIX) /* AIX does not understand '/proc/self' - it requires the real process ID */ @@ -91,18 +100,22 @@ 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)) >= fd_from) { + if (markCloseOnExec(fd) == -1) { + closedir(dp); + return -1; + } + } } closedir(dp); - return 1; + return 0; } static int @@ -394,11 +407,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) + for (fd = STDERR_FILENO + 1; fd < max_fd; fd++) + if (markCloseOnExec(fd) == -1 && errno != EBADF) goto WhyCantJohnnyExec; } @@ -413,9 +426,6 @@ childProcess(void *arg) sigprocmask(SIG_SETMASK, &unblock_signals, NULL); } - if (fcntl(FAIL_FILENO, F_SETFD, FD_CLOEXEC) == -1) - goto WhyCantJohnnyExec; - JDK_execvpe(p->mode, p->argv[0], p->argv, p->envv); WhyCantJohnnyExec: diff --git a/test/jdk/java/lang/ProcessBuilder/FDLeakTest/FDLeakTest.java b/test/jdk/java/lang/ProcessBuilder/FDLeakTest/FDLeakTest.java new file mode 100644 index 00000000000..146c2be563f --- /dev/null +++ b/test/jdk/java/lang/ProcessBuilder/FDLeakTest/FDLeakTest.java @@ -0,0 +1,72 @@ +/* + * 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 id=posix_spawn + * @summary Check that we don't leak FDs + * @requires os.family != "windows" + * @library /test/lib + * @run main/othervm/native -Djdk.lang.Process.launchMechanism=posix_spawn -agentlib:FDLeaker FDLeakTest + */ + +/** + * @test id=fork + * @summary Check that we don't leak FDs + * @requires os.family != "windows" + * @library /test/lib + * @run main/othervm/native -Djdk.lang.Process.launchMechanism=fork -agentlib:FDLeaker FDLeakTest + */ + +/** + * @test id=vfork + * @summary Check that we don't leak FDs + * @requires os.family == "linux" + * @library /test/lib + * @run main/othervm/native -Djdk.lang.Process.launchMechanism=vfork -agentlib:FDLeaker FDLeakTest + */ + +import jdk.test.lib.process.ProcessTools; +public class FDLeakTest { + // This test has two native parts: + // - a library invoked with -agentlib that ensures that, in the parent JVM, we open + // a native fd 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. + // + // What should happen: In the child process, between the initial fork and the exec of + // the target binary, we should close all filedescriptors that are not stdin/out/err. + // If that works, the child process should not see any other file descriptors save + // those three. + 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"); + } + } +} diff --git a/test/jdk/java/lang/ProcessBuilder/FDLeakTest/exeFDLeakTester.c b/test/jdk/java/lang/ProcessBuilder/FDLeakTest/exeFDLeakTester.c new file mode 100644 index 00000000000..b47228ee5de --- /dev/null +++ b/test/jdk/java/lang/ProcessBuilder/FDLeakTest/exeFDLeakTester.c @@ -0,0 +1,56 @@ +/* + * 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 +#include +#include +#include +#include + +/* Check if any fd past stderr is valid; if true, print warning on stderr and return -1 + * + * Note: check without accessing /proc since: + * - non-portable + * - may cause creation of temporary file descriptors + */ +int main(int argc, char** argv) { + int errors = 0; + int rc = 0; + char buf[128]; + int max_fd = (int)sysconf(_SC_OPEN_MAX); + if (max_fd == -1) { + snprintf(buf, sizeof(buf), "*** sysconf(_SC_OPEN_MAX) failed? (%d) ***\n", errno); + rc = write(2, buf, strlen(buf)); + max_fd = 10000; + } + // We start after stderr fd + for (int fd = 3; fd < max_fd; fd++) { + if (fcntl(fd, F_GETFD, 0) >= 0) { + // Error: found valid file descriptor + errors++; + snprintf(buf, sizeof(buf), "*** Parent leaked file descriptor %d ***\n", fd); + rc = write(2, buf, strlen(buf)); + } + } + return errors > 0 ? -1 : 0; +} diff --git a/test/jdk/java/lang/ProcessBuilder/FDLeakTest/libFDLeaker.c b/test/jdk/java/lang/ProcessBuilder/FDLeakTest/libFDLeaker.c new file mode 100644 index 00000000000..b3fa296cae2 --- /dev/null +++ b/test/jdk/java/lang/ProcessBuilder/FDLeakTest/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 +#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; +}