12 files changed, 140 insertions, 120 deletions

diff --git a/src/ChangeLog b/src/ChangeLog
index 069d39ce6be..07285d564b2 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,41 @@
+2013-07-07  Paul Eggert  <eggert@cs.ucla.edu>
+
+        Make file descriptors close-on-exec when possible (Bug#14803).
+        This simplifies Emacs a bit, since it no longer needs to worry
+        about closing file descriptors by hand in some cases.
+        It also fixes some unlikely races.  Not all such races, as
+        libraries often open files internally without setting
+        close-on-exec, but it's an improvement.
+        * alloc.c (valid_pointer_p) [!WINDOWSNT]:
+        * callproc.c (Fcall_process) [!MSDOS]:
+        * emacs.c (main) [!DOS_NT]:
+        * nsterm.m (ns_term_init):
+        * process.c (create_process):
+        Use 'pipe2' with O_CLOEXEC instead of 'pipe'.
+        * emacs.c (Fcall_process_region) [HAVE_MKOSTEMP]:
+        * filelock.c (create_lock_file) [HAVE_MKOSTEMP]:
+        Prefer mkostemp with O_CLOEXEC to mkstemp.
+        * callproc.c (relocate_fd) [!WINDOWSNT]:
+        * emacs.c (main): Use F_DUPFD_CLOEXEC, not plain F_DUPFD.
+        No need to use fcntl (..., F_SETFD, FD_CLOEXEC), since we're
+        now using pipe2.
+        * filelock.c (create_lock_file) [! HAVE_MKOSTEMP]:
+        Make the resulting file descriptor close-on-exec.
+        * lisp.h, lread.c, process.c (close_load_descs, close_process_descs):
+        * lread.c (load_descriptor_list, load_descriptor_unwind):
+        Remove; no longer needed.  All uses removed.
+        * process.c (SOCK_CLOEXEC): Define to 0 if not supplied by system.
+        (close_on_exec, accept4, process_socket) [!SOCK_CLOEXEC]:
+        New functions.
+        (socket) [!SOCK_CLOEXEC]: Supply a substitute.
+        (Fmake_network_process, Fnetwork_interface_list):
+        (Fnetwork_interface_info, server_accept_connection):
+        Make newly-created socket close-on-exec.
+        * sysdep.c (emacs_open, emacs_fopen):
+        Make new-created descriptor close-on-exec.
+        * w32.c (fcntl): Support F_DUPFD_CLOEXEC well enough for Emacs.
+        * w32.c, w32.h (pipe2): Rename from 'pipe', with new flags arg.
+
 2013-07-07  Jan Djärv  <jan.h.d@swipnet.se>
 
         * nsterm.m (sendEvent:): Propagate keyboard events to modal windows
diff --git a/src/alloc.c b/src/alloc.c
index a31a176caa7..b71cdb98d78 100644
--- a/src/alloc.c
+++ b/src/alloc.c
@@ -4741,7 +4741,7 @@ valid_pointer_p (void *p)
      Unfortunately, we cannot use NULL_DEVICE here, as emacs_write may
      not validate p in that case.  */
 
-  if (pipe (fd) == 0)
+  if (pipe2 (fd, O_CLOEXEC) == 0)
     {
       bool valid = emacs_write (fd[1], (char *) p, 16) == 16;
       emacs_close (fd[1]);
diff --git a/src/callproc.c b/src/callproc.c
index 185dc9a493e..3e70b1c2e49 100644
--- a/src/callproc.c
+++ b/src/callproc.c
@@ -519,7 +519,7 @@ usage: (call-process PROGRAM &optional INFILE DESTINATION DISPLAY &rest ARGS)  *
     {
 #ifndef MSDOS
       int fd[2];
-      if (pipe (fd) == -1)
+      if (pipe2 (fd, O_CLOEXEC) != 0)
         {
           int pipe_errno = errno;
           emacs_close (filefd);
@@ -1036,12 +1036,16 @@ usage: (call-process-region START END PROGRAM &optional DELETE BUFFER DISPLAY &r
     memcpy (tempfile, SDATA (encoded_tem), SBYTES (encoded_tem) + 1);
     coding_systems = Qt;
 
-#ifdef HAVE_MKSTEMP
+#if defined HAVE_MKOSTEMP || defined HAVE_MKSTEMP
     {
       int fd;
 
       block_input ();
+# ifdef HAVE_MKOSTEMP
+      fd = mkostemp (tempfile, O_CLOEXEC);
+# else
       fd = mkstemp (tempfile);
+# endif
       unblock_input ();
       if (fd == -1)
         report_file_error ("Failed to open temporary file",
@@ -1184,15 +1188,6 @@ child_setup (int in, int out, int err, char **new_argv, bool set_pgrp,
 
   pid_t pid = getpid ();
 
-  /* Close Emacs's descriptors that this process should not have.  */
-  close_process_descs ();
-
-  /* DOS_NT isn't in a vfork, so if we are in the middle of load-file,
-     we will lose if we call close_load_descs here.  */
-#ifndef DOS_NT
-  close_load_descs ();
-#endif
-
   /* Note that use of alloca is always safe here.  It's obvious for systems
      that do not have true vfork or that have true (stack) alloca.
      If using vfork and C_ALLOCA (when Emacs used to include
@@ -1354,9 +1349,11 @@ child_setup (int in, int out, int err, char **new_argv, bool set_pgrp,
   emacs_close (1);
   emacs_close (2);
 
+  /* Redirect file descriptors and clear FD_CLOEXEC on the redirected ones.  */
   dup2 (in, 0);
   dup2 (out, 1);
   dup2 (err, 2);
+
   emacs_close (in);
   if (out != in)
     emacs_close (out);
@@ -1394,7 +1391,7 @@ relocate_fd (int fd, int minfd)
     return fd;
   else
     {
-      int new = fcntl (fd, F_DUPFD, minfd);
+      int new = fcntl (fd, F_DUPFD_CLOEXEC, minfd);
       if (new == -1)
         {
           const char *message_1 = "Error while setting up child: ";
diff --git a/src/emacs.c b/src/emacs.c
index 6ba01d1a443..e4412e2ea1a 100644
--- a/src/emacs.c
+++ b/src/emacs.c
@@ -889,7 +889,7 @@ main (int argc, char **argv)
           emacs_close (0);
           emacs_close (1);
           result = emacs_open (term, O_RDWR, 0);
-          if (result < 0 || dup (0) < 0)
+          if (result < 0 || fcntl (0, F_DUPFD_CLOEXEC, 1) < 0)
             {
               char *errstring = strerror (errno);
               fprintf (stderr, "%s: %s: %s\n", argv[0], term, errstring);
@@ -969,7 +969,7 @@ main (int argc, char **argv)
          use a pipe for synchronization.  The parent waits for the child
          to close its end of the pipe (using `daemon-initialized')
          before exiting.  */
-      if (pipe (daemon_pipe) == -1)
+      if (pipe2 (daemon_pipe, O_CLOEXEC) != 0)
         {
           fprintf (stderr, "Cannot pipe!\n");
           exit (1);
@@ -1065,9 +1065,6 @@ Using an Emacs configured with --with-x-toolkit=lucid does not have this problem
         daemon_name = xstrdup (dname_arg);
       /* Close unused reading end of the pipe.  */
       close (daemon_pipe[0]);
-      /* Make sure that the used end of the pipe is closed on exec, so
-         that it is not accessible to programs started from .emacs.  */
-      fcntl (daemon_pipe[1], F_SETFD, FD_CLOEXEC);
 
       setsid ();
 #else /* DOS_NT */
diff --git a/src/filelock.c b/src/filelock.c
index de6aba8385c..1fcd2432484 100644
--- a/src/filelock.c
+++ b/src/filelock.c
@@ -416,8 +416,13 @@ create_lock_file (char *lfname, char *lock_info_str, bool force)
       memcpy (nonce, lfname, lfdirlen);
       strcpy (nonce + lfdirlen, nonce_base);
 
-#if HAVE_MKSTEMP
-      /* Prefer mkstemp if available, as it avoids a race between
+#if HAVE_MKOSTEMP
+      /* Prefer mkostemp to mkstemp, as it avoids a window where FD is
+         temporarily open without close-on-exec.  */
+      fd = mkostemp (nonce, O_BINARY | O_CLOEXEC);
+      need_fchmod = 1;
+#elif HAVE_MKSTEMP
+      /* Prefer mkstemp to mktemp, as it avoids a race between
          mktemp and emacs_open.  */
       fd = mkstemp (nonce);
       need_fchmod = 1;
@@ -432,7 +437,11 @@ create_lock_file (char *lfname, char *lock_info_str, bool force)
         err = errno;
       else
         {
-          ptrdiff_t lock_info_len = strlen (lock_info_str);
+          ptrdiff_t lock_info_len;
+#if ! HAVE_MKOSTEMP
+          fcntl (fd, F_SETFD, FD_CLOEXEC);
+#endif
+          lock_info_len = strlen (lock_info_str);
           err = 0;
           if (emacs_write (fd, lock_info_str, lock_info_len) != lock_info_len
               || (need_fchmod && fchmod (fd, world_readable) != 0))
diff --git a/src/lisp.h b/src/lisp.h
index 5d6fa760108..692f73cdbc3 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -3668,7 +3668,6 @@ extern Lisp_Object string_to_number (char const *, int, bool);
 extern void map_obarray (Lisp_Object, void (*) (Lisp_Object, Lisp_Object),
                          Lisp_Object);
 extern void dir_warning (const char *, Lisp_Object);
-extern void close_load_descs (void);
 extern void init_obarray (void);
 extern void init_lread (void);
 extern void syms_of_lread (void);
@@ -3995,7 +3994,6 @@ extern void delete_keyboard_wait_descriptor (int);
 extern void add_gpm_wait_descriptor (int);
 extern void delete_gpm_wait_descriptor (int);
 #endif
-extern void close_process_descs (void);
 extern void init_process_emacs (void);
 extern void syms_of_process (void);
 extern void setup_process_coding_systems (Lisp_Object);
diff --git a/src/lread.c b/src/lread.c
index 2ceeb106653..83d2e8d954a 100644
--- a/src/lread.c
+++ b/src/lread.c
@@ -95,9 +95,6 @@ static Lisp_Object Qload_in_progress;
    It must be set to nil before all top-level calls to read0.  */
 static Lisp_Object read_objects;
 
-/* List of descriptors now open for Fload.  */
-static Lisp_Object load_descriptor_list;
-
 /* File for get_file_char to read from.  Use by load.  */
 static FILE *instream;
 
@@ -149,7 +146,6 @@ static void readevalloop (Lisp_Object, FILE *, Lisp_Object, bool,
                           Lisp_Object, Lisp_Object,
                           Lisp_Object, Lisp_Object);
 static Lisp_Object load_unwind (Lisp_Object);
-static Lisp_Object load_descriptor_unwind (Lisp_Object);
 
 /* Functions that read one byte from the current source READCHARFUN
    or unreads one byte.  If the integer argument C is -1, it returns
@@ -1328,11 +1324,8 @@ Return t if the file exists and loads successfully.  */)
     }
 
   record_unwind_protect (load_unwind, make_save_pointer (stream));
-  record_unwind_protect (load_descriptor_unwind, load_descriptor_list);
   specbind (Qload_file_name, found);
   specbind (Qinhibit_file_name_operation, Qnil);
-  load_descriptor_list
-    = Fcons (make_number (fileno (stream)), load_descriptor_list);
   specbind (Qload_in_progress, Qt);
 
   instream = stream;
@@ -1395,26 +1388,6 @@ load_unwind (Lisp_Object arg)  /* Used as unwind-protect function in load.  */
     }
   return Qnil;
 }
-
-static Lisp_Object
-load_descriptor_unwind (Lisp_Object oldlist)
-{
-  load_descriptor_list = oldlist;
-  return Qnil;
-}
-
-/* Close all descriptors in use for Floads.
-   This is used when starting a subprocess.  */
-
-void
-close_load_descs (void)
-{
-#ifndef WINDOWSNT
-  Lisp_Object tail;
-  for (tail = load_descriptor_list; CONSP (tail); tail = XCDR (tail))
-    emacs_close (XFASTINT (XCAR (tail)));
-#endif
-}
 
 static bool
 complete_filename_p (Lisp_Object pathname)
@@ -4376,9 +4349,6 @@ init_lread (void)
 
   load_in_progress = 0;
   Vload_file_name = Qnil;
-
-  load_descriptor_list = Qnil;
-
   Vstandard_input = Qt;
   Vloads_in_progress = Qnil;
 }
@@ -4651,9 +4621,6 @@ variables, this must be set in the first line of a file.  */);
 
   /* Vsource_directory was initialized in init_lread.  */
 
-  load_descriptor_list = Qnil;
-  staticpro (&load_descriptor_list);
-
   DEFSYM (Qcurrent_load_list, "current-load-list");
   DEFSYM (Qstandard_input, "standard-input");
   DEFSYM (Qread_char, "read-char");
diff --git a/src/nsterm.m b/src/nsterm.m
index dfb82edd738..074a5adf78c 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -4142,7 +4142,7 @@ ns_term_init (Lisp_Object display_name)
 
   if (selfds[0] == -1)
     {
-      if (pipe (selfds) == -1)
+      if (pipe2 (selfds, O_CLOEXEC) != 0)
         {
           fprintf (stderr, "Failed to create pipe: %s\n",
                    emacs_strerror (errno));
diff --git a/src/process.c b/src/process.c
index b77fb97168a..cad42470bc1 100644
--- a/src/process.c
+++ b/src/process.c
@@ -135,6 +135,34 @@ extern int sys_select (int, SELECT_TYPE *, SELECT_TYPE *, SELECT_TYPE *,
                        EMACS_TIME *, void *);
 #endif
 
+#ifndef SOCK_CLOEXEC
+# define SOCK_CLOEXEC 0
+
+/* Emulate GNU/Linux accept4 and socket well enough for this module.  */
+
+static int
+close_on_exec (int fd)
+{
+  if (0 <= fd)
+    fcntl (fd, F_SETFD, FD_CLOEXEC);
+  return fd;
+}
+
+static int
+accept4 (int sockfd, struct sockaddr *addr, socklen_t *addrlen, int flags)
+{
+  return close_on_exec (accept (sockfd, addr, addrlen));
+}
+
+static int
+process_socket (int domain, int type, int protocol)
+{
+  return close_on_exec (socket (domain, type, protocol));
+}
+# undef socket
+# define socket(domain, type, protocol) process_socket (domain, type, protocol)
+#endif
+
 /* Work around GCC 4.7.0 bug with strict overflow checking; see
    <http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52904>.
    These lines can be removed once the GCC bug is fixed.  */
@@ -1619,14 +1647,11 @@ create_process (Lisp_Object process, char **new_argv, Lisp_Object current_dir)
   else
 #endif /* HAVE_PTYS */
     {
-      int tem;
-      tem = pipe (sv);
-      if (tem < 0)
+      if (pipe2 (sv, O_CLOEXEC) != 0)
         report_file_error ("Creating pipe", Qnil);
       inchannel = sv[0];
       forkout = sv[1];
-      tem = pipe (sv);
-      if (tem < 0)
+      if (pipe2 (sv, O_CLOEXEC) != 0)
         {
           emacs_close (inchannel);
           emacs_close (forkout);
@@ -1637,29 +1662,14 @@ create_process (Lisp_Object process, char **new_argv, Lisp_Object current_dir)
     }
 
 #ifndef WINDOWSNT
-    {
-      int tem;
-
-      tem = pipe (wait_child_setup);
-      if (tem < 0)
-        report_file_error ("Creating pipe", Qnil);
-      tem = fcntl (wait_child_setup[1], F_GETFD, 0);
-      if (tem >= 0)
-        tem = fcntl (wait_child_setup[1], F_SETFD, tem | FD_CLOEXEC);
-      if (tem < 0)
-        {
-          emacs_close (wait_child_setup[0]);
-          emacs_close (wait_child_setup[1]);
-          report_file_error ("Setting file descriptor flags", Qnil);
-        }
-    }
+  if (pipe2 (wait_child_setup, O_CLOEXEC) != 0)
+    report_file_error ("Creating pipe", Qnil);
 #endif
 
   fcntl (inchannel, F_SETFL, O_NONBLOCK);
   fcntl (outchannel, F_SETFL, O_NONBLOCK);
 
-  /* Record this as an active process, with its channels.
-     As a result, child_setup will close Emacs's side of the pipes.  */
+  /* Record this as an active process, with its channels.  */
   chan_process[inchannel] = process;
   XPROCESS (process)->infd = inchannel;
   XPROCESS (process)->outfd = outchannel;
@@ -3135,7 +3145,8 @@ usage: (make-network-process &rest ARGS)  */)
     retry_connect:
 #endif
 
-      s = socket (lres->ai_family, lres->ai_socktype, lres->ai_protocol);
+      s = socket (lres->ai_family, lres->ai_socktype | SOCK_CLOEXEC,
+                  lres->ai_protocol);
       if (s < 0)
         {
           xerrno = errno;
@@ -3532,7 +3543,7 @@ format; see the description of ADDRESS in `make-network-process'.  */)
   int s;
   Lisp_Object res;
 
-  s = socket (AF_INET, SOCK_STREAM, 0);
+  s = socket (AF_INET, SOCK_STREAM | SOCK_CLOEXEC, 0);
   if (s < 0)
     return Qnil;
 
@@ -3688,7 +3699,7 @@ FLAGS is the current flags of the interface.  */)
     error ("interface name too long");
   strcpy (rq.ifr_name, SSDATA (ifname));
 
-  s = socket (AF_INET, SOCK_STREAM, 0);
+  s = socket (AF_INET, SOCK_STREAM | SOCK_CLOEXEC, 0);
   if (s < 0)
     return Qnil;
 
@@ -3984,7 +3995,7 @@ server_accept_connection (Lisp_Object server, int channel)
   } saddr;
   socklen_t len = sizeof saddr;
 
-  s = accept (channel, &saddr.sa, &len);
+  s = accept4 (channel, &saddr.sa, &len, SOCK_CLOEXEC);
 
   if (s < 0)
     {
@@ -6858,32 +6869,6 @@ setup_process_coding_systems (Lisp_Object process)
 #endif
 }
 
-/* Close all descriptors currently in use for communication
-   with subprocess.  This is used in a newly-forked subprocess
-   to get rid of irrelevant descriptors.  */
-
-void
-close_process_descs (void)
-{
-#ifndef DOS_NT
-  int i;
-  for (i = 0; i < MAXDESC; i++)
-    {
-      Lisp_Object process;
-      process = chan_process[i];
-      if (!NILP (process))
-        {
-          int in  = XPROCESS (process)->infd;
-          int out = XPROCESS (process)->outfd;
-          if (in >= 0)
-            emacs_close (in);
-          if (out >= 0 && in != out)
-            emacs_close (out);
-        }
-    }
-#endif
-}
-
 DEFUN ("get-buffer-process", Fget_buffer_process, Sget_buffer_process, 1, 1, 0,
        doc: /* Return the (or a) process associated with BUFFER.
 BUFFER may be a buffer or the name of one.  */)
diff --git a/src/sysdep.c b/src/sysdep.c
index 91e941a2050..faca7fae461 100644
--- a/src/sysdep.c
+++ b/src/sysdep.c
@@ -543,8 +543,6 @@ sys_subshell (void)
 #endif
         }
 
-      close_process_descs ();   /* Close Emacs's pipes/ptys */
-
 #ifdef MSDOS    /* Demacs 1.1.2 91/10/20 Manabu Higashida */
       {
         char *epwd = getenv ("PWD");
@@ -2152,6 +2150,8 @@ emacs_abort (void)
 #endif
 
 /* Open FILE for Emacs use, using open flags OFLAG and mode MODE.
+   Arrange for subprograms to not inherit the file descriptor.
+   Prefer a method that is multithread-safe, if available.
    Do not fail merely because the open was interrupted by a signal.
    Allow the user to quit.  */
 
@@ -2159,8 +2159,11 @@ int
 emacs_open (const char *file, int oflags, int mode)
 {
   int fd;
+  oflags |= O_CLOEXEC;
   while ((fd = open (file, oflags, mode)) < 0 && errno == EINTR)
     QUIT;
+  if (! O_CLOEXEC && 0 <= fd)
+    fcntl (fd, F_SETFD, FD_CLOEXEC);
   return fd;
 }
 
@@ -2170,10 +2173,29 @@ emacs_open (const char *file, int oflags, int mode)
 FILE *
 emacs_fopen (char const *file, char const *mode)
 {
-  FILE *fp;
-  while (! (fp = fopen (file, mode)) && errno == EINTR)
-    QUIT;
-  return fp;
+  int fd, omode, oflags;
+  int bflag = 0;
+  char const *m = mode;
+
+  switch (*m++)
+    {
+    case 'r': omode = O_RDONLY; oflags = 0; break;
+    case 'w': omode = O_WRONLY; oflags = O_CREAT | O_TRUNC; break;
+    case 'a': omode = O_WRONLY; oflags = O_CREAT | O_APPEND; break;
+    default: emacs_abort ();
+    }
+
+  while (*m)
+    switch (*m++)
+      {
+      case '+': omode = O_RDWR; break;
+      case 'b': bflag = O_BINARY; break;
+      case 't': bflag = O_TEXT; break;
+      default: /* Ignore.  */ break;
+      }
+
+  fd = emacs_open (file, omode | oflags | bflag, 0666);
+  return fd < 0 ? 0 : fdopen (fd, mode);
 }
 
 int
diff --git a/src/w32.c b/src/w32.c
index 230479cd61a..46aebe8b634 100644
--- a/src/w32.c
+++ b/src/w32.c
@@ -6719,10 +6719,16 @@ sys_sendto (int s, const char * buf, int len, int flags,
 }
 
 /* Windows does not have an fcntl function.  Provide an implementation
-   solely for making sockets non-blocking.  */
+   good enough for Emacs.  */
 int
 fcntl (int s, int cmd, int options)
 {
+  /* In the w32 Emacs port, fcntl (fd, F_DUPFD_CLOEXEC, fd1) is always
+     invoked in a context where fd1 is closed and all descriptors less
+     than fd1 are open, so sys_dup is an adequate implementation.  */
+  if (cmd == F_DUPFD_CLOEXEC)
+    return sys_dup (s);
+
   if (winsock_lib == NULL)
     {
       errno = ENETDOWN;
@@ -6864,13 +6870,14 @@ sys_dup2 (int src, int dst)
   return rc;
 }
 
-/* Unix pipe() has only one arg */
 int
-sys_pipe (int * phandles)
+pipe2 (int * phandles, int pipe2_flags)
 {
   int rc;
   unsigned flags;
 
+  eassert (pipe2_flags == O_CLOEXEC);
+
   /* make pipe handles non-inheritable; when we spawn a child, we
      replace the relevant handle with an inheritable one.  Also put
      pipes into binary mode; we will do text mode translation ourselves
diff --git a/src/w32.h b/src/w32.h
index 17da0778db1..9c1f1efa699 100644
--- a/src/w32.h
+++ b/src/w32.h
@@ -188,7 +188,7 @@ extern int random (void);
 
 extern int fchmod (int, mode_t);
 extern int sys_rename_replace (char const *, char const *, BOOL);
-extern int sys_pipe (int *);
+extern int pipe2 (int *, int);
 
 extern void set_process_dir (char *);
 extern int sys_spawnve (int, char *, char **, char **);