Make sure not to leak file descriptors if running gpg-agent with a

command.  Restore the signal mask to solve a problem in Mono.

ChangeLog

@@ -1,3 +1,7 @@
+2009-03-18  Werner Koch  <wk@g10code.com>
+
+	* configure.ac: Test for getrlimit.
+
 2009-03-03  Werner Koch  <wk@g10code.com>
 
 	Release 2.0.11.

NEWS

@@ -7,6 +7,9 @@ Noteworthy changes in version 2.0.12
  * New command "KEYINFO" for GPG_AGENT.  GPGSM now also returns
    information about smartcards.
 
+ * Make sure not to leak file descriptors if running gpg-agent with a
+   command.  Restore the signal mask to solve a problem in Mono.
+
 
 Noteworthy changes in version 2.0.11 (2009-03-03)
 -------------------------------------------------

agent/ChangeLog

@@ -1,3 +1,8 @@
+2009-03-19  Werner Koch  <wk@g10code.com>
+
+	* gpg-agent.c (main): Save signal mask and open fds.  Restore mask
+	and close all fds prior to the exec.  Fixes bug#1013.
+
 2009-03-17  Werner Koch  <wk@g10code.com>
 
 	* command.c (cmd_get_passphrase): Break repeat loop on error.

agent/gpg-agent.c

@@ -47,6 +47,7 @@
 #include "sysutils.h"
 #include "setenv.h"
 #include "gc-opt-flags.h"
+#include "exechelp.h"
 
 
 enum cmd_and_opt_values 
@@ -197,6 +198,17 @@ static ARGPARSE_OPTS opts[] = {
 #define TIMERTICK_INTERVAL    (2)    /* Seconds.  */
 #endif
 
+
+/* The list of open file descriptors at startup.  Note that this list
+   has been allocated using the standard malloc.  */
+static int *startup_fd_list;
+
+/* The signal mask at startup and a flag telling whether it is valid.  */
+#ifdef HAVE_SIGPROCMASK
+static sigset_t startup_signal_mask;
+static int startup_signal_mask_valid;
+#endif
+
 /* Flag to indicate that a shutdown was requested.  */
 static int shutdown_pending;
 
@@ -530,6 +542,16 @@ main (int argc, char **argv )
   const char *env_file_name = NULL;
 
 
+  /* Before we do anything else we save the list of currently open
+     file descriptors and the signal mask.  This info is required to
+     do the exec call properly. */
+  startup_fd_list = get_all_open_fds ();
+#ifdef HAVE_SIGPROCMASK
+  if (!sigprocmask (SIG_UNBLOCK, NULL, &startup_signal_mask))
+    startup_signal_mask_valid = 1;
+#endif /*HAVE_SIGPROCMASK*/
+
+  /* Set program name etc.  */
   set_strusage (my_strusage);
   gcry_control (GCRYCTL_SUSPEND_SECMEM_WARN);
   /* Please note that we may running SUID(ROOT), so be very CAREFUL
@@ -961,6 +983,27 @@ main (int argc, char **argv )
           
           close (fd);
           
+          /* Note that we used a standard fork so that Pth runs in
+             both the parent and the child.  The pth_fork would
+             terminate Pth in the child but that is not the way we
+             want it.  Thus we use a plain fork and terminate Pth here
+             in the parent.  The pth_kill may or may not work reliable
+             but it should not harm to call it.  Because Pth fiddles
+             with the signal mask the signal mask might not be correct
+             right now and thus we restore it.  That is not strictly
+             necessary but some programs falsely assume a cleared
+             signal mask.  */
+#ifdef HAVE_SIGPROCMASK
+          if (startup_signal_mask_valid)
+            {
+              if (sigprocmask (SIG_SETMASK, &startup_signal_mask, NULL))
+                log_error ("error restoring signal mask: %s\n",
+                           strerror (errno));
+            }
+          else
+            log_info ("no saved signal mask\n");
+#endif /*HAVE_SIGPROCMASK*/          
+
           /* Create the info string: <name>:<pid>:<protocol_version> */
           if (asprintf (&infostr, "GPG_AGENT_INFO=%s:%lu:1",
                         socket_name, (ulong)pid ) < 0)
@@ -1039,6 +1082,14 @@ main (int argc, char **argv )
                   kill (pid, SIGTERM );
                   exit (1);
                 }
+
+              /* Close all the file descriptors except the standard
+                 ones and those open at startup.  We explicitly don't
+                 close 0,1,2 in case something went wrong collecting
+                 them at startup.  */
+              close_all_fds (3, startup_fd_list);
+
+              /* Run the command.  */
               execvp (argv[0], argv);
               log_error ("failed to run the command: %s\n", strerror (errno));
               kill (pid, SIGTERM);

common/ChangeLog

@@ -1,3 +1,13 @@
+2009-03-18  Werner Koch  <wk@g10code.com>
+
+	* exechelp.c: Include sys/resource.h and sys/stat.h.
+	(get_max_open_fds): New.
+	(do_exec): Use it.
+	(get_all_open_fds): New.
+	(close_all_fds): New.
+	(do_exec): Use close_all_fds.
+	* t-exechelp.c: New.
+
 2009-03-13  David Shaw  <dshaw@jabberwocky.com>
 
 	* http.c (do_parse_uri): Properly handle IPv6 literal addresses as

common/Makefile.am

@@ -108,7 +108,7 @@ status-codes.h: Makefile mkstrtable.awk exstatus.awk status.h
 #
 # Module tests
 #
-module_tests = t-convert t-percent t-gettime t-sysutils t-sexputil
+module_tests = t-convert t-percent t-gettime t-sysutils t-sexputil t-exechelp
 module_maint_tests = t-helpfile t-b64
 
 t_common_ldadd = libcommon.a ../jnlib/libjnlib.a ../gl/libgnu.a \
@@ -121,6 +121,7 @@ t_sysutils_LDADD = $(t_common_ldadd)
 t_helpfile_LDADD = $(t_common_ldadd)
 t_sexputil_LDADD = $(t_common_ldadd)
 t_b64_LDADD = $(t_common_ldadd)
+t_exechelp_LDADD = $(t_common_ldadd)
 
 
 

common/exechelp.c

@@ -1,5 +1,5 @@
 /* exechelp.c - fork and exec helpers
- *	Copyright (C) 2004, 2007, 2008 Free Software Foundation, Inc.
+ * Copyright (C) 2004, 2007, 2008, 2009 Free Software Foundation, Inc.
  *
  * This file is part of GnuPG.
  *
@@ -40,6 +40,16 @@
 #include <sys/wait.h>
 #endif
 
+#ifdef HAVE_GETRLIMIT
+#include <sys/time.h>
+#include <sys/resource.h>
+#endif /*HAVE_GETRLIMIT*/
+
+#ifdef HAVE_STAT
+# include <sys/stat.h>
+#endif
+
+
 #include "util.h"
 #include "i18n.h"
 #include "exechelp.h"
@@ -48,12 +58,6 @@
 #define DEBUG_W32_SPAWN 1
 
 
-#ifdef _POSIX_OPEN_MAX
-#define MAX_OPEN_FDS _POSIX_OPEN_MAX
-#else
-#define MAX_OPEN_FDS 20
-#endif
-
 /* We have the usual problem here: Some modules are linked against pth
    and some are not.  However we want to use pth_fork and pth_waitpid
    here. Using a weak symbol works but is not portable - we should
@@ -85,6 +89,145 @@
 #endif
 
 
+/* Return the maximum number of currently allowed open file
+   descriptors.  Only useful on POSIX systems but returns a value on
+   other systems too.  */
+int
+get_max_fds (void)
+{
+  int max_fds = -1;
+#ifdef HAVE_GETRLIMIT
+  struct rlimit rl;
+
+# ifdef RLIMIT_NOFILE
+  if (!getrlimit (RLIMIT_NOFILE, &rl))
+    max_fds = rl.rlim_max;
+# endif
+
+# ifdef RLIMIT_OFILE
+  if (max_fds == -1 && !getrlimit (RLIMIT_OFILE, &rl))
+    max_fds = rl.rlim_max;
+
+# endif
+#endif /*HAVE_GETRLIMIT*/
+
+#ifdef _SC_OPEN_MAX
+  if (max_fds == -1)
+    {
+      long int scres = sysconf (_SC_OPEN_MAX);
+      if (scres >= 0)
+        max_fds = scres;
+    }
+#endif
+
+#ifdef _POSIX_OPEN_MAX
+  if (max_fds == -1)
+    max_fds = _POSIX_OPEN_MAX;
+#endif
+
+#ifdef OPEN_MAX
+  if (max_fds == -1)
+    max_fds = OPEN_MAX;
+#endif
+
+  if (max_fds == -1)
+    max_fds = 256;  /* Arbitrary limit.  */
+
+  return max_fds;
+}
+
+
+/* Close all file descriptors starting with descriptor FIRST.  If
+   EXCEPT is not NULL, it is expected to be a list of file descriptors
+   which shall not be closed.  This list shall be sorted in ascending
+   order with the end marked by -1.  */
+void
+close_all_fds (int first, int *except)
+{
+  int max_fd = get_max_fds ();
+  int fd, i, except_start;
+
+  if (except)
+    {
+      except_start = 0;
+      for (fd=first; fd < max_fd; fd++)
+        {
+          for (i=except_start; except[i] != -1; i++)
+            {
+              if (except[i] == fd)
+                {
+                  /* If we found the descriptor in the exception list
+                     we can start the next compare run at the next
+                     index because the exception list is ordered.  */
+                except_start = i + 1;
+                break;
+                }
+            }
+          if (except[i] == -1)
+            close (fd);
+        }
+    }
+  else
+    {
+      for (fd=first; fd < max_fd; fd++)
+        close (fd);
+    }
+
+  errno = 0;
+}
+
+
+/* Returns an array with all currently open file descriptors.  The end
+   of the array is marked by -1.  The caller needs to release this
+   array using the *standard free* and not with xfree.  This allow the
+   use of this fucntion right at startup even before libgcrypt has
+   been initialized.  Returns NULL on error and sets ERRNO
+   accordingly.  */
+int *
+get_all_open_fds (void)
+{
+  int *array;
+  size_t narray;
+  int fd, max_fd, idx;
+#ifndef HAVE_STAT
+  array = calloc (1, sizeof *array);
+  if (array)
+    array[0] = -1;
+#else /*HAVE_STAT*/
+  struct stat statbuf;
+
+  max_fd = get_max_fds ();
+  narray = 32;  /* If you change this change also t-exechelp.c.  */
+  array = calloc (narray, sizeof *array);
+  if (!array)
+    return NULL;
+  
+  /* Note:  The list we return is ordered.  */
+  for (idx=0, fd=0; fd < max_fd; fd++)
+    if (!(fstat (fd, &statbuf) == -1 && errno == EBADF))
+      {
+        if (idx+1 >= narray)
+          {
+            int *tmp;
+
+            narray += (narray < 256)? 32:256;
+            tmp = realloc (array, narray * sizeof *array);
+            if (!tmp)
+              {
+                free (array);
+                return NULL;
+              }
+            array = tmp;
+          }
+        array[idx++] = fd;
+      }
+  array[idx] = -1;
+#endif /*HAVE_STAT*/
+  return array;
+}
+
+
+
 #ifdef HAVE_W32_SYSTEM
 /* Helper function to build_w32_commandline. */
 static char *
@@ -216,7 +359,7 @@ do_exec (const char *pgmname, const char *argv[],
          void (*preexec)(void) )
 {
   char **arg_list;
-  int n, i, j;
+  int i, j;
   int fds[3];
 
   fds[0] = fd_in;
@@ -259,12 +402,7 @@ do_exec (const char *pgmname, const char *argv[],
     }
 
   /* Close all other files. */
-  n = sysconf (_SC_OPEN_MAX);
+  close_all_fds (3, NULL);
-  if (n < 0)
-    n = MAX_OPEN_FDS;
-  for (i=3; i < n; i++)
-    close(i);
-  errno = 0;
   
   if (preexec)
     preexec ();

common/exechelp.h

@@ -1,5 +1,5 @@
 /* exechelp.h - Definitions for the fork and exec helpers
- *	Copyright (C) 2004 Free Software Foundation, Inc.
+ *	Copyright (C) 2004, 2009 Free Software Foundation, Inc.
  *
  * This file is part of GnuPG.
  *
@@ -20,6 +20,25 @@
 #ifndef GNUPG_COMMON_EXECHELP_H
 #define GNUPG_COMMON_EXECHELP_H
 
+/* Return the maximum number of currently allowed file descriptors.
+   Only useful on POSIX systems.  */
+int get_max_fds (void);
+
+
+/* Close all file descriptors starting with descriptor FIRST.  If
+   EXCEPT is not NULL, it is expected to be a list of file descriptors
+   which are not to close.  This list shall be sorted in ascending
+   order with its end marked by -1.  */
+void close_all_fds (int first, int *except);
+
+
+/* Returns an array with all currently open file descriptors.  The end
+   of the array is marked by -1.  The caller needs to release this
+   array using the *standard free* and not with xfree.  This allow the
+   use of this fucntion right at startup even before libgcrypt has
+   been initialized.  Returns NULL on error and sets ERRNO accordingly.  */
+int *get_all_open_fds (void);
+
 
 /* Portable function to create a pipe.  Under Windows the write end is
    inheritable.  */

configure.ac

@@ -1059,7 +1059,7 @@ AC_FUNC_FORK
 AC_CHECK_FUNCS([strerror strlwr tcgetattr mmap])
 AC_CHECK_FUNCS([strcasecmp strncasecmp ctermid times gmtime_r])
 AC_CHECK_FUNCS([unsetenv getpwnam getpwuid fcntl ftruncate])
-AC_CHECK_FUNCS([gettimeofday getrusage setrlimit clock_gettime])
+AC_CHECK_FUNCS([gettimeofday getrusage getrlimit setrlimit clock_gettime])
 AC_CHECK_FUNCS([atexit raise getpagesize strftime nl_langinfo setlocale])
 AC_CHECK_FUNCS([waitpid wait4 sigaction sigprocmask pipe stat getaddrinfo])
 AC_CHECK_FUNCS([ttyname rand ftello])

scd/ChangeLog

@@ -1,5 +1,7 @@
 2009-03-18  Werner Koch  <wk@g10code.com>
 
+	* apdu.c (open_pcsc_reader_wrapped): Use close_all_fds.
+
 	* command.c (cmd_learn): Add option --keypairinfo.
 	* app.c (app_write_learn_status): Add arg FLAGS.
 	* app-common.h (struct app_ctx_s): Add arg FLAGS to LEARN_STATUS.

scd/apdu.c

@@ -57,6 +57,7 @@
 #include "cardglue.h"
 #else /* GNUPG_MAJOR_VERSION != 1 */
 #include "scdaemon.h"
+#include "exechelp.h"
 #endif /* GNUPG_MAJOR_VERSION != 1 */
 
 #include "apdu.h"
@@ -81,11 +82,6 @@
 #define DLSTDCALL
 #endif
 
-#ifdef _POSIX_OPEN_MAX
-#define MAX_OPEN_FDS _POSIX_OPEN_MAX
-#else
-#define MAX_OPEN_FDS 20
-#endif
 
 /* Helper to pass parameters related to keypad based operations. */
 struct pininfo_s
@@ -1653,12 +1649,7 @@ open_pcsc_reader_wrapped (const char *portstr)
         log_fatal ("dup2 stderr failed: %s\n", strerror (errno));
 
       /* Close all other files. */
-      n = sysconf (_SC_OPEN_MAX);
+      close_all_fds (3, NULL);
-      if (n < 0)
-        n = MAX_OPEN_FDS;
-      for (i=3; i < n; i++)
-        close(i);
-      errno = 0;
 
       execl (wrapperpgm,
              "pcsc-wrapper",