[Pkg-gnupg-commit] [gnupg2] 25/166: scd: Improve watching USB device removal.
Daniel Kahn Gillmor
dkg at fifthhorseman.net
Thu Mar 16 22:33:02 UTC 2017
This is an automated email from the git hooks/post-receive script.
dkg pushed a commit to branch experimental
in repository gnupg2.
commit 25cc8575da9a9b8bf60c64c8059cb5f73cc52e1d
Author: NIIBE Yutaka <gniibe at fsij.org>
Date: Fri Jan 27 18:01:52 2017 +0900
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 <gniibe at fsij.org>
---
scd/apdu.c | 47 +++++++++++++--------
scd/apdu.h | 2 +-
scd/app-common.h | 2 -
scd/app.c | 120 ++++++++++++++++++++++++++----------------------------
scd/ccid-driver.c | 44 +++++++++++++-------
scd/ccid-driver.h | 4 +-
scd/scdaemon.c | 12 +++---
scd/scdaemon.h | 2 +-
8 files changed, 127 insertions(+), 106 deletions(-)
diff --git a/scd/apdu.c b/scd/apdu.c
index f88d970..0037c47 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 d71c8dd..473def5 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 b979f54..521fcf3 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 daff0b7..3cf219c 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)
+ require_get_status = apdu_connect (slot);
+ if (require_get_status < 0)
{
- /* Try again. */
- sw = apdu_reset (slot);
+ /* We close a reader with no card. */
+ err = gpg_error (GPG_ERR_ENODEV);
}
-
- 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
{
- /* We close a reader with no card. */
- apdu_close_reader (slot);
+ 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)
+ {
+ /* 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 (a->card_status != status)
{
- int sw;
- unsigned int status;
- sw = apdu_get_status (a->slot, 0, &status);
+ report_change (a->slot, a->card_status, status);
+ send_client_notifications (a, status == 0);
- if (sw == SW_HOST_NO_READER)
- {
- /* Most likely the _reader_ has been unplugged. */
- status = 0;
- }
- else if (sw)
+ if (status == 0)
{
- /* Get status failed. Ignore that. */
- continue;
+ log_debug ("Removal of a card: %d\n", a->slot);
+ apdu_close_reader (a->slot);
+ deallocate_app (a);
+ removal_detected = 1;
}
-
- if (a->card_status != status)
+ else
{
- 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;
+ 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 8ca8e96..1bb0fb2 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 8e9f9e2..cff3f19 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 14e0b05..4ab0fcf 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 9d92ff2..9e616f4 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*/
--
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-gnupg/gnupg2.git
More information about the Pkg-gnupg-commit
mailing list