Do not use pth functions after pth_kill. Fixes bug#1320.

Fabian Keil found the reason for a SIGBUS:

  In the "gpg-agent --daemon" case, main() calls pth_kill()
  after the client has been forked, so when es_deinit() is
  called on exit, acquiring the estream_list_lock seems to
  cause pth to dereference a pointer located in a memory
  region that has previously been free()'d.

My approach to fix it is different than his suggestion.  It should
allow to continue all estream operations after a pth_kill except for
restarting pth.

agent/ChangeLog

@@ -1,3 +1,7 @@
+2011-04-29  Werner Koch  <wk@g10code.com>
+
+	* gpg-agent.c (main): s/pth_kill/es_pth_kill/.
+
 2010-11-11  Werner Koch  <wk@g10code.com>
 
 	* agent.h (opt): Add field SIGUSR2_ENABLED.

agent/gpg-agent.c

@@ -1060,8 +1060,10 @@ main (int argc, char **argv )
              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.  */
-          if ( !pth_kill () )
+             signal mask.  es_pth_kill is a wrapper around pth_kill to
+             take care not to use any Pth functions in the estream
+             code after Pth has been killed.  */
+          if ( !es_pth_kill () )
             log_error ("pth_kill failed in forked process\n");
 
 #ifdef HAVE_SIGPROCMASK

common/ChangeLog

@@ -1,3 +1,14 @@
+2011-04-29  Werner Koch  <wk@g10code.com>
+
+	* estream.c (es_pth_kill): New.
+	(estream_pth_killed): New.
+	(ESTREAM_MUTEX_LOCK, ESTREAM_MUTEX_UNLOCK)
+	(ESTREAM_MUTEX_TRYLOCK, ESTREAM_MUTEX_INITIALIZE): Take care of
+	the killed status.
+	(ESTREAM_SYS_YIELD): Ditto.
+	(es_pth_read, es_pth_write): Ditto.
+	(es_init_do): Ditto.
+
 2011-01-20  Werner Koch  <wk@g10code.com>
 
 	* estream.c (es_func_mem_write): Fix computation of NEWSIZE.
@@ -1604,5 +1615,3 @@
  This file is distributed in the hope that it will be useful, but
  WITHOUT ANY WARRANTY, to the extent permitted by law; without even the
  implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
-		
-

common/estream.c

@@ -150,20 +150,10 @@ typedef void (*func_free_t) (void *mem);
 /* Locking.  */
 
 #ifdef HAVE_PTH
-
 typedef pth_mutex_t estream_mutex_t;
-# define ESTREAM_MUTEX_INITIALIZER PTH_MUTEX_INIT
-# define ESTREAM_MUTEX_LOCK(mutex)        \
-  pth_mutex_acquire (&(mutex), 0, NULL)
-# define ESTREAM_MUTEX_UNLOCK(mutex)      \
-  pth_mutex_release (&(mutex))
-# define ESTREAM_MUTEX_TRYLOCK(mutex)     \
-  ((pth_mutex_acquire (&(mutex), 1, NULL) == TRUE) ? 0 : -1)
-# define ESTREAM_MUTEX_INITIALIZE(mutex)  \
-  pth_mutex_init    (&(mutex))
-#else
-
+#else /*!HAVE_PTH*/
 typedef void *estream_mutex_t;
+#endif /*!HAVE_PTH*/
 
 static inline void
 dummy_mutex_call_void (estream_mutex_t mutex)
@@ -178,19 +168,42 @@ dummy_mutex_call_int (estream_mutex_t mutex)
   return 0;
 }
 
+
+#ifdef HAVE_PTH
+
+static int estream_pth_killed;
+
+# define ESTREAM_MUTEX_INITIALIZER PTH_MUTEX_INIT
+# define ESTREAM_MUTEX_LOCK(mutex)                              \
+  (estream_pth_killed ? dummy_mutex_call_void ((mutex))         \
+   : pth_mutex_acquire (&(mutex), 0, NULL))
+# define ESTREAM_MUTEX_UNLOCK(mutex)                            \
+  (estream_pth_killed ? dummy_mutex_call_void ((mutex))         \
+   : pth_mutex_release (&(mutex)))
+# define ESTREAM_MUTEX_TRYLOCK(mutex)                                   \
+  (estream_pth_killed ? dummy_mutex_call_int ((mutex))                  \
+   : ((pth_mutex_acquire (&(mutex), 1, NULL) == TRUE)? 0:-1))
+# define ESTREAM_MUTEX_INITIALIZE(mutex)                        \
+  (estream_pth_killed ? dummy_mutex_call_void ((mutex))         \
+   : pth_mutex_init (&(mutex)))
+
+#else /*!HAVE_PTH*/
+
 # define ESTREAM_MUTEX_INITIALIZER NULL
 # define ESTREAM_MUTEX_LOCK(mutex) dummy_mutex_call_void ((mutex))
 # define ESTREAM_MUTEX_UNLOCK(mutex) dummy_mutex_call_void ((mutex))
 # define ESTREAM_MUTEX_TRYLOCK(mutex) dummy_mutex_call_int ((mutex))
 # define ESTREAM_MUTEX_INITIALIZE(mutex) dummy_mutex_call_void ((mutex))
-#endif
+
+#endif /*!HAVE_PTH*/
 
 /* Primitive system I/O.  */
 
 #ifdef HAVE_PTH
 # define ESTREAM_SYS_READ  es_pth_read
 # define ESTREAM_SYS_WRITE es_pth_write
-# define ESTREAM_SYS_YIELD() pth_yield (NULL)
+# define ESTREAM_SYS_YIELD() \
+  do { if (!estream_pth_killed) pth_yield (NULL); } while (0)
 #else
 # define ESTREAM_SYS_READ  read
 # define ESTREAM_SYS_WRITE write
@@ -411,6 +424,10 @@ es_list_iterate (estream_iterator_t iterator)
 static int
 es_pth_read (int fd, void *buffer, size_t size)
 {
+  if (estream_pth_killed)
+    return read (fd, buffer, size);
+  else
+    {
 # ifdef HAVE_W32_SYSTEM
       int rc = pth_read (fd, buffer, size);
       if (rc == -1 && errno == EINVAL)
@@ -420,10 +437,15 @@ es_pth_read (int fd, void *buffer, size_t size)
       return pth_read (fd, buffer, size);
 # endif /* !HAVE_W32_SYSTEM*/
     }
+}
 
 static int
 es_pth_write (int fd, const void *buffer, size_t size)
 {
+  if (estream_pth_killed)
+    return write (fd, buffer, size);
+  else
+    {
 # ifdef HAVE_W32_SYSTEM
       int rc = pth_write (fd, buffer, size);
       if (rc == -1 && errno == EINVAL)
@@ -433,6 +455,7 @@ es_pth_write (int fd, const void *buffer, size_t size)
       return pth_write (fd, buffer, size);
 # endif /* !HAVE_W32_SYSTEM*/
     }
+}
 #endif /*HAVE_PTH*/
 
 
@@ -445,6 +468,26 @@ es_deinit (void)
 }
 
 
+/* A replacement for pth_kill.  The reason we need this is that after
+   a pth_kill all our pth functions may not be used anymore.  Thus
+   applications using estream and pth need to use this function
+   instead of a plain pth_kill.  */
+int
+es_pth_kill (void)
+{
+#ifdef HAVE_PTH
+  int rc;
+
+  rc = pth_kill ();
+  if (rc)
+    estream_pth_killed = 1;
+  return rc;
+#else /*!HAVE_PTH*/
+  return 0;
+#endif /*!HAVE_PTH*/
+}
+
+
 /*
  * Initialization.
  */
@@ -457,10 +500,15 @@ es_init_do (void)
   if (!initialized)
     {
 #ifdef HAVE_PTH
+      if (estream_pth_killed)
+        initialized = 1;
+      else
+        {
           if (!pth_init () && errno != EPERM )
             return -1;
           if (pth_mutex_init (&estream_list_lock))
             initialized = 1;
+        }
 #else
       initialized = 1;
 #endif

common/estream.h

@@ -231,6 +231,8 @@ typedef struct es_cookie_io_functions
 
 int es_init (void);
 
+int es_pth_kill (void);
+
 estream_t es_fopen (const char *ES__RESTRICT path,
 		    const char *ES__RESTRICT mode);
 estream_t es_mopen (unsigned char *ES__RESTRICT data,