From 25cc8575da9a9b8bf60c64c8059cb5f73cc52e1d Mon Sep 17 00:00:00 2001 From: NIIBE Yutaka Date: Fri, 27 Jan 2017 18:01:52 +0900 Subject: [PATCH] scd: Improve watching USB device removal. * scd/apdu.c(struct reader_table_s): Add require_get_status. (apdu_connect): Change return value meaning. Call apdu_reset here. * scd/app.c (app_new_register): Add require_get_status. (select_application): Use the return value of apdu_connect. (scd_update_reader_status_file): Call update_fdset_for_usb with checking all_have_intr_endp. (app_list_start, app_list_finish): Remove. * scd/ccid-driver.c (struct ccid_driver_s): Add transfer. (intr_cb): Don't call libusb_transfer in this callback. (ccid_require_get_status): New. (do_close_reader): Call libusb_transfer here. * scd/scdaemon.c (update_fdset_for_usb): Remove the first argument. -- With Gnuk Token, it works fine as expected. With Gemalto reader, intr_cb is not called when card is removed. So, the macro LIBUSB_WORKS_EXPECTED_FOR_INTERRUPT_ENDP is not defined yet. Signed-off-by: NIIBE Yutaka --- scd/apdu.c | 47 ++++++++++------- scd/apdu.h | 2 +- scd/app-common.h | 2 - scd/app.c | 126 ++++++++++++++++++++++------------------------ scd/ccid-driver.c | 44 ++++++++++------ scd/ccid-driver.h | 4 +- scd/scdaemon.c | 12 +++-- scd/scdaemon.h | 2 +- 8 files changed, 130 insertions(+), 109 deletions(-) diff --git a/scd/apdu.c b/scd/apdu.c index f88d97035..0037c476c 100644 --- a/scd/apdu.c +++ b/scd/apdu.c @@ -141,11 +141,12 @@ struct reader_table_s { } rapdu; #endif /*USE_G10CODE_RAPDU*/ char *rdrname; /* Name of the connected reader or NULL if unknown. */ - int is_t0; /* True if we know that we are running T=0. */ - int is_spr532; /* True if we know that the reader is a SPR532. */ - int pinpad_varlen_supported; /* True if we know that the reader - supports variable length pinpad - input. */ + unsigned int is_t0:1; /* True if we know that we are running T=0. */ + unsigned int is_spr532:1; /* True if we know that the reader is a SPR532. */ + unsigned int pinpad_varlen_supported:1; /* True if we know that the reader + supports variable length pinpad + input. */ + unsigned int require_get_status:1; unsigned char atr[33]; size_t atrlen; /* A zero length indicates that the ATR has not yet been read; i.e. the card is not @@ -470,6 +471,7 @@ new_reader_slot (void) reader_table[reader].is_t0 = 1; reader_table[reader].is_spr532 = 0; reader_table[reader].pinpad_varlen_supported = 0; + reader_table[reader].require_get_status = 1; #ifdef NEED_PCSC_WRAPPER reader_table[reader].pcsc.req_fd = -1; reader_table[reader].pcsc.rsp_fd = -1; @@ -2572,6 +2574,7 @@ open_ccid_reader (struct dev_list *dl) { int err; int slot; + int require_get_status; reader_table_t slotp; slot = new_reader_slot (); @@ -2596,6 +2599,8 @@ open_ccid_reader (struct dev_list *dl) err = 0; } + require_get_status = ccid_require_get_status (slotp->ccid.handle); + reader_table[slot].close_reader = close_ccid_reader; reader_table[slot].reset_reader = reset_ccid_reader; reader_table[slot].get_status_reader = get_status_ccid; @@ -2608,6 +2613,7 @@ open_ccid_reader (struct dev_list *dl) /* Our CCID reader code does not support T=0 at all, thus reset the flag. */ reader_table[slot].is_t0 = 0; + reader_table[slot].require_get_status = require_get_status; dump_reader_status (slot); unlock_slot (slot); @@ -2970,22 +2976,15 @@ apdu_dev_list_start (const char *portstr, struct dev_list **l_p) return 0; } -int +void apdu_dev_list_finish (struct dev_list *dl) { - int all_have_intr_endp; - #ifdef HAVE_LIBUSB if (dl->ccid_table) - all_have_intr_endp = ccid_dev_scan_finish (dl->ccid_table, dl->idx_max); - else - all_have_intr_endp = 0; -#else - all_have_intr_endp = 0; + ccid_dev_scan_finish (dl->ccid_table, dl->idx_max); #endif xfree (dl); npth_mutex_unlock (&reader_table_lock); - return all_have_intr_endp; } @@ -3347,8 +3346,11 @@ apdu_enum_reader (int slot, int *used) /* Connect a card. This is used to power up the card and make sure that an ATR is available. Depending on the reader backend it may - return an error for an inactive card or if no card is - available. */ + return an error for an inactive card or if no card is available. + Return -1 on error. Return 1 if reader requires get_status to + watch card removal. Return 0 if it's a token (always with a card), + or it supports INTERRUPT endpoint to watch card removal. + */ int apdu_connect (int slot) { @@ -3362,7 +3364,7 @@ apdu_connect (int slot) { if (DBG_READER) log_debug ("leave: apdu_connect => SW_HOST_NO_DRIVER\n"); - return SW_HOST_NO_DRIVER; + return -1; } /* Only if the access method provides a connect function we use it. @@ -3393,10 +3395,19 @@ apdu_connect (int slot) else if ((status & APDU_CARD_PRESENT) && !(status & APDU_CARD_ACTIVE)) sw = SW_HOST_CARD_INACTIVE; + if (sw == SW_HOST_CARD_INACTIVE) + { + /* Try power it up again. */ + sw = apdu_reset (slot); + } + if (DBG_READER) log_debug ("leave: apdu_connect => sw=0x%x\n", sw); - return sw; + if (sw) + return -1; + + return reader_table[slot].require_get_status; } diff --git a/scd/apdu.h b/scd/apdu.h index d71c8dd5d..473def518 100644 --- a/scd/apdu.h +++ b/scd/apdu.h @@ -88,7 +88,7 @@ struct dev_list; gpg_error_t apdu_init (void); gpg_error_t apdu_dev_list_start (const char *portstr, struct dev_list **l_p); -int apdu_dev_list_finish (struct dev_list *l); +void apdu_dev_list_finish (struct dev_list *l); /* Note, that apdu_open_reader returns no status word but -1 on error. */ int apdu_open_reader (struct dev_list *l); diff --git a/scd/app-common.h b/scd/app-common.h index b979f5476..521fcf329 100644 --- a/scd/app-common.h +++ b/scd/app-common.h @@ -121,8 +121,6 @@ size_t app_help_read_length_of_cert (int slot, int fid, size_t *r_certoff); /*-- app.c --*/ -app_t app_list_start (void); -void app_list_finish (void); void app_send_card_list (ctrl_t ctrl); char *app_get_serialno (app_t app); diff --git a/scd/app.c b/scd/app.c index daff0b7d7..3cf219cd9 100644 --- a/scd/app.c +++ b/scd/app.c @@ -174,7 +174,8 @@ 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) +app_new_register (int slot, ctrl_t ctrl, const char *name, + int require_get_status) { gpg_error_t err = 0; app_t app = NULL; @@ -303,7 +304,7 @@ app_new_register (int slot, ctrl_t ctrl, const char *name) return err; } - app->require_get_status = 1; /* For token, this can be 0. */ + app->require_get_status = require_get_status; npth_mutex_lock (&app_list_lock); app->next = app_top; @@ -330,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; + int all_have_intr_endp = 1; err = apdu_dev_list_start (opt.reader_port, &l); if (err) @@ -339,39 +340,31 @@ select_application (ctrl_t ctrl, const char *name, app_t *r_app, while (1) { int slot; - int sw; + int require_get_status; slot = apdu_open_reader (l); if (slot < 0) break; - err = 0; - sw = apdu_connect (slot); - - if (sw == SW_HOST_CARD_INACTIVE) - { - /* Try again. */ - sw = apdu_reset (slot); - } - - if (!sw || sw == SW_HOST_ALREADY_CONNECTED) - err = 0; - else if (sw == SW_HOST_NO_CARD) - err = gpg_error (GPG_ERR_CARD_NOT_PRESENT); - else - err = gpg_error (GPG_ERR_ENODEV); - - if (!err) - err = app_new_register (slot, ctrl, name); - else + require_get_status = apdu_connect (slot); + if (require_get_status < 0) { /* We close a reader with no card. */ - apdu_close_reader (slot); + 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; + } + + if (err) + apdu_close_reader (slot); } - all_have_intr_endp = apdu_dev_list_finish (l); - update_fdset_for_usb (1, all_have_intr_endp); + apdu_dev_list_finish (l); + update_fdset_for_usb (all_have_intr_endp); } npth_mutex_lock (&app_list_lock); @@ -1021,46 +1014,60 @@ void scd_update_reader_status_file (void) { app_t a, app_next; + int all_have_intr_endp = 1; + int removal_detected = 0; npth_mutex_lock (&app_list_lock); for (a = app_top; a; a = app_next) { + int sw; + unsigned int status; + + sw = apdu_get_status (a->slot, 0, &status); app_next = a->next; - if (a->require_get_status) + + if (sw == SW_HOST_NO_READER) { - int sw; - unsigned int status; - sw = apdu_get_status (a->slot, 0, &status); + /* Most likely the _reader_ has been unplugged. */ + status = 0; + } + else if (sw) + { + /* Get status failed. Ignore that. */ + if (a->require_get_status) + all_have_intr_endp = 0; + continue; + } - if (sw == SW_HOST_NO_READER) - { - /* Most likely the _reader_ has been unplugged. */ - status = 0; - } - else if (sw) - { - /* Get status failed. Ignore that. */ - continue; - } + if (a->card_status != status) + { + report_change (a->slot, a->card_status, status); + send_client_notifications (a, status == 0); - if (a->card_status != status) + if (status == 0) { - report_change (a->slot, a->card_status, status); - send_client_notifications (a, status == 0); - - if (status == 0) - { - log_debug ("Removal of a card: %d\n", a->slot); - apdu_close_reader (a->slot); - deallocate_app (a); - update_fdset_for_usb (0, 0); - } - else - a->card_status = status; + log_debug ("Removal of a card: %d\n", a->slot); + apdu_close_reader (a->slot); + deallocate_app (a); + removal_detected = 1; } + else + { + a->card_status = status; + if (a->require_get_status) + all_have_intr_endp = 0; + } + } + else + { + if (a->require_get_status) + all_have_intr_endp = 0; } } npth_mutex_unlock (&app_list_lock); + + if (removal_detected) + update_fdset_for_usb (all_have_intr_endp); } /* This function must be called once to initialize this module. This @@ -1082,19 +1089,6 @@ initialize_module_command (void) return apdu_init (); } -app_t -app_list_start (void) -{ - npth_mutex_lock (&app_list_lock); - return app_top; -} - -void -app_list_finish (void) -{ - npth_mutex_unlock (&app_list_lock); -} - void app_send_card_list (ctrl_t ctrl) { diff --git a/scd/ccid-driver.c b/scd/ccid-driver.c index 8ca8e9646..1bb0fb267 100644 --- a/scd/ccid-driver.c +++ b/scd/ccid-driver.c @@ -274,6 +274,7 @@ struct ccid_driver_s void *progress_cb_arg; unsigned char intr_buf[64]; + struct libusb_transfer *transfer; }; @@ -1699,17 +1700,13 @@ ccid_dev_scan (int *idx_max_p, struct ccid_dev_table **t_p) return err; } -int +void ccid_dev_scan_finish (struct ccid_dev_table *tbl, int max) { - int all_have_intr_endp = 1; int i; for (i = 0; i < max; i++) { - if (tbl[i].ep_intr == -1) - all_have_intr_endp = 0; - free (tbl[i].ifcdesc_extra); tbl[i].transport = 0; tbl[i].n = 0; @@ -1723,8 +1720,6 @@ ccid_dev_scan_finish (struct ccid_dev_table *tbl, int max) } libusb_free_device_list (ccid_usb_dev_list, 1); ccid_usb_dev_list = NULL; - - return all_have_intr_endp; } unsigned int @@ -1767,14 +1762,14 @@ intr_cb (struct libusb_transfer *transfer) { ccid_driver_t handle = transfer->user_data; - DEBUGOUT ("CCID: interrupt callback\n"); + DEBUGOUT_1 ("CCID: interrupt callback %d\n", transfer->status); - if (transfer->status == LIBUSB_TRANSFER_NO_DEVICE) + if (transfer->status == LIBUSB_TRANSFER_NO_DEVICE + || transfer->status == LIBUSB_TRANSFER_ERROR) { device_removed: DEBUGOUT ("CCID: device removed\n"); handle->powered_off = 1; - libusb_free_transfer (transfer); } else if (transfer->status == LIBUSB_TRANSFER_COMPLETED) { @@ -1784,7 +1779,6 @@ intr_cb (struct libusb_transfer *transfer) { DEBUGOUT ("CCID: card removed\n"); handle->powered_off = 1; - libusb_free_transfer (transfer); } else { @@ -1795,12 +1789,12 @@ intr_cb (struct libusb_transfer *transfer) if (err == LIBUSB_ERROR_NO_DEVICE) goto device_removed; - DEBUGOUT_1 ("CCID submit transfer again %d", err); + DEBUGOUT_1 ("CCID submit transfer again %d\n", err); } } else { - ; + DEBUGOUT_1 ("CCID intr_cb: %d\n", transfer->status); } } @@ -1811,6 +1805,7 @@ ccid_setup_intr (ccid_driver_t handle) int err; transfer = libusb_alloc_transfer (0); + handle->transfer = transfer; libusb_fill_interrupt_transfer (transfer, handle->idev, handle->ep_intr, handle->intr_buf, sizeof (handle->intr_buf), intr_cb, handle, 0); @@ -1913,7 +1908,7 @@ ccid_open_usb_reader (const char *spec_reader_name, } } - if ((*handle)->ep_intr) + if ((*handle)->ep_intr >= 0) ccid_setup_intr (*handle); rc = ccid_vendor_specific_init (*handle); @@ -2010,6 +2005,26 @@ 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. */ + switch (handle->id_vendor) + { + case VENDOR_FSIJ: + return 0; + } + + return 1; +} + + static void do_close_reader (ccid_driver_t handle) { @@ -2037,6 +2052,7 @@ do_close_reader (ccid_driver_t handle) } if (handle->idev) { + libusb_free_transfer (handle->transfer); libusb_release_interface (handle->idev, handle->ifc_no); libusb_close (handle->idev); handle->idev = NULL; diff --git a/scd/ccid-driver.h b/scd/ccid-driver.h index 8e9f9e251..cff3f198f 100644 --- a/scd/ccid-driver.h +++ b/scd/ccid-driver.h @@ -115,7 +115,7 @@ int ccid_set_debug_level (int level); char *ccid_get_reader_list (void); gpg_error_t ccid_dev_scan (int *idx_max, struct ccid_dev_table **t_p); -int ccid_dev_scan_finish (struct ccid_dev_table *tbl, int max); +void ccid_dev_scan_finish (struct ccid_dev_table *tbl, int max); unsigned int ccid_get_BAI (int, struct ccid_dev_table *tbl); int ccid_compare_BAI (ccid_driver_t handle, unsigned int); int ccid_open_reader (const char *spec_reader_name, @@ -140,7 +140,7 @@ int ccid_transceive_escape (ccid_driver_t handle, const unsigned char *data, size_t datalen, unsigned char *resp, size_t maxresplen, size_t *nresp); - +int ccid_require_get_status (ccid_driver_t handle); #endif /*CCID_DRIVER_H*/ diff --git a/scd/scdaemon.c b/scd/scdaemon.c index 14e0b05f0..4ab0fcf9a 100644 --- a/scd/scdaemon.c +++ b/scd/scdaemon.c @@ -1202,15 +1202,14 @@ start_connection_thread (void *arg) void -update_fdset_for_usb (int scanned, int all_have_intr_endp) +update_fdset_for_usb (int all_have_intr_endp) { #ifdef HAVE_LIBUSB const struct libusb_pollfd **pfd_array = libusb_get_pollfds (NULL); const struct libusb_pollfd **p; #endif - if (scanned) - usb_all_have_intr_endp = all_have_intr_endp; + usb_all_have_intr_endp = all_have_intr_endp; FD_ZERO (&fdset); nfd = 0; @@ -1230,6 +1229,7 @@ update_fdset_for_usb (int scanned, int all_have_intr_endp) if (nfd < fd) nfd = fd; p++; + log_debug ("USB: add %d to fdset\n", fd); } libusb_free_pollfds (pfd_array); @@ -1238,8 +1238,7 @@ update_fdset_for_usb (int scanned, int all_have_intr_endp) /* Kick the select loop. */ write (notify_fd, "", 1); - log_debug ("update_fdset_for_usb (%d, %d): %d %lx\n", - scanned, all_have_intr_endp, nfd, fdset.fds_bits[0]); + log_debug ("update_fdset_for_usb (%d): %d\n", all_have_intr_endp, nfd); } static int @@ -1395,6 +1394,7 @@ handle_connections (void) char buf[256]; read (pipe_fd[0], buf, sizeof buf); + ret--; } if (listen_fd != -1 && FD_ISSET (listen_fd, &read_fdset)) @@ -1439,6 +1439,8 @@ handle_connections (void) if (ret) { struct timeval tv = {0, 0}; + + log_debug ("scd main: USB handle events\n"); libusb_handle_events_timeout_completed (NULL, &tv, NULL); } #endif diff --git a/scd/scdaemon.h b/scd/scdaemon.h index 9d92ff2c3..9e616f4c9 100644 --- a/scd/scdaemon.h +++ b/scd/scdaemon.h @@ -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 scanned, int all_have_intr_endp); +void update_fdset_for_usb (int all_have_intr_endp); #endif /*SCDAEMON_H*/