Handle short-lived removals of reader in reader driver for PC/SC

src/libopensc/reader-pcsc.c

@@ -375,14 +375,29 @@ static int refresh_attributes(sc_reader_t *reader)
 
  if (rv != SCARD_S_SUCCESS) {
  if (rv == (LONG)SCARD_E_TIMEOUT) {
- /* Timeout, no change from previous recorded state. Make sure that
-  * changed flag is not set. */
- reader->flags &= ~SC_READER_CARD_CHANGED;
+ DWORD readers_len = 0, cstate = 0, prot, atr_len = SC_MAX_ATR_SIZE;
+ unsigned char atr[SC_MAX_ATR_SIZE];
+
  /* Make sure to preserve the CARD_PRESENT flag if the reader was
  * reattached and we called the refresh_attributes too recently */
  if (priv->reader_state.dwEventState & SCARD_STATE_PRESENT) {
  reader->flags |= SC_READER_CARD_PRESENT;
  }
+
+ /* Timeout should denote no change from previous recorded state,
+ * but check for valid card handle */
+ reader->flags &= ~SC_READER_CARD_CHANGED;
+ if (priv->pcsc_card != 0) {
+ /* When reader is removed between two subsequent calls to refresh_attributes,
+ * SCardGetStatusChange does not notice the change, test the card handle with SCardStatus */
+ rv = priv->gpriv->SCardStatus(priv->pcsc_card, NULL, &readers_len, &cstate, &prot, atr, &atr_len);
+
+ if (rv == (LONG)SCARD_W_REMOVED_CARD || rv == (LONG)SCARD_E_INVALID_VALUE || rv == (LONG)SCARD_E_INVALID_HANDLE)
+ reader->flags |= SC_READER_CARD_CHANGED;
+ /* If this happens, card must be reconnected, otherwise SCardGetStatusChange() will still return timeout
+ * and card handle will be invalid. */
+ }
+
  LOG_FUNC_RETURN(reader->ctx, SC_SUCCESS);
  }
 
@@ -392,7 +407,13 @@ static int refresh_attributes(sc_reader_t *reader)
  || rv == (LONG)SCARD_E_NO_READERS_AVAILABLE
 #endif
  || rv == (LONG)SCARD_E_SERVICE_STOPPED) {
- reader->flags &= ~(SC_READER_CARD_PRESENT);
+ /* Set flag when state changes */
+ if (reader->flags & SC_READER_REMOVED) {
+ reader->flags &= ~SC_READER_CARD_CHANGED;
+ } else {
+ reader->flags |= SC_READER_CARD_CHANGED;
+ }
+ reader->flags &= ~SC_READER_CARD_PRESENT;
  reader->flags |= SC_READER_REMOVED;
  priv->gpriv->removed_reader = reader;
  SC_FUNC_RETURN(reader->ctx, SC_LOG_DEBUG_VERBOSE, SC_SUCCESS);
@@ -417,7 +438,7 @@ static int refresh_attributes(sc_reader_t *reader)
  SC_FUNC_RETURN(reader->ctx, SC_LOG_DEBUG_VERBOSE, SC_SUCCESS);
  }
 
- reader->flags &= ~(SC_READER_CARD_CHANGED|SC_READER_CARD_INUSE|SC_READER_CARD_EXCLUSIVE);
+ reader->flags &= ~(SC_READER_CARD_CHANGED|SC_READER_CARD_INUSE|SC_READER_CARD_EXCLUSIVE|SC_READER_REMOVED);
 
  if (state & SCARD_STATE_PRESENT) {
  reader->flags |= SC_READER_CARD_PRESENT;

src/pkcs11/slot.c

@@ -215,6 +215,7 @@ CK_RV card_detect(sc_reader_t *reader)
  CK_RV rv;
  unsigned int i;
  int j;
+ int retry = 3;
 
  sc_log(context, "%s: Detecting smart card", reader->name);
  /* Check if someone inserted a card */
@@ -235,10 +236,10 @@ CK_RV card_detect(sc_reader_t *reader)
  sc_log(context, "%s: Card changed", reader->name);
  /* The following should never happen - but if it
  * does we'll be stuck in an endless loop.
- * So better be fussy.
- if (!retry--)
- return CKR_TOKEN_NOT_PRESENT; */
+ * So better be fussy.*/
  card_removed(reader);
+ if (!retry--)
+ return CKR_TOKEN_NOT_PRESENT;
  goto again;
  }
 
@@ -427,6 +428,17 @@ card_detect_all(void)
  }
  }
  card_detect(reader);
+
+ /* If reader was removed, card_removed() was already called in card_detect(),
+ * remove only relation between reader and slot. */
+ if (reader->flags & SC_READER_REMOVED) {
+ for (j = 0; j<list_size(&virtual_slots); j++) {
+ sc_pkcs11_slot_t *slot = (sc_pkcs11_slot_t *) list_get_at(&virtual_slots, j);
+ if (slot->reader == reader) {
+ slot->reader = NULL;
+ }
+ }
+ }
  }
  }
  sc_log(context, "All cards detected");