scd: More changes on watching removal of card/reader.
authorNIIBE Yutaka <gniibe@fsij.org>
Fri, 27 Jan 2017 15:18:11 +0000 (00:18 +0900)
committerNIIBE Yutaka <gniibe@fsij.org>
Fri, 27 Jan 2017 15:36:27 +0000 (00:36 +0900)
* scd/app-common.h (struct app_ctx_s): Rename field to
periodical_check_needed.
* scd/scdaemon.c (update_usb): Rename from update_fdset_for_usb.
Don't use libusb_get_pollfds any more.
(scd_kick_the_loop): New.
(need_tick): Follow the rename.
(handle_connections): No libusb event handling here.
* scd/app.c (app_new_register): Follow the change of rename.
(select_application, scd_update_reader_status_file): Likewise.
* scd/ccid-driver.c (ccid_usb_thread_is_alive): New.
(intr_cb): Call scd_kick_the_loop.
(ccid_usb_thread): New.  Thread to invoke INTERRUPT callback.
(ccid_open_usb_reader): Add thread invocation.
(ccid_require_get_status): Remove
LIBUSB_WORKS_EXPECTED_FOR_INTERRUPT_ENDP.
(do_close_reader): Carefully handle handle->transfer.
(get_escaped_usb_string): Insert npth_unprotect/npth_protect.
(do_close_reader, bulk_out, bulk_in, abort_cmd, ccid_slot_status)
(ccid_transceive, ccid_transceive_secure): Likewise.

--

It found that libusb_get_pollfds is not supported on Windows.
Besides, it's a bit difficult to use for the select loop.
Thus, we use the thread named ccid_usb_thread, instead.

Signed-off-by: NIIBE Yutaka <gniibe@fsij.org>
scd/app-common.h
scd/app.c
scd/ccid-driver.c
scd/scdaemon.c
scd/scdaemon.h

index 521fcf3..c79e36b 100644 (file)
@@ -54,7 +54,7 @@ struct app_ctx_s {
   const char *apptype;
   unsigned int card_version;
   unsigned int card_status;
-  unsigned int require_get_status:1;
+  unsigned int periodical_check_needed:1;
   unsigned int did_chv1:1;
   unsigned int force_chv1:1;   /* True if the card does not cache CHV1. */
   unsigned int did_chv2:1;
index 3cf219c..2cf7d11 100644 (file)
--- a/scd/app.c
+++ b/scd/app.c
@@ -175,7 +175,7 @@ app_reset (app_t app, ctrl_t ctrl, int send_reset)
 
 static gpg_error_t
 app_new_register (int slot, ctrl_t ctrl, const char *name,
-                  int require_get_status)
+                  int periodical_check_needed)
 {
   gpg_error_t err = 0;
   app_t app = NULL;
@@ -304,7 +304,7 @@ app_new_register (int slot, ctrl_t ctrl, const char *name,
       return err;
     }
 
-  app->require_get_status = require_get_status;
+  app->periodical_check_needed = periodical_check_needed;
 
   npth_mutex_lock (&app_list_lock);
   app->next = app_top;
@@ -331,7 +331,7 @@ select_application (ctrl_t ctrl, const char *name, app_t *r_app,
   if (scan || !app_top)
     {
       struct dev_list *l;
-      int all_have_intr_endp = 1;
+      int periodical_check_needed = 0;
 
       err = apdu_dev_list_start (opt.reader_port, &l);
       if (err)
@@ -340,23 +340,24 @@ select_application (ctrl_t ctrl, const char *name, app_t *r_app,
       while (1)
         {
           int slot;
-          int require_get_status;
+          int periodical_check_needed;
 
           slot = apdu_open_reader (l);
           if (slot < 0)
             break;
 
-          require_get_status = apdu_connect (slot);
-          if (require_get_status < 0)
+          periodical_check_needed = apdu_connect (slot);
+          if (periodical_check_needed < 0)
             {
               /* We close a reader with no card.  */
               err = gpg_error (GPG_ERR_ENODEV);
             }
           else
             {
-              err = app_new_register (slot, ctrl, name, require_get_status);
-              if (require_get_status)
-                all_have_intr_endp = 0;
+              err = app_new_register (slot, ctrl, name,
+                                      periodical_check_needed);
+              if (periodical_check_needed)
+                periodical_check_needed = 1;
             }
 
           if (err)
@@ -364,7 +365,7 @@ select_application (ctrl_t ctrl, const char *name, app_t *r_app,
         }
 
       apdu_dev_list_finish (l);
-      update_fdset_for_usb (all_have_intr_endp);
+      update_usb (periodical_check_needed);
     }
 
   npth_mutex_lock (&app_list_lock);
@@ -1014,7 +1015,7 @@ void
 scd_update_reader_status_file (void)
 {
   app_t a, app_next;
-  int all_have_intr_endp = 1;
+  int periodical_check_needed = 0;
   int removal_detected = 0;
 
   npth_mutex_lock (&app_list_lock);
@@ -1034,8 +1035,8 @@ scd_update_reader_status_file (void)
       else if (sw)
         {
           /* Get status failed.  Ignore that.  */
-          if (a->require_get_status)
-            all_have_intr_endp = 0;
+          if (a->periodical_check_needed)
+            periodical_check_needed = 1;
           continue;
         }
 
@@ -1054,20 +1055,20 @@ scd_update_reader_status_file (void)
           else
             {
               a->card_status = status;
-              if (a->require_get_status)
-                all_have_intr_endp = 0;
+              if (a->periodical_check_needed)
+                periodical_check_needed = 1;
             }
         }
       else
         {
-          if (a->require_get_status)
-            all_have_intr_endp = 0;
+          if (a->periodical_check_needed)
+            periodical_check_needed = 1;
         }
     }
   npth_mutex_unlock (&app_list_lock);
 
   if (removal_detected)
-    update_fdset_for_usb (all_have_intr_endp);
+    update_usb (periodical_check_needed);
 }
 
 /* This function must be called once to initialize this module.  This
index cb4b4e6..bbdd697 100644 (file)
@@ -285,6 +285,7 @@ static int debug_level;     /* Flag to control the debug output.
                                2 = Level 1 + T=1 protocol tracing
                                3 = Level 2 + USB/I/O tracing of SlotStatus.
                               */
+static int ccid_usb_thread_is_alive;
 
 
 static unsigned int compute_edc (const unsigned char *data, size_t datalen,
@@ -1007,19 +1008,31 @@ get_escaped_usb_string (libusb_device_handle *idev, int idx,
   /* First get the list of supported languages and use the first one.
      If we do don't find it we try to use English.  Note that this is
      all in a 2 bute Unicode encoding using little endian. */
+#ifdef USE_NPTH
+  npth_unprotect ();
+#endif
   rc = libusb_control_transfer (idev, LIBUSB_ENDPOINT_IN,
                                 LIBUSB_REQUEST_GET_DESCRIPTOR,
                                 (LIBUSB_DT_STRING << 8), 0,
                                 (char*)buf, sizeof buf, 1000 /* ms timeout */);
+#ifdef USE_NPTH
+  npth_protect ();
+#endif
   if (rc < 4)
     langid = 0x0409; /* English.  */
   else
     langid = (buf[3] << 8) | buf[2];
 
+#ifdef USE_NPTH
+  npth_unprotect ();
+#endif
   rc = libusb_control_transfer (idev, LIBUSB_ENDPOINT_IN,
                                 LIBUSB_REQUEST_GET_DESCRIPTOR,
                                 (LIBUSB_DT_STRING << 8) + idx, langid,
                                 (char*)buf, sizeof buf, 1000 /* ms timeout */);
+#ifdef USE_NPTH
+  npth_protect ();
+#endif
   if (rc < 2 || buf[1] != LIBUSB_DT_STRING)
     return NULL; /* Error or not a string. */
   len = buf[0];
@@ -1799,6 +1812,8 @@ intr_cb (struct libusb_transfer *transfer)
       DEBUGOUT ("CCID: device removed\n");
       handle->powered_off = 1;
     }
+
+  scd_kick_the_loop ();
 }
 
 static void
@@ -1817,6 +1832,26 @@ ccid_setup_intr  (ccid_driver_t handle)
 }
 
 
+static void *
+ccid_usb_thread (void *arg)
+{
+  libusb_context *ctx = arg;
+
+  while (ccid_usb_thread_is_alive)
+    {
+#ifdef USE_NPTH
+      npth_unprotect ();
+#endif
+      libusb_handle_events_completed (ctx, NULL);
+#ifdef USE_NPTH
+      npth_protect ();
+#endif
+    }
+
+  return NULL;
+}
+
+
 static int
 ccid_open_usb_reader (const char *spec_reader_name,
                       int idx, struct ccid_dev_table *ccid_table,
@@ -1824,7 +1859,7 @@ ccid_open_usb_reader (const char *spec_reader_name,
 {
   libusb_device *dev;
   libusb_device_handle *idev = NULL;
-  char *rid;
+  char *rid = NULL;
   int rc = 0;
   int ifc_no, set_no;
   struct libusb_device_descriptor desc;
@@ -1850,13 +1885,39 @@ ccid_open_usb_reader (const char *spec_reader_name,
       return rc;
     }
 
+  if (ccid_usb_thread_is_alive++ == 0)
+    {
+      npth_t thread;
+      npth_attr_t tattr;
+      int err;
+
+      err = npth_attr_init (&tattr);
+      if (err)
+        {
+          DEBUGOUT_1 ("npth_attr_init failed: %s\n", strerror (err));
+          free (*handle);
+          *handle = NULL;
+          return err;
+        }
+
+      npth_attr_setdetachstate (&tattr, NPTH_CREATE_DETACHED);
+      err = npth_create (&thread, &tattr, ccid_usb_thread, NULL);
+      if (err)
+        {
+          DEBUGOUT_1 ("npth_create failed: %s\n", strerror (err));
+          free (*handle);
+          *handle = NULL;
+          return err;
+        }
+
+      npth_attr_destroy (&tattr);
+    }
+
   rc = libusb_get_device_descriptor (dev, &desc);
   if (rc)
     {
-      libusb_close (idev);
-      free (*handle);
-      *handle = NULL;
-      return rc;
+      DEBUGOUT ("get_device_descripor failed\n");
+      goto leave;
     }
 
   rid = make_reader_id (idev, desc.idVendor, desc.idProduct,
@@ -1919,6 +1980,7 @@ ccid_open_usb_reader (const char *spec_reader_name,
  leave:
   if (rc)
     {
+      --ccid_usb_thread_is_alive;
       free (rid);
       libusb_close (idev);
       free (*handle);
@@ -2011,10 +2073,8 @@ ccid_open_reader (const char *spec_reader_name, int idx,
 int
 ccid_require_get_status (ccid_driver_t handle)
 {
-#ifdef LIBUSB_WORKS_EXPECTED_FOR_INTERRUPT_ENDP
   if (handle->ep_intr >= 0)
     return 0;
-#endif
 
   /* Here comes products check for tokens which
      always have card inserted.  */
@@ -2056,15 +2116,38 @@ do_close_reader (ccid_driver_t handle)
     {
       if (handle->transfer)
         {
+          /* By locking libusb events, make sure handle->transfer is
+             canceled properly;  Don't cancel completed transfer.  */
+#ifdef USE_NPTH
+          npth_unprotect ();
+#endif
+          libusb_lock_events (NULL);
+#ifdef USE_NPTH
+          npth_protect ();
+#endif
           if (!handle->powered_off)
             {
               libusb_cancel_transfer (handle->transfer);
+              libusb_unlock_events (NULL);
+
               while (!handle->powered_off)
-                libusb_handle_events_completed (NULL, &handle->powered_off);
+                {
+#ifdef USE_NPTH
+                  npth_unprotect ();
+#endif
+                  libusb_handle_events_completed (NULL, &handle->powered_off);
+#ifdef USE_NPTH
+                  npth_protect ();
+#endif
+                }
             }
+          else
+            libusb_unlock_events (NULL);
+
           libusb_free_transfer (handle->transfer);
         }
       libusb_release_interface (handle->idev, handle->ifc_no);
+      --ccid_usb_thread_is_alive;
       libusb_close (handle->idev);
       handle->idev = NULL;
     }
@@ -2206,9 +2289,15 @@ bulk_out (ccid_driver_t handle, unsigned char *msg, size_t msglen,
     {
       int transferred;
 
+#ifdef USE_NPTH
+      npth_unprotect ();
+#endif
       rc = libusb_bulk_transfer (handle->idev, handle->ep_bulk_out,
                                  (char*)msg, msglen, &transferred,
                                  5000 /* ms timeout */);
+#ifdef USE_NPTH
+      npth_protect ();
+#endif
       if (rc == 0 && transferred == msglen)
         return 0;
 
@@ -2257,8 +2346,14 @@ bulk_in (ccid_driver_t handle, unsigned char *buffer, size_t length,
  retry:
   if (handle->idev)
     {
+#ifdef USE_NPTH
+      npth_unprotect ();
+#endif
       rc = libusb_bulk_transfer (handle->idev, handle->ep_bulk_in,
                                  (char*)buffer, length, &msglen, timeout);
+#ifdef USE_NPTH
+      npth_protect ();
+#endif
       if (rc)
         {
           DEBUGOUT_1 ("usb_bulk_read error: %s\n", libusb_error_name (rc));
@@ -2393,6 +2488,9 @@ abort_cmd (ccid_driver_t handle, int seqno)
   /* 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.  */
+#ifdef USE_NPTH
+  npth_unprotect ();
+#endif
   rc = libusb_control_transfer (handle->idev,
                                 0x21,/* bmRequestType: host-to-device,
                                         class specific, to interface.  */
@@ -2401,6 +2499,9 @@ abort_cmd (ccid_driver_t handle, int seqno)
                                 handle->ifc_no,
                                 dummybuf, 0,
                                 1000 /* ms timeout */);
+#ifdef USE_NPTH
+  npth_protect ();
+#endif
   if (rc)
     {
       DEBUGOUT_1 ("usb_control_msg error: %s\n", libusb_error_name (rc));
@@ -2425,9 +2526,15 @@ abort_cmd (ccid_driver_t handle, int seqno)
       msglen = 10;
       set_msg_len (msg, 0);
 
+#ifdef USE_NPTH
+      npth_unprotect ();
+#endif
       rc = libusb_bulk_transfer (handle->idev, handle->ep_bulk_out,
                                  (char*)msg, msglen, &transferred,
                                  5000 /* ms timeout */);
+#ifdef USE_NPTH
+      npth_protect ();
+#endif
       if (rc == 0 && transferred == msglen)
         rc = 0;
       else if (rc)
@@ -2437,9 +2544,15 @@ abort_cmd (ccid_driver_t handle, int seqno)
       if (rc)
         return rc;
 
+#ifdef USE_NPTH
+      npth_unprotect ();
+#endif
       rc = libusb_bulk_transfer (handle->idev, handle->ep_bulk_in,
                                  (char*)msg, sizeof msg, &msglen,
                                  5000 /*ms timeout*/);
+#ifdef USE_NPTH
+      npth_protect ();
+#endif
       if (rc)
         {
           DEBUGOUT_1 ("usb_bulk_read error in abort_cmd: %s\n",
@@ -2637,8 +2750,14 @@ ccid_slot_status (ccid_driver_t handle, int *statusbits)
       if (!retries)
         {
           DEBUGOUT ("USB: CALLING USB_CLEAR_HALT\n");
+#ifdef USE_NPTH
+          npth_unprotect ();
+#endif
           libusb_clear_halt (handle->idev, handle->ep_bulk_in);
           libusb_clear_halt (handle->idev, handle->ep_bulk_out);
+#ifdef USE_NPTH
+          npth_protect ();
+#endif
         }
       else
           DEBUGOUT ("USB: RETRYING bulk_in AGAIN\n");
@@ -3383,7 +3502,13 @@ ccid_transceive (ccid_driver_t handle,
 
       if (tpdulen < 4)
         {
+#ifdef USE_NPTH
+          npth_unprotect ();
+#endif
           libusb_clear_halt (handle->idev, handle->ep_bulk_in);
+#ifdef USE_NPTH
+          npth_protect ();
+#endif
           return CCID_DRIVER_ERR_ABORTED;
         }
 
@@ -3817,7 +3942,13 @@ ccid_transceive_secure (ccid_driver_t handle,
 
   if (tpdulen < 4)
     {
+#ifdef USE_NPTH
+      npth_unprotect ();
+#endif
       libusb_clear_halt (handle->idev, handle->ep_bulk_in);
+#ifdef USE_NPTH
+      npth_protect ();
+#endif
       return CCID_DRIVER_ERR_ABORTED;
     }
   if (debug_level > 1)
index 4ab0fcf..e2a6ba8 100644 (file)
 
 #include <assuan.h> /* malloc hooks */
 
-#ifdef HAVE_LIBUSB
-#include <libusb.h>
-#endif
-
 #include "i18n.h"
 #include "sysutils.h"
 #include "app-common.h"
@@ -229,17 +225,8 @@ static assuan_sock_nonce_t socket_nonce;
    disabled but it won't perform any ticker specific actions. */
 static int ticker_disabled;
 
-/* Set of FD to select.  */
-static fd_set fdset;
-
-/* Max FD to select.  */
-static int nfd;
-
-/* Set if all usb devices have INTERRUPT endpoints.  */
-static int usb_all_have_intr_endp;
-
-/* FD to listen incomming connections.  */
-static int listen_fd;
+/* Set if usb devices require periodical check.  */
+static int usb_periodical_check;
 
 /* FD to notify update of usb devices.  */
 static int notify_fd;
@@ -250,7 +237,7 @@ static gnupg_fd_t create_server_socket (const char *name,
                                         assuan_sock_nonce_t *nonce);
 
 static void *start_connection_thread (void *arg);
-static void handle_connections (void);
+static void handle_connections (int listen_fd);
 
 /* Pth wrapper function definitions. */
 ASSUAN_SYSTEM_NPTH_IMPL;
@@ -798,8 +785,7 @@ main (int argc, char **argv )
 
       /* We run handle_connection to wait for the shutdown signal and
          to run the ticker stuff.  */
-      listen_fd = fd;
-      handle_connections ();
+      handle_connections (fd);
       if (fd != -1)
         close (fd);
     }
@@ -931,8 +917,7 @@ main (int argc, char **argv )
 
 #endif /*!HAVE_W32_SYSTEM*/
 
-      listen_fd = fd;
-      handle_connections ();
+      handle_connections (fd);
 
       close (fd);
     }
@@ -1029,7 +1014,7 @@ handle_signal (int signo)
           log_info ("%s %s stopped\n", strusage(11), strusage(13) );
           cleanup ();
           scd_exit (0);
-       }
+        }
       break;
 
     case SIGINT:
@@ -1136,7 +1121,7 @@ create_server_socket (const char *name, char **r_redir_name,
  if (rc == -1)
     {
       log_error (_("error binding socket to '%s': %s\n"),
-                unaddr->sun_path,
+                 unaddr->sun_path,
                  gpg_strerror (gpg_error_from_syserror ()));
       assuan_sock_close (fd);
       scd_exit (2);
@@ -1202,43 +1187,17 @@ start_connection_thread (void *arg)
 
 
 void
-update_fdset_for_usb (int all_have_intr_endp)
+update_usb (int periodical_check_needed)
 {
-#ifdef HAVE_LIBUSB
-  const struct libusb_pollfd **pfd_array = libusb_get_pollfds (NULL);
-  const struct libusb_pollfd **p;
-#endif
-
-  usb_all_have_intr_endp = all_have_intr_endp;
-
-  FD_ZERO (&fdset);
-  nfd = 0;
-
-  if (listen_fd != -1)
-    {
-      FD_SET (listen_fd, &fdset);
-      nfd = listen_fd;
-    }
-
-#ifdef HAVE_LIBUSB
-  for (p = pfd_array; *p; p++)
-    {
-      int fd = (*p)->fd;
-
-      FD_SET (fd, &fdset);
-      if (nfd < fd)
-        nfd = fd;
-      p++;
-      log_debug ("USB: add %d to fdset\n", fd);
-    }
-
-  libusb_free_pollfds (pfd_array);
-#endif
+  usb_periodical_check = periodical_check_needed;
+  log_debug ("update_usb (%d)\n", periodical_check_needed);
+}
 
+void
+scd_kick_the_loop (void)
+{
   /* Kick the select loop.  */
   write (notify_fd, "", 1);
-
-  log_debug ("update_fdset_for_usb (%d): %d\n", all_have_intr_endp, nfd);
 }
 
 static int
@@ -1247,10 +1206,7 @@ need_tick (void)
   if (shutdown_pending)
     return 1;
 
-  if (listen_fd != -1 && nfd == listen_fd)
-    return 1;
-
-  if (usb_all_have_intr_endp)
+  if (!usb_periodical_check)
     return 0;
 
   return 1;
@@ -1261,12 +1217,13 @@ need_tick (void)
    in which case this code will only do regular timeouts and handle
    signals. */
 static void
-handle_connections (void)
+handle_connections (int listen_fd)
 {
   npth_attr_t tattr;
   struct sockaddr_un paddr;
   socklen_t plen;
-  fd_set read_fdset;
+  fd_set fdset, read_fdset;
+  int nfd;
   int ret;
   int fd;
   struct timespec abstime;
@@ -1314,6 +1271,10 @@ handle_connections (void)
       nfd = listen_fd;
     }
 
+  FD_SET (pipe_fd[0], &fdset);
+  if (nfd < pipe_fd[0])
+    nfd = pipe_fd[0];
+
   npth_clock_gettime (&curtime);
   timeout.tv_sec = TIMERTICK_INTERVAL_SEC;
   timeout.tv_nsec = TIMERTICK_INTERVAL_USEC * 1000;
@@ -1322,8 +1283,6 @@ handle_connections (void)
 
   for (;;)
     {
-      int max_fd;
-
       if (shutdown_pending)
         {
           if (active_connections == 0)
@@ -1334,10 +1293,13 @@ handle_connections (void)
              file descriptors to wait for, so that the select will be
              used to just wait on a signal or timeout event. */
           FD_ZERO (&fdset);
+          FD_SET (pipe_fd[0], &fdset);
+          nfd = pipe_fd[0];
           listen_fd = -1;
         }
 
-      if (need_tick ())
+      if ((listen_fd != -1 && nfd == listen_fd)
+          || need_tick ())
         {
           npth_clock_gettime (&curtime);
           if (!(npth_timercmp (&curtime, &abstime, <)))
@@ -1359,21 +1321,15 @@ handle_connections (void)
          thus a simple assignment is fine to copy the entire set.  */
       read_fdset = fdset;
 
-      FD_SET (pipe_fd[0], &read_fdset);
-      if (nfd < pipe_fd[0])
-        max_fd = pipe_fd[0];
-      else
-        max_fd = nfd;
-
 #ifndef HAVE_W32_SYSTEM
-      ret = npth_pselect (max_fd+1, &read_fdset, NULL, NULL, t,
+      ret = npth_pselect (nfd+1, &read_fdset, NULL, NULL, t,
                           npth_sigev_sigmask ());
       saved_errno = errno;
 
       while (npth_sigev_get_pending(&signo))
         handle_signal (signo);
 #else
-      ret = npth_eselect (max_fd+1, &read_fdset, NULL, NULL, t, NULL, NULL);
+      ret = npth_eselect (nfd+1, &read_fdset, NULL, NULL, t, NULL, NULL);
       saved_errno = errno;
 #endif
 
@@ -1431,19 +1387,7 @@ handle_connections (void)
               else
                 npth_setname_np (thread, threadname);
             }
-
-          ret--;
         }
-
-#ifdef HAVE_LIBUSB
-      if (ret)
-        {
-          struct timeval tv = {0, 0};
-
-          log_debug ("scd main: USB handle events\n");
-          libusb_handle_events_timeout_completed (NULL, &tv, NULL);
-        }
-#endif
     }
 
   close (pipe_fd[0]);
index 9e616f4..6d950b5 100644 (file)
@@ -125,7 +125,7 @@ void send_status_info (ctrl_t ctrl, const char *keyword, ...)
 void send_status_direct (ctrl_t ctrl, const char *keyword, const char *args);
 void scd_update_reader_status_file (void);
 void send_client_notifications (app_t app, int removal);
-void update_fdset_for_usb (int all_have_intr_endp);
-
+void update_usb (int periodical_check_needed);
+void scd_kick_the_loop (void);
 
 #endif /*SCDAEMON_H*/