From f08d37af049bf1718b301644020658dd2bb07638 Mon Sep 17 00:00:00 2001 From: NIIBE Yutaka Date: Tue, 31 Jan 2017 12:56:11 +0900 Subject: [PATCH] scd: Fix SERIALNO for multiple devices. * scd/app.c (select_application): Fix the logic if periodical check is needed. If it is needed for newly found device(s), kick the loop. (scd_update_reader_status_file): Return value if select(2) should be called with timeout. * scd/ccid-driver.c (ccid_require_get_status): Don't return 0 for token with no interrupt transfer for now. * scd/command.c (open_card_with_request): Fix scan by SERIALNO. * scd/scdaemon.c (update_usb): Remove. (handle_connections): Evaluate need_tick after handle_tick. Signed-off-by: NIIBE Yutaka --- scd/app.c | 24 +++++++------ scd/ccid-driver.c | 25 ++++++++++---- scd/command.c | 7 +++- scd/scdaemon.c | 86 +++++++++++++++++------------------------------ scd/scdaemon.h | 5 +-- 5 files changed, 71 insertions(+), 76 deletions(-) diff --git a/scd/app.c b/scd/app.c index 2cf7d11ed..5b8da1c38 100644 --- a/scd/app.c +++ b/scd/app.c @@ -333,6 +333,7 @@ select_application (ctrl_t ctrl, const char *name, app_t *r_app, struct dev_list *l; int periodical_check_needed = 0; + /* Scan the devices to find new device(s). */ err = apdu_dev_list_start (opt.reader_port, &l); if (err) return err; @@ -340,14 +341,14 @@ select_application (ctrl_t ctrl, const char *name, app_t *r_app, while (1) { int slot; - int periodical_check_needed; + int periodical_check_needed_this; slot = apdu_open_reader (l); if (slot < 0) break; - periodical_check_needed = apdu_connect (slot); - if (periodical_check_needed < 0) + periodical_check_needed_this = apdu_connect (slot); + if (periodical_check_needed_this < 0) { /* We close a reader with no card. */ err = gpg_error (GPG_ERR_ENODEV); @@ -355,8 +356,8 @@ select_application (ctrl_t ctrl, const char *name, app_t *r_app, else { err = app_new_register (slot, ctrl, name, - periodical_check_needed); - if (periodical_check_needed) + periodical_check_needed_this); + if (periodical_check_needed_this) periodical_check_needed = 1; } @@ -365,7 +366,11 @@ select_application (ctrl_t ctrl, const char *name, app_t *r_app, } apdu_dev_list_finish (l); - update_usb (periodical_check_needed); + + /* If periodical check is needed for new device(s), kick the + scdaemon loop. */ + if (periodical_check_needed) + scd_kick_the_loop (); } npth_mutex_lock (&app_list_lock); @@ -1011,12 +1016,11 @@ report_change (int slot, int old_status, int cur_status) xfree (homestr); } -void +int scd_update_reader_status_file (void) { app_t a, app_next; int periodical_check_needed = 0; - int removal_detected = 0; npth_mutex_lock (&app_list_lock); for (a = app_top; a; a = app_next) @@ -1050,7 +1054,6 @@ scd_update_reader_status_file (void) log_debug ("Removal of a card: %d\n", a->slot); apdu_close_reader (a->slot); deallocate_app (a); - removal_detected = 1; } else { @@ -1067,8 +1070,7 @@ scd_update_reader_status_file (void) } npth_mutex_unlock (&app_list_lock); - if (removal_detected) - update_usb (periodical_check_needed); + return periodical_check_needed; } /* This function must be called once to initialize this module. This diff --git a/scd/ccid-driver.c b/scd/ccid-driver.c index 7124c6e7a..1a01ff0a1 100644 --- a/scd/ccid-driver.c +++ b/scd/ccid-driver.c @@ -2073,17 +2073,28 @@ ccid_open_reader (const char *spec_reader_name, int idx, int ccid_require_get_status (ccid_driver_t handle) { + /* When a card reader supports interrupt transfer to check the + status of card, it is possible to submit only an interrupt + transfer, and no check is required by application layer. USB can + detect removal of a card and can detect removal of a reader. + */ if (handle->ep_intr >= 0) return 0; - /* Here comes products check for tokens which - always have card inserted. */ - switch (handle->id_vendor) - { - case VENDOR_FSIJ: - return 0; - } + /* Libusb actually detects the removal of USB device in use. + However, there is no good API to handle the removal (yet), + cleanly and with good portability. + There is libusb_set_pollfd_notifiers function, but it doesn't + offer libusb_device_handle* data to its callback. So, when it + watches multiple devices, there is no way to know which device is + removed. + + Once, we will have a good programming interface of libusb, we can + list tokens (with no interrupt transfer support, but always with + card inserted) here to return 0, so that scdaemon can submit + minimum packet on wire. + */ return 1; } diff --git a/scd/command.c b/scd/command.c index 26f8630fb..0ae6d29aa 100644 --- a/scd/command.c +++ b/scd/command.c @@ -217,13 +217,18 @@ open_card_with_request (ctrl_t ctrl, const char *apptype, const char *serialno) gpg_error_t err; unsigned char *serialno_bin = NULL; size_t serialno_bin_len = 0; + app_t app = ctrl->app_ctx; /* If we are already initialized for one specific application we need to check that the client didn't requested a specific application different from the one in use before we continue. */ - if (ctrl->app_ctx) + if (apptype && ctrl->app_ctx) return check_application_conflict (apptype, ctrl->app_ctx); + /* Re-scan USB devices. Release APP, before the scan. */ + ctrl->app_ctx = NULL; + release_application (app); + if (serialno) serialno_bin = hex_to_buffer (serialno, &serialno_bin_len); diff --git a/scd/scdaemon.c b/scd/scdaemon.c index e2a6ba86c..3cc35079a 100644 --- a/scd/scdaemon.c +++ b/scd/scdaemon.c @@ -61,10 +61,10 @@ enum cmd_and_opt_values { aNull = 0, - oCsh = 'c', - oQuiet = 'q', - oSh = 's', - oVerbose = 'v', + oCsh = 'c', + oQuiet = 'q', + oSh = 's', + oVerbose = 'v', oNoVerbose = 500, aGPGConfList, @@ -115,11 +115,11 @@ static ARGPARSE_OPTS opts[] = { N_("run in multi server mode (foreground)")), ARGPARSE_s_n (oDaemon, "daemon", N_("run in daemon mode (background)")), ARGPARSE_s_n (oVerbose, "verbose", N_("verbose")), - ARGPARSE_s_n (oQuiet, "quiet", N_("be somewhat more quiet")), - ARGPARSE_s_n (oSh, "sh", N_("sh-style command output")), - ARGPARSE_s_n (oCsh, "csh", N_("csh-style command output")), + ARGPARSE_s_n (oQuiet, "quiet", N_("be somewhat more quiet")), + ARGPARSE_s_n (oSh, "sh", N_("sh-style command output")), + ARGPARSE_s_n (oCsh, "csh", N_("csh-style command output")), ARGPARSE_s_s (oOptions, "options", N_("|FILE|read options from FILE")), - ARGPARSE_s_s (oDebug, "debug", "@"), + ARGPARSE_s_s (oDebug, "debug", "@"), ARGPARSE_s_n (oDebugAll, "debug-all", "@"), ARGPARSE_s_s (oDebugLevel, "debug-level" , N_("|LEVEL|set the debugging level to LEVEL")), @@ -461,13 +461,13 @@ main (int argc, char **argv ) parse_debug++; else if (pargs.r_opt == oOptions) { /* yes there is one, so we do not try the default one, but - read the option file when it is encountered at the - commandline */ + read the option file when it is encountered at the + commandline */ default_config = 0; - } - else if (pargs.r_opt == oNoOptions) + } + else if (pargs.r_opt == oNoOptions) default_config = 0; /* --no-options */ - else if (pargs.r_opt == oHomedir) + else if (pargs.r_opt == oHomedir) gnupg_set_homedir (pargs.r.ret_str); } @@ -502,16 +502,16 @@ main (int argc, char **argv ) if( parse_debug ) log_info (_("Note: no default option file '%s'\n"), configname ); - } + } else { log_error (_("option file '%s': %s\n"), configname, strerror(errno) ); exit(2); - } + } xfree (configname); configname = NULL; - } + } if (parse_debug && configname ) log_info (_("reading options from '%s'\n"), configname ); default_config = 0; @@ -558,10 +558,10 @@ main (int argc, char **argv ) /* config files may not be nested (silently ignore them) */ if (!configfp) { - xfree(configname); - configname = xstrdup(pargs.r.ret_str); - goto next_pass; - } + xfree(configname); + configname = xstrdup(pargs.r.ret_str); + goto next_pass; + } break; case oNoGreeting: nogreeting = 1; break; case oNoVerbose: opt.verbose = 0; break; @@ -593,12 +593,12 @@ main (int argc, char **argv ) add_to_strlist (&opt.disabled_applications, pargs.r.ret_str); break; - case oEnablePinpadVarlen: opt.enable_pinpad_varlen = 1; break; + case oEnablePinpadVarlen: opt.enable_pinpad_varlen = 1; break; default: pargs.err = configfp? ARGPARSE_PRINT_WARNING:ARGPARSE_PRINT_ERROR; break; - } + } } if (configfp) { @@ -661,7 +661,7 @@ main (int argc, char **argv ) char *filename_esc; if (config_filename) - filename = xstrdup (config_filename); + filename = xstrdup (config_filename); else filename = make_filename (gnupg_homedir (), SCDAEMON_NAME EXTSEP_S "conf", NULL); @@ -1035,7 +1035,7 @@ static void handle_tick (void) { if (!ticker_disabled) - scd_update_reader_status_file (); + usb_periodical_check = scd_update_reader_status_file (); } @@ -1186,13 +1186,6 @@ start_connection_thread (void *arg) } -void -update_usb (int periodical_check_needed) -{ - usb_periodical_check = periodical_check_needed; - log_debug ("update_usb (%d)\n", periodical_check_needed); -} - void scd_kick_the_loop (void) { @@ -1226,8 +1219,6 @@ handle_connections (int listen_fd) int nfd; int ret; int fd; - struct timespec abstime; - struct timespec curtime; struct timespec timeout; struct timespec *t; int saved_errno; @@ -1275,12 +1266,6 @@ handle_connections (int listen_fd) 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; - npth_timeradd (&curtime, &timeout, &abstime); - /* We only require abstime here. The others will be reused. */ - for (;;) { if (shutdown_pending) @@ -1298,25 +1283,16 @@ handle_connections (int listen_fd) listen_fd = -1; } - if ((listen_fd != -1 && nfd == listen_fd) - || need_tick ()) - { - npth_clock_gettime (&curtime); - if (!(npth_timercmp (&curtime, &abstime, <))) - { - /* Timeout. */ - timeout.tv_sec = TIMERTICK_INTERVAL_SEC; - timeout.tv_nsec = TIMERTICK_INTERVAL_USEC * 1000; - npth_timeradd (&curtime, &timeout, &abstime); - } - npth_timersub (&abstime, &curtime, &timeout); - t = &timeout; - } + handle_tick (); + + timeout.tv_sec = TIMERTICK_INTERVAL_SEC; + timeout.tv_nsec = TIMERTICK_INTERVAL_USEC * 1000; + + if (need_tick ()) + t = &timeout; else t = NULL; - handle_tick (); - /* POSIX says that fd_set should be implemented as a structure, thus a simple assignment is fine to copy the entire set. */ read_fdset = fdset; diff --git a/scd/scdaemon.h b/scd/scdaemon.h index 6d950b5c0..37590b65e 100644 --- a/scd/scdaemon.h +++ b/scd/scdaemon.h @@ -123,9 +123,10 @@ int scd_command_handler (ctrl_t, int); void send_status_info (ctrl_t ctrl, const char *keyword, ...) GPGRT_ATTR_SENTINEL(1); 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_usb (int periodical_check_needed); void scd_kick_the_loop (void); +/*-- app.c --*/ +int scd_update_reader_status_file (void); + #endif /*SCDAEMON_H*/