re PR libgcj/31228 (Race condition between setting close-on-exec and Runtime.exec())

	PR libgcj/31228
	* configure.ac: Add checks for getrlimit and sys/resource.h.
	* include/posix.h (_Jv_platform_close_on_exec): Remove.
	* include/config.h.in: Regenerate.
	* configure: Regenerate.
	* gnu/java/nio/channels/natFileChannelPosix.cc (open): Remove call to
	_Jv_platform_close_on_exec;
	* gnu/java/net/natPlainSocketImplPosix.cc (create): Likewise.
	(accept): Likewise.
	* gnu/java/net/natPlainDatagramSocketImplPosix.cc (create):Likewise.
	* java/lang/natPosixProcess.cc: Include sys/resource.h.
	(nativeSpawn): Close all file descriptors.  Don't set FD_CLOEXEC on
	pipes.

From-SVN: r123138

diff --git a/libjava/ChangeLog b/libjava/ChangeLog
index 03ac64e880e0d2657b0ebfc27f505c36f68b010c..281fb22fd388930d7376f7b215f05a5c9830a6e6 100644 (file)
--- a/libjava/ChangeLog
+++ b/libjava/ChangeLog
@@ -1,3 +1,19 @@
+2007-03-22  David Daney  <ddaney@avtrex.com>
+
+       PR libgcj/31228
+       * configure.ac: Add checks for getrlimit and sys/resource.h.
+       * include/posix.h (_Jv_platform_close_on_exec): Remove.
+       * include/config.h.in: Regenerate.
+       * configure: Regenerate.
+       * gnu/java/nio/channels/natFileChannelPosix.cc (open): Remove call to
+       _Jv_platform_close_on_exec;
+       * gnu/java/net/natPlainSocketImplPosix.cc (create): Likewise.
+       (accept): Likewise.
+       * gnu/java/net/natPlainDatagramSocketImplPosix.cc (create):Likewise.
+       * java/lang/natPosixProcess.cc: Include sys/resource.h.
+       (nativeSpawn): Close all file descriptors.  Don't set FD_CLOEXEC on
+       pipes.
+
 2007-03-20  Andrew Haley  <aph@redhat.com>
 
        * testsuite/libjava.lang/PR31264.java: New test.

diff --git a/libjava/configure b/libjava/configure
index 8a81965a5ec7533152e5f83f3dcc77149b3abdec..79806d34c76d45e7e3f9f86f9b1334c840db67eb 100755 (executable)
--- a/libjava/configure
+++ b/libjava/configure
@@ -9600,6 +9600,7 @@ else
 
 
 
+
 
 
 for ac_func in strerror ioctl select fstat open fsync sleep opendir \
@@ -9607,7 +9608,7 @@ for ac_func in strerror ioctl select fstat open fsync sleep opendir \
                   access stat lstat mkdir rename rmdir unlink utime chmod readlink \
                   nl_langinfo setlocale \
                   inet_pton uname inet_ntoa \
-                  fork execvp pipe sigaction ftruncate mmap \
+                  fork execvp getrlimit pipe sigaction ftruncate mmap \
                   getifaddrs
 do
 as_ac_var=`echo "ac_cv_func_$ac_func" | $as_tr_sh`
@@ -10063,7 +10064,8 @@ done
 
 
 
-for ac_header in execinfo.h unistd.h dlfcn.h
+
+for ac_header in execinfo.h unistd.h dlfcn.h sys/resource.h
 do
 as_ac_Header=`echo "ac_cv_header_$ac_header" | $as_tr_sh`
 if eval "test \"\${$as_ac_Header+set}\" = set"; then

diff --git a/libjava/configure.ac b/libjava/configure.ac
index d21654ae00becac18bac76b3272d3b622a197ccd..f5e27fab31307226420ccdaebec9aefeeca3f2be 100644 (file)
--- a/libjava/configure.ac
+++ b/libjava/configure.ac
@@ -1006,10 +1006,10 @@ else
                   access stat lstat mkdir rename rmdir unlink utime chmod readlink \
                   nl_langinfo setlocale \
                   inet_pton uname inet_ntoa \
-                  fork execvp pipe sigaction ftruncate mmap \
+                  fork execvp getrlimit pipe sigaction ftruncate mmap \
                   getifaddrs])
    AC_CHECK_FUNCS(inet_aton inet_addr, break)
-   AC_CHECK_HEADERS(execinfo.h unistd.h dlfcn.h)
+   AC_CHECK_HEADERS(execinfo.h unistd.h dlfcn.h sys/resource.h)
    # Do an additional check on dld, HP-UX for example has dladdr in libdld.sl
    AC_CHECK_LIB(dl, dladdr, [
        AC_DEFINE(HAVE_DLADDR, 1, [Define if you have dladdr()])], [

diff --git a/libjava/gnu/java/net/natPlainDatagramSocketImplPosix.cc b/libjava/gnu/java/net/natPlainDatagramSocketImplPosix.cc
index f7ffaa895c3f2f43151e4a0b221ccaaf5a1dbc9c..7cbf011ab50521d985f21549c72bbcb0d862b692 100644 (file)
--- a/libjava/gnu/java/net/natPlainDatagramSocketImplPosix.cc
+++ b/libjava/gnu/java/net/natPlainDatagramSocketImplPosix.cc
@@ -83,8 +83,6 @@ gnu::java::net::PlainDatagramSocketImpl::create ()
       throw new ::java::net::SocketException (JvNewStringUTF (strerr));
     }
 
-  _Jv_platform_close_on_exec (sock);
-
   // We use native_fd in place of fd here.  From leaving fd null we avoid
   // the double close problem in FileDescriptor.finalize.
   native_fd = sock;

diff --git a/libjava/gnu/java/net/natPlainSocketImplPosix.cc b/libjava/gnu/java/net/natPlainSocketImplPosix.cc
index 9fc619649d3c4c0a283a6ac91df8c417849a7858..d16f1d31f7b06adbb2c2560713760d6d7c5c2551 100644 (file)
--- a/libjava/gnu/java/net/natPlainSocketImplPosix.cc
+++ b/libjava/gnu/java/net/natPlainSocketImplPosix.cc
@@ -72,8 +72,6 @@ gnu::java::net::PlainSocketImpl::create (jboolean stream)
       throw new ::java::io::IOException (JvNewStringUTF (strerr));
     }
 
-  _Jv_platform_close_on_exec (sock);
-
   // We use native_fd in place of fd here.  From leaving fd null we avoid
   // the double close problem in FileDescriptor.finalize.
   native_fd = sock;
@@ -285,8 +283,6 @@ gnu::java::net::PlainSocketImpl::accept (gnu::java::net::PlainSocketImpl *s)
   if (new_socket < 0)
     goto error;
 
-  _Jv_platform_close_on_exec (new_socket);
-
   jbyteArray raddr;
   jint rport;
   if (u.address.sin_family == AF_INET)

diff --git a/libjava/gnu/java/nio/channels/natFileChannelPosix.cc b/libjava/gnu/java/nio/channels/natFileChannelPosix.cc
index 4851403a8db4171dd2648f01ee0ebb6b620d0ad4..52caf828345c173b324ff3a129deb4904b7eeb71 100644 (file)
--- a/libjava/gnu/java/nio/channels/natFileChannelPosix.cc
+++ b/libjava/gnu/java/nio/channels/natFileChannelPosix.cc
@@ -178,8 +178,6 @@ FileChannelImpl::open (jstring path, jint jflags)
       throw new ::java::io::FileNotFoundException (msg->toString ());
     }
 
-  _Jv_platform_close_on_exec (fd);
-
   return fd;
 }
 

diff --git a/libjava/include/config.h.in b/libjava/include/config.h.in
index 2c025a27af5b9468aa6bc249e1cd3ad3af44f127..f0919e2673cf9cfc2d6770dd680cf181344efdd6 100644 (file)
--- a/libjava/include/config.h.in
+++ b/libjava/include/config.h.in
@@ -127,6 +127,9 @@
 /* Define to 1 if you have the `getpwuid_r' function. */
 #undef HAVE_GETPWUID_R
 
+/* Define to 1 if you have the `getrlimit' function. */
+#undef HAVE_GETRLIMIT
+
 /* Define to 1 if you have the `gettimeofday' function. */
 #undef HAVE_GETTIMEOFDAY
 
@@ -316,6 +319,9 @@
 /* Define to 1 if you have the <sys/ioctl.h> header file. */
 #undef HAVE_SYS_IOCTL_H
 
+/* Define to 1 if you have the <sys/resource.h> header file. */
+#undef HAVE_SYS_RESOURCE_H
+
 /* Define to 1 if you have the <sys/rw_lock.h> header file. */
 #undef HAVE_SYS_RW_LOCK_H
 

diff --git a/libjava/include/posix.h b/libjava/include/posix.h
index ee836e0b809496c115c8a1293daeaf00a9e7531f..5f522a3804aff679fe521598cfc90444e22e1281 100644 (file)
--- a/libjava/include/posix.h
+++ b/libjava/include/posix.h
@@ -98,15 +98,6 @@ extern jlong _Jv_platform_nanotime ();
 extern void _Jv_platform_initialize (void);
 extern void _Jv_platform_initProperties (java::util::Properties*);
 
-inline void
-_Jv_platform_close_on_exec (jint fd)
-{
-  // Ignore errors.
-  ::fcntl (fd, F_SETFD, FD_CLOEXEC);
-}
-
-#undef fcntl
-
 #ifdef JV_HASH_SYNCHRONIZATION
 #ifndef HAVE_USLEEP_DECL
 extern "C" int usleep (useconds_t useconds);

diff --git a/libjava/java/lang/natPosixProcess.cc b/libjava/java/lang/natPosixProcess.cc
index 149b5d8ba3416bc6999f852341f5f56d9822d79c..6763273546cc5d68c48b19ad0418a1cf10fc9846 100644 (file)
--- a/libjava/java/lang/natPosixProcess.cc
+++ b/libjava/java/lang/natPosixProcess.cc
@@ -17,6 +17,9 @@ details.  */
 #include <fcntl.h>
 #include <sys/types.h>
 #include <sys/wait.h>
+#ifdef HAVE_SYS_RESOURCE_H
+#include <sys/resource.h>
+#endif
 #include <signal.h>
 #include <string.h>
 #include <stdlib.h>
@@ -352,7 +355,31 @@ java::lang::PosixProcess::nativeSpawn ()
                  _exit (127);
                }
            }
-
+          // Make sure all file descriptors are closed.  In
+          // multi-threaded programs, there is a race between when a
+          // descriptor is obtained, when we can set FD_CLOEXEC, and
+          // fork().  If the fork occurs before FD_CLOEXEC is set, the
+          // descriptor would leak to the execed process if we did not
+          // manually close it.  So that is what we do.  Since we
+          // close all the descriptors, it is redundant to set
+          // FD_CLOEXEC on them elsewhere.
+          int max_fd;
+#ifdef HAVE_GETRLIMIT
+          rlimit rl;
+          int rv = getrlimit(RLIMIT_NOFILE, &rl);
+          if (rv == 0)
+            max_fd = rl.rlim_max - 1;
+          else
+            max_fd = 1024 - 1;
+#else
+          max_fd = 1024 - 1;
+#endif
+          while(max_fd > 2)
+            {
+              if (max_fd != msgp[1])
+                close (max_fd);
+              max_fd--;
+            }
          // Make sure that SIGCHLD is unblocked for the new process.
          sigset_t mask;
          sigemptyset (&mask);
@@ -438,12 +465,4 @@ java::lang::PosixProcess::nativeSpawn ()
 
   myclose (msgp[0]);
   cleanup (args, env, path);
-
-  if (exception == NULL)
-    {
-      fcntl (outp[1], F_SETFD, FD_CLOEXEC);
-      fcntl (inp[0], F_SETFD, FD_CLOEXEC);
-      if (! redirect)
-       fcntl (errp[0], F_SETFD, FD_CLOEXEC);
-    }
 }