From 484bafda4dbf5ffe9e7c41ef24fbc5bd791a3b32 Mon Sep 17 00:00:00 2001
From: NIIBE Yutaka <gniibe@fsij.org>
Date: Thu, 5 Nov 2020 15:10:18 +0900
Subject: [PATCH] scd: Internal CCID driver: Fix a race condition on close.

* scd/ccid-driver.c (ccid_require_get_status): For VENDOR_SCM reader,
return 0 only at the initial call.
(bulk_in): Don't detect an error for VENDOR_SCM reader, just kicking
the loop, to invoke scd_update_reader_status_file, which calls
ccid_slot_status again.
(ccid_slot_status): Move the call of ccid_vendor_specific_setup to...
(ccid_get_atr): ... here.

--

For readers with interrupt transfer support, it is only intr_cb which
sets handle->powered_off to 1.  Keeping this condition makes no race.
The function ccid_slot_status can also detect a communication error,
which causes apdu_close_reader (but not setting ->powered_off).

GnuPG-bug-id: 5121
Fixes-commit: 920f258eb6018ecec1d63bad6a0fb0772f72affa
Signed-off-by: NIIBE Yutaka <gniibe@fsij.org>
---
 scd/ccid-driver.c | 39 ++++++++++++++++++++++++++-------------
 1 file changed, 26 insertions(+), 13 deletions(-)

diff --git a/scd/ccid-driver.c b/scd/ccid-driver.c
index 6869821d6..eed8e0320 100644
--- a/scd/ccid-driver.c
+++ b/scd/ccid-driver.c
@@ -1787,7 +1787,25 @@ ccid_require_get_status (ccid_driver_t handle)
      detect removal of a card and can detect removal of a reader.
   */
   if (handle->ep_intr >= 0)
-    return 0;
+    {
+      if (handle->id_vendor != VENDOR_SCM)
+        return 0;
+
+      /*
+       * For card reader with interrupt transfer support, ideally,
+       * removal is detected by intr_cb, but some card reader
+       * (e.g. SPR532) has a possible case of missing report to
+       * intr_cb, and another case of valid report to intr_cb.
+       *
+       * For such a reader, the removal should be able to be detected
+       * by PC_to_RDR_GetSlotStatus, too.  Thus, calls to
+       * ccid_slot_status should go on wire even if "on_wire" is not
+       * requested.
+       *
+       */
+      if (handle->transfer == NULL)
+        return 0;
+    }
 
   /* Libusb actually detects the removal of USB device in use.
      However, there is no good API to handle the removal (yet),
@@ -2148,19 +2166,16 @@ bulk_in (ccid_driver_t handle, unsigned char *buffer, size_t length,
       /*
        * Communication failure by device side.
        * Possibly, it was forcibly suspended and resumed.
-       *
-       * For card reader with interrupt transfer support, ideally,
-       * removal is detected by intr_cb, but some card reader
-       * (e.g. SPR532) has a case of missing report to intr_cb.
        */
-      if (handle->ep_intr < 0 || handle->id_vendor == VENDOR_SCM)
+      if (handle->ep_intr < 0)
         {
           DEBUGOUT ("CCID: card inactive/removed\n");
           handle->powered_off = 1;
-#if defined(GNUPG_MAJOR_VERSION)
-          scd_kick_the_loop ();
-#endif
         }
+
+#if defined(GNUPG_MAJOR_VERSION)
+      scd_kick_the_loop ();
+#endif
     }
 
   return rc;
@@ -2427,10 +2442,7 @@ ccid_slot_status (ccid_driver_t handle, int *statusbits, int on_wire)
       /* Setup interrupt transfer at the initial call of slot_status
          with ON_WIRE == 0 */
       if (handle->transfer == NULL)
-        {
-          ccid_setup_intr (handle);
-          ccid_vendor_specific_setup (handle);
-        }
+        ccid_setup_intr (handle);
 
       *statusbits = 0;
       return 0;
@@ -2899,6 +2911,7 @@ ccid_get_atr (ccid_driver_t handle,
       DEBUGOUT_1 ("IFSD has been set to %d\n", tpdu[3]);
     }
 
+  ccid_vendor_specific_setup (handle);
   return 0;
 }