Fixed some card related problems.

4 files changed, 236 insertions, 49 deletions

diff --git a/scd/ChangeLog b/scd/ChangeLog
index 4b6575679..eb898c185 100644
--- a/scd/ChangeLog
+++ b/scd/ChangeLog
@@ -1,3 +1,17 @@
+2008-12-18  Werner Koch  <[email protected]>
+
+	* ccid-driver.c (abort_cmd): New. 
+	(bulk_in): Call abort_cmd after severe errors.
+
+	* apdu.c (reader_table_s): Add field ANY_STATUS.
+	(new_reader_slot): Clear it.
+	(apdu_get_status): Use ANY_STATUS to update the change counter.
+	Remove the use of the flag bit from LAST_STATUS everywhere.
+	* command.c (update_reader_status_file): Factor code out to ...
+	(send_client_notifications): New.  Track signals already sent.
+	(update_reader_status_file): Shutdown the reader  after a failed
+	apdu_get_status.
+
 2008-12-09  Werner Koch  <[email protected]>
 
 	* scdaemon.c (main): Call i18n_init before init_common_subsystems.
diff --git a/scd/apdu.c b/scd/apdu.c
index d02736a51..e14f2b2ca 100644
--- a/scd/apdu.c
+++ b/scd/apdu.c
@@ -133,6 +133,7 @@ struct reader_table_s {
   } rapdu;
 #endif /*USE_G10CODE_RAPDU*/
   char *rdrname;     /* Name of the connected reader or NULL if unknown. */
+  int any_status;    /* True if we have seen any status.  */
   int last_status;
   int status;
   int is_t0;         /* True if we know that we are running T=0. */
@@ -340,7 +341,8 @@ new_reader_slot (void)
   reader_table[reader].check_keypad = NULL;
   reader_table[reader].dump_status_reader = NULL;
 
-  reader_table[reader].used = 1;
+  reader_table[reader].used = 1;  
+  reader_table[reader].any_status = 0;
   reader_table[reader].last_status = 0;
   reader_table[reader].is_t0 = 1;
 #ifdef NEED_PCSC_WRAPPER
@@ -1289,8 +1291,7 @@ connect_pcsc_card (int slot)
              and usable.  Remember this.  */
           reader_table[slot].last_status = (   APDU_CARD_USABLE
                                              | APDU_CARD_PRESENT
-                                             | APDU_CARD_ACTIVE
-                                             | 0x8000);
+                                             | APDU_CARD_ACTIVE);
           reader_table[slot].is_t0 = !!(card_protocol & PCSC_PROTOCOL_T0);
         }
     }
@@ -1743,8 +1744,7 @@ open_pcsc_reader_wrapped (const char *portstr)
          and usable.  Thus remember this.  */
       slotp->last_status = (  APDU_CARD_USABLE
                             | APDU_CARD_PRESENT
-                            | APDU_CARD_ACTIVE
-                            | 0x8000);
+                            | APDU_CARD_ACTIVE);
     }
   slotp->atrlen = len;
 
@@ -1947,8 +1947,7 @@ open_ccid_reader (const char *portstr)
          and usable.  Thus remember this.  */
       reader_table[slot].last_status = (APDU_CARD_USABLE
                                         | APDU_CARD_PRESENT
-                                        | APDU_CARD_ACTIVE 
-                                        | 0x8000);
+                                        | APDU_CARD_ACTIVE);
     }
 
   reader_table[slot].close_reader = close_ccid_reader;
@@ -2615,8 +2614,7 @@ apdu_reset (int slot)
          and usable.  Thus remember this.  */
       reader_table[slot].last_status = (APDU_CARD_USABLE
                                         | APDU_CARD_PRESENT
-                                        | APDU_CARD_ACTIVE 
-                                        | 0x8000);
+                                        | APDU_CARD_ACTIVE);
     }
 
   unlock_slot (slot);
@@ -2662,8 +2660,7 @@ apdu_activate (int slot)
                      and usable.  Thus remember this.  */
                   reader_table[slot].last_status = (APDU_CARD_USABLE
                                                     | APDU_CARD_PRESENT
-                                                    | APDU_CARD_ACTIVE
-                                                    | 0x8000);
+                                                    | APDU_CARD_ACTIVE);
                 }
             }
         }
@@ -2733,17 +2730,16 @@ apdu_get_status (int slot, int hang,
       return sw;
     }
 
-  /* Keep track of changes.  We use one extra bit to test whether we
-     have checked the status at least once. */
-  if ( s != (reader_table[slot].last_status & 0x07ff)
-       || !reader_table[slot].last_status )
+  /* Keep track of changes.  */
+  if (s != reader_table[slot].last_status
+      || !reader_table[slot].any_status )
     {
       reader_table[slot].change_counter++;
       /* Make sure that the ATR is invalid so that a reset will be by
          activate.  */
       reader_table[slot].atrlen = 0;
     }
-  reader_table[slot].last_status = (s | 0x8000);
+  reader_table[slot].any_status = 1;
 
   if (status)
     *status = s;
diff --git a/scd/ccid-driver.c b/scd/ccid-driver.c
index 57e617ab5..08604131f 100644
--- a/scd/ccid-driver.c
+++ b/scd/ccid-driver.c
@@ -1,5 +1,6 @@
 /* ccid-driver.c - USB ChipCardInterfaceDevices driver
- * Copyright (C) 2003, 2004, 2005, 2006, 2007 Free Software Foundation, Inc.
+ * Copyright (C) 2003, 2004, 2005, 2006, 2007
+ *               2008  Free Software Foundation, Inc.
  * Written by Werner Koch.
  *
  * This file is part of GnuPG.
@@ -262,6 +263,7 @@ static int bulk_out (ccid_driver_t handle, unsigned char *msg, size_t msglen);
 static int bulk_in (ccid_driver_t handle, unsigned char *buffer, size_t length,
                     size_t *nread, int expected_type, int seqno, int timeout,
                     int no_debug);
+static int abort_cmd (ccid_driver_t handle);
 
 /* Convert a little endian stored 4 byte value into an unsigned
    integer. */
@@ -1069,7 +1071,10 @@ scan_or_find_devices (int readerno, const char *readerid,
 /* Set the level of debugging to LEVEL and return the old level.  -1
    just returns the old level.  A level of 0 disables debugging, 1
    enables debugging, 2 enables additional tracing of the T=1
-   protocol, other values are not yet defined. */
+   protocol, other values are not yet defined.
+
+   Note that libusb may provide its own debugging feature which is
+   enabled by setting the envvar USB_DEBUG.  */
 int
 ccid_set_debug_level (int level)
 {
@@ -1395,7 +1400,7 @@ bulk_out (ccid_driver_t handle, unsigned char *msg, size_t msglen)
       rc = usb_bulk_write (handle->idev, 
                            handle->ep_bulk_out,
                            (char*)msg, msglen,
-                           1000 /* ms timeout */);
+                           5000 /* ms timeout */);
       if (rc == msglen)
         return 0;
       if (rc == -1)
@@ -1463,17 +1468,20 @@ bulk_in (ccid_driver_t handle, unsigned char *buffer, size_t length,
   if (msglen < 10)
     {
       DEBUGOUT_1 ("bulk-in msg too short (%u)\n", (unsigned int)msglen);
+      abort_cmd (handle);
       return CCID_DRIVER_ERR_INV_VALUE;
     }
   if (buffer[5] != 0)    
     {
       DEBUGOUT_1 ("unexpected bulk-in slot (%d)\n", buffer[5]);
+      abort_cmd (handle);
       return CCID_DRIVER_ERR_INV_VALUE;
     }
   if (buffer[6] != seqno)    
     {
       DEBUGOUT_2 ("bulk-in seqno does not match (%d/%d)\n",
                   seqno, buffer[6]);
+      abort_cmd (handle);
       return CCID_DRIVER_ERR_INV_VALUE;
     }
 
@@ -1492,6 +1500,7 @@ bulk_in (ccid_driver_t handle, unsigned char *buffer, size_t length,
   if (buffer[0] != expected_type)
     {
       DEBUGOUT_1 ("unexpected bulk-in msg type (%02x)\n", buffer[0]);
+      abort_cmd (handle);
       return CCID_DRIVER_ERR_INV_VALUE;
     }
 
@@ -1521,6 +1530,113 @@ bulk_in (ccid_driver_t handle, unsigned char *buffer, size_t length,
 }
 
 
+
+/* Send an abort sequence and wait until everything settled.  */
+static int
+abort_cmd (ccid_driver_t handle)
+{
+  int rc;
+  char dummybuf[8];
+  unsigned char seqno;
+  unsigned char msg[100];
+  size_t msglen;
+  int i;
+
+  if (!handle->idev)
+    {
+      /* I don't know how to send an abort to non-USB devices.  */
+      rc = CCID_DRIVER_ERR_NOT_SUPPORTED;
+    }
+  
+  DEBUGOUT ("sending abort sequence\n");
+  /* Send the abort command to the control pipe.  Note that we don't
+     need to keep track of sent abort commands because there should
+     never be another thread using the same slot concurrently.  */
+  seqno = (handle->seqno & 0xff);
+  rc = usb_control_msg (handle->idev, 
+                        0x21,/* bmRequestType: host-to-device,
+                                class specific, to interface.  */
+                        1,   /* ABORT */
+                        (seqno << 8 | 0 /* slot */),
+                        handle->ifc_no,
+                        dummybuf, 0,
+                        1000 /* ms timeout */);
+  if (rc < 0)
+    {
+      DEBUGOUT_1 ("usb_control_msg error: %s\n", strerror (errno));
+      return CCID_DRIVER_ERR_CARD_IO_ERROR;
+    }
+
+  /* Now send the abort command to the bulk out pipe using the same
+     SEQNO and SLOT. */
+  msg[0] = PC_to_RDR_Abort;
+  msg[5] = 0; /* slot */
+  msg[6] = seqno;
+  msg[7] = 0; /* RFU */
+  msg[8] = 0; /* RFU */
+  msg[9] = 0; /* RFU */
+  msglen = 10;
+  set_msg_len (msg, 0);
+  handle->seqno++;  /* Bumb up for the next use. */
+
+  DEBUGOUT ("sending");
+  for (i=0; i < msglen; i++)
+    DEBUGOUT_CONT_1 (" %02X", msg[i]);
+  DEBUGOUT_LF ();
+  rc = usb_bulk_write (handle->idev, 
+                       handle->ep_bulk_out,
+                       (char*)msg, msglen,
+                       5000 /* ms timeout */);
+  if (rc == msglen)
+    rc = 0;
+  else if (rc == -1)
+    DEBUGOUT_1 ("usb_bulk_write error in abort_cmd: %s\n", strerror (errno));
+  else
+    DEBUGOUT_1 ("usb_bulk_write failed in abort_cmd: %d\n", rc);
+
+  if (rc)
+    return rc;
+
+  /* Wait for the expected response.  */
+  do
+    {
+      rc = usb_bulk_read (handle->idev, 
+                          handle->ep_bulk_in,
+                          (char*)msg, sizeof msg, 
+                          5000 /*ms timeout*/);
+      if (rc < 0)
+        {
+          DEBUGOUT_1 ("usb_bulk_read error in abort_cmd: %s\n",
+                      strerror (errno));
+          return CCID_DRIVER_ERR_CARD_IO_ERROR;
+        }
+      msglen = rc;
+
+      if (msglen < 10)
+        {
+          DEBUGOUT_1 ("bulk-in msg in abort_cmd too short (%u)\n",
+                      (unsigned int)msglen);
+          return CCID_DRIVER_ERR_INV_VALUE;
+        }
+      if (msg[5] != 0)    
+        {
+          DEBUGOUT_1 ("unexpected bulk-in slot (%d) in abort_cmd\n", msg[5]);
+          return CCID_DRIVER_ERR_INV_VALUE;
+        }
+
+      DEBUGOUT_3 ("status: %02X  error: %02X  octet[9]: %02X\n",
+                  msg[7], msg[8], msg[9]);
+      if (CCID_COMMAND_FAILED (msg))
+        print_command_failed (msg);
+    }
+  while (msg[0] != RDR_to_PC_SlotStatus && msg[5] != 0 && msg[6] != seqno);
+
+  DEBUGOUT ("sending abort sequence succeeded\n");
+
+  return 0;
+}
+
+
 /* Note that this function won't return the error codes NO_CARD or
    CARD_INACTIVE.  IF RESULT is not NULL, the result from the
    operation will get returned in RESULT and its length in RESULTLEN.
diff --git a/scd/command.c b/scd/command.c
index d94612daa..1037f1667 100644
--- a/scd/command.c
+++ b/scd/command.c
@@ -1964,6 +1964,84 @@ send_status_info (ctrl_t ctrl, const char *keyword, ...)
 }
 
 
+
+/* Helper to send the clients a status change notification.  */
+static void
+send_client_notifications (void)
+{
+  struct {
+    pid_t pid; 
+#ifdef HAVE_W32_SYSTEM
+    HANDLE handle;
+#else
+    int signo; 
+#endif
+  } killed[50];
+  int killidx = 0;
+  int kidx;
+  struct server_local_s *sl;
+  
+  for (sl=session_list; sl; sl = sl->next_session)
+    {
+      if (sl->event_signal && sl->assuan_ctx)
+        {
+          pid_t pid = assuan_get_pid (sl->assuan_ctx);
+#ifdef HAVE_W32_SYSTEM
+          HANDLE handle = (void *)sl->event_signal;
+          
+          for (kidx=0; kidx < killidx; kidx++)
+            if (killed[kidx].pid == pid 
+                && killed[kidx].handle == handle)
+              break;
+          if (kidx < killidx)
+            log_info ("event %lx (%p) already triggered for client %d\n",
+                      sl->event_signal, handle, (int)pid);
+          else
+            {
+              log_info ("triggering event %lx (%p) for client %d\n",
+                        sl->event_signal, handle, (int)pid);
+              if (!SetEvent (handle))
+                log_error ("SetEvent(%lx) failed: %s\n",
+                           sl->event_signal, w32_strerror (-1));
+              if (killidx < DIM (killed))
+                {
+                  killed[killidx].pid = pid;
+                  killed[killidx].handle = handle;
+                  killidx++;
+                }
+            }
+#else /*!HAVE_W32_SYSTEM*/
+          int signo = sl->event_signal;
+          
+          if (pid != (pid_t)(-1) && pid && signo > 0)
+            {
+              for (kidx=0; kidx < killidx; kidx++)
+                if (killed[kidx].pid == pid 
+                    && killed[kidx].signo == signo)
+                  break;
+              if (kidx < killidx)
+                log_info ("signal %d already sent to client %d\n",
+                          signo, (int)pid);
+              else
+                {
+                  log_info ("sending signal %d to client %d\n",
+                            signo, (int)pid);
+                  kill (pid, signo);
+                  if (killidx < DIM (killed))
+                    {
+                      killed[killidx].pid = pid;
+                      killed[killidx].signo = signo;
+                      killidx++;
+                    }
+                }
+#endif /*!HAVE_W32_SYSTEM*/
+            }
+        }
+    }
+}
+
+
+
 /* This is the core of scd_update_reader_status_file but the caller
    needs to take care of the locking.  */
 static void
@@ -1985,12 +2063,17 @@ update_reader_status_file (int set_card_removed_flag)
     {
       struct slot_status_s *ss = slot_table + idx;
       struct server_local_s *sl;
+      int sw_apdu;
 
       if (!ss->valid || ss->slot == -1)
         continue; /* Not valid or reader not yet open. */
       
-      if ( apdu_get_status (ss->slot, 0, &status, &changed) )
-        continue; /* Get status failed. */
+      sw_apdu = apdu_get_status (ss->slot, 0, &status, &changed);
+      if (sw_apdu)
+        {
+          /* Get status failed.  Ignore that.  */
+          continue; 
+        }
 
       if (!ss->any || ss->status != status || ss->changed != changed )
         {
@@ -1998,8 +2081,10 @@ update_reader_status_file (int set_card_removed_flag)
           char templ[50];
           FILE *fp;
 
-          log_info ("updating status of slot %d to 0x%04X\n",
-                    ss->slot, status);
+          log_info ("updating slot %d status: 0x%04X->0x%04X (%u->%u)\n",
+                    ss->slot, ss->status, status, ss->changed, changed);
+          ss->status = status;
+          ss->changed = changed;
 
 	  /* FIXME: Should this be IDX instead of ss->slot?  This
 	     depends on how client sessions will associate the reader
@@ -2065,33 +2150,9 @@ update_reader_status_file (int set_card_removed_flag)
             update_card_removed (idx, 1);
           
           ss->any = 1;
-          ss->status = status;
-          ss->changed = changed;
 
           /* Send a signal to all clients who applied for it.  */
-          for (sl=session_list; sl; sl = sl->next_session)
-            if (sl->event_signal && sl->assuan_ctx)
-              {
-                pid_t pid = assuan_get_pid (sl->assuan_ctx);
-
-#ifdef HAVE_W32_SYSTEM
-                HANDLE handle = (void *)sl->event_signal;
-                
-                log_info ("client pid is %d, triggering event %lx (%p)\n",
-                          pid, sl->event_signal, handle);
-                if (!SetEvent (handle))
-                  log_error ("SetEvent(%lx) failed: %s\n",
-                             sl->event_signal, w32_strerror (-1));
-#else
-                int signo = sl->event_signal;
-                
-                log_info ("client pid is %d, sending signal %d\n",
-                          pid, signo);
-                if (pid != (pid_t)(-1) && pid && signo > 0)
-                  kill (pid, signo);
-#endif
-              }
-
+          send_client_notifications ();
         }
       
       /* Check whether a disconnect is pending.  */
@@ -2104,7 +2165,7 @@ update_reader_status_file (int set_card_removed_flag)
             {
               /* FIXME: Use a real timeout.  */
               /* At least one connection and all allow a disconnect.  */
-              log_debug ("disconnecting card in slot %d\n", ss->slot);
+              log_info ("disconnecting card in slot %d\n", ss->slot);
               apdu_disconnect (ss->slot);
             }
         }