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.

4 files changed, 65 insertions, 17 deletions

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 */