Fixed a nasty bug in scdaemon which led to a card reset if the card was

inserted  during scdaemon startup and a connection was made before the
ticker had a chance to run.  Add some stuff for better debugging.

9 files changed, 93 insertions, 20 deletions

diff --git a/agent/gpg-agent.c b/agent/gpg-agent.c
index d5045d263..90319d22f 100644
--- a/agent/gpg-agent.c
+++ b/agent/gpg-agent.c
@@ -2065,7 +2065,7 @@ check_own_socket (void)
   tattr = pth_attr_new();
   pth_attr_set (tattr, PTH_ATTR_JOINABLE, 0);
   pth_attr_set (tattr, PTH_ATTR_STACK_SIZE, 256*1024);
-  pth_attr_set (tattr, PTH_ATTR_NAME, "check-owb-socket");
+  pth_attr_set (tattr, PTH_ATTR_NAME, "check-own-socket");
 
   if (!pth_spawn (tattr, check_own_socket_thread, sockname))
       log_error ("error spawning check_own_socket_thread: %s\n",
diff --git a/doc/scdaemon.texi b/doc/scdaemon.texi
index 99716db79..61671f441 100644
--- a/doc/scdaemon.texi
+++ b/doc/scdaemon.texi
@@ -205,6 +205,10 @@ aborts.  For debugging purposes it is sometimes better to allow core
 dump.  This options enables it and also changes the working directory to
 @file{/tmp} when running in @option{--server} mode.
 
+@item --debug-log-tid
+@opindex debug-log-tid
+This option appends a thread ID to the PID in the log output.
+
 
 @item --no-detach
 @opindex no-detach
diff --git a/jnlib/ChangeLog b/jnlib/ChangeLog
index 651ed799f..05c8e98a5 100644
--- a/jnlib/ChangeLog
+++ b/jnlib/ChangeLog
@@ -1,3 +1,9 @@
+2009-02-25  Werner Koch  <[email protected]>
+
+	* logging.c (get_tid_callback): New.
+	(do_logv): Use it.
+	(log_set_get_tid_callback): New.
+
 2009-01-22  Werner Koch  <[email protected]>
 
 	* t-support.c (gpg_err_code_from_errno) 
diff --git a/jnlib/logging.c b/jnlib/logging.c
index df2ec4463..b06361980 100644
--- a/jnlib/logging.c
+++ b/jnlib/logging.c
@@ -1,6 +1,6 @@
 /* logging.c - Useful logging functions
  * Copyright (C) 1998, 1999, 2000, 2001, 2003,
- *               2004, 2005, 2006 Free Software Foundation, Inc.
+ *               2004, 2005, 2006, 2009 Free Software Foundation, Inc.
  *
  * This file is part of JNLIB.
  *
@@ -61,6 +61,7 @@ static char prefix_buffer[80];
 static int with_time;
 static int with_prefix;
 static int with_pid;
+static unsigned long (*get_tid_callback)(void);
 static int running_detached;
 static int force_prefixes;
 
@@ -366,6 +367,13 @@ log_set_fd (int fd)
 
 
 void
+log_set_get_tid_callback (unsigned long (*cb)(void))
+{
+  get_tid_callback = cb;
+}
+
+
+void
 log_set_prefix (const char *text, unsigned int flags)
 {
   if (text)
@@ -460,7 +468,13 @@ do_logv (int level, const char *fmt, va_list arg_ptr)
       if (with_prefix || force_prefixes)
         fputs (prefix_buffer, logstream);
       if (with_pid || force_prefixes)
-        fprintf (logstream, "[%u]", (unsigned int)getpid ());
+        {
+          if (get_tid_callback)
+            fprintf (logstream, "[%u.%lx]", 
+                     (unsigned int)getpid (), get_tid_callback ());
+          else
+            fprintf (logstream, "[%u]", (unsigned int)getpid ());
+        }
       if (!with_time || force_prefixes)
         putc (':', logstream);
       /* A leading backspace suppresses the extra space so that we can
diff --git a/jnlib/logging.h b/jnlib/logging.h
index 9da46e29e..0b96108a8 100644
--- a/jnlib/logging.h
+++ b/jnlib/logging.h
@@ -33,6 +33,7 @@ int  log_get_errorcount (int clear);
 void log_inc_errorcount (void);
 void log_set_file( const char *name );
 void log_set_fd (int fd);
+void log_set_get_tid_callback (unsigned long (*cb)(void));
 void log_set_prefix (const char *text, unsigned int flags);
 const char *log_get_prefix (unsigned int *flags);
 int log_test_fd (int fd);
diff --git a/scd/ChangeLog b/scd/ChangeLog
index 4c47ebd03..23544438c 100644
--- a/scd/ChangeLog
+++ b/scd/ChangeLog
@@ -1,3 +1,13 @@
+2009-02-25  Werner Koch  <[email protected]>
+
+	* apdu.c (apdu_get_status): Factor all code out to ...
+	(apdu_private_get_status): .. new.  Add arg NO_ATR_RESET.
+	(apdu_connect): Call new function.
+
+	* scdaemon.c: New option --debug-log-tid.
+	(tid_log_callback): New.
+	(main): Move debug-wait code after debug stream init.
+
 2009-02-24  Werner Koch  <[email protected]>
 
 	* ccid-driver.c (ccid_get_atr): Move debug output to ..
diff --git a/scd/apdu.c b/scd/apdu.c
index b3c0f66db..bbd7ea801 100644
--- a/scd/apdu.c
+++ b/scd/apdu.c
@@ -1,5 +1,5 @@
 /* apdu.c - ISO 7816 APDU functions and low level I/O
- * Copyright (C) 2003, 2004, 2008 Free Software Foundation, Inc.
+ * Copyright (C) 2003, 2004, 2008, 2009 Free Software Foundation, Inc.
  *
  * This file is part of GnuPG.
  *
@@ -295,6 +295,9 @@ long (* DLSTDCALL pcsc_set_timeout) (unsigned long context,
 /*  Prototypes.  */
 static int pcsc_get_status (int slot, unsigned int *status);
 static int reset_pcsc_reader (int slot);
+static int apdu_get_status_internal (int slot, int hang, int no_atr_reset,
+                                     unsigned int *status,
+                                     unsigned int *changed);
 
 
 
@@ -2565,9 +2568,18 @@ apdu_connect (int slot)
     }
   else
     sw = 0;
+  
+  /* We need to call apdu_get_status_internal, so that the last-status
+     machinery gets setup properly even if a card is inserted while
+     scdaemon is fired up and apdu_get_status has not yet been called.
+     Without that we would force a reset of the card with the next
+     call to apdu_get_status.  */
+  apdu_get_status_internal (slot, 1, 1, NULL, NULL);
+
   return sw;
 }
 
+
 int
 apdu_disconnect (int slot)
 {
@@ -2706,9 +2718,9 @@ apdu_get_atr (int slot, size_t *atrlen)
    of card insertions.  This value may be used to detect a card
    change.
 */
-int
-apdu_get_status (int slot, int hang,
-                 unsigned int *status, unsigned int *changed)
+static int
+apdu_get_status_internal (int slot, int hang, int no_atr_reset,
+                          unsigned int *status, unsigned int *changed)
 {
   int sw;
   unsigned int s;
@@ -2736,8 +2748,9 @@ apdu_get_status (int slot, int hang,
     {
       reader_table[slot].change_counter++;
       /* Make sure that the ATR is invalid so that a reset will be
-         triggered by activate.  */
-      reader_table[slot].atrlen = 0;
+         triggered by apdu_activate.  */
+      if (!no_atr_reset)
+        reader_table[slot].atrlen = 0;
     }
   reader_table[slot].any_status = 1;
   reader_table[slot].last_status = s;
@@ -2750,6 +2763,15 @@ apdu_get_status (int slot, int hang,
 }
 
 
+/* See above for a description.  */
+int
+apdu_get_status (int slot, int hang,
+                 unsigned int *status, unsigned int *changed)
+{
+  return apdu_get_status_internal (slot, hang, 0, status, changed);
+}
+
+
 /* Check whether the reader supports the ISO command code COMMAND on
    the keypad.  Return 0 on success.  For a description of the pin
    parameters, see ccid-driver.c */
diff --git a/scd/ccid-driver.c b/scd/ccid-driver.c
index bec1a6b26..854aca1b5 100644
--- a/scd/ccid-driver.c
+++ b/scd/ccid-driver.c
@@ -357,7 +357,7 @@ print_pr_data (const unsigned char *data, size_t datalen, size_t off)
       DEBUGOUT_CONT_1 (" %02X", data[off]);
       any = 1;
     }
-  if (any && !(off % 16))
+  if (any && (off % 16))
     DEBUGOUT_LF ();
 }
 
diff --git a/scd/scdaemon.c b/scd/scdaemon.c
index 74dd8ff41..0afb5febc 100644
--- a/scd/scdaemon.c
+++ b/scd/scdaemon.c
@@ -1,6 +1,6 @@
 /* scdaemon.c  -  The GnuPG Smartcard Daemon
  * Copyright (C) 2001, 2002, 2004, 2005, 
- *               2007, 2008 Free Software Foundation, Inc.
+ *               2007, 2008, 2009 Free Software Foundation, Inc.
  *
  * This file is part of GnuPG.
  *
@@ -69,6 +69,7 @@ enum cmd_and_opt_values
   oDebugWait,
   oDebugAllowCoreDump,
   oDebugCCIDDriver,
+  oDebugLogTid,
   oNoGreeting,
   oNoOptions,
   oHomedir,
@@ -117,6 +118,7 @@ static ARGPARSE_OPTS opts[] = {
   ARGPARSE_s_n (oDebugAllowCoreDump, "debug-allow-core-dump", "@"),
   ARGPARSE_s_n (oDebugCCIDDriver, "debug-ccid-driver", "@"),
   ARGPARSE_s_n (oDebugDisableTicker, "debug-disable-ticker", "@"),
+  ARGPARSE_s_n (oDebugLogTid, "debug-log-tid", "@"),
   ARGPARSE_s_n (oNoDetach, "no-detach", N_("do not detach from the console")),
   ARGPARSE_s_s (oLogFile,  "log-file", N_("|FILE|write a log to FILE")),
   ARGPARSE_s_s (oReaderPort, "reader-port", 
@@ -263,6 +265,17 @@ my_strusage (int level)
 }
 
 
+static unsigned long
+tid_log_callback (void)
+{
+#ifdef PTH_HAVE_PTH_THREAD_ID
+  return pth_thread_id ();
+#else
+  return (unsigned long)pth_self ();
+#endif
+}
+
+
 
 
 
@@ -505,6 +518,9 @@ main (int argc, char **argv )
 #endif /*HAVE_LIBUSB*/
           break;
         case oDebugDisableTicker: ticker_disabled = 1; break;
+        case oDebugLogTid: 
+          log_set_get_tid_callback (tid_log_callback);
+          break;
 
         case oOptions:
           /* config files may not be nested (silently ignore them) */
@@ -586,14 +602,6 @@ main (int argc, char **argv )
 
   set_debug (debug_level);
 
-  if (debug_wait && pipe_server)
-    {
-      log_debug ("waiting for debugger - my pid is %u .....\n",
-                 (unsigned int)getpid());
-      gnupg_sleep (debug_wait);
-      log_debug ("... okay\n");
-    }
-  
   initialize_module_command ();
 
   if (gpgconf_list == 2)
@@ -638,13 +646,21 @@ main (int argc, char **argv )
       scd_exit (0);
     }
 
-  /* now start with logging to a file if this is desired */
+  /* Now start with logging to a file if this is desired.  */
   if (logfile)
     {
       log_set_file (logfile);
       log_set_prefix (NULL, 1|2|4);
     }
 
+  if (debug_wait && pipe_server)
+    {
+      log_debug ("waiting for debugger - my pid is %u .....\n",
+                 (unsigned int)getpid());
+      gnupg_sleep (debug_wait);
+      log_debug ("... okay\n");
+    }
+  
   if (pipe_server)
     { 
       /* This is the simple pipe based server */