[Pcsclite-muscle] Data races related to SCardCancel

[Maksim Ivanov]

Hello,

It seems that there are still some issues with regard to the blocking
requests cancellation functionality in PC/SC-Lite:

1. Extra SCARD_E_CANCELLED events may be sent by the daemon to the
   client that was previously performing an SCardGetStatusChange call.

   One scenario is that two concurrent SCardCancel calls succeeding
   simultaneously, and another scenario is an SCardCancel call
   succeeding simultaneously with an event sending from the status
   handler thread.

   As a result, the app<->daemon communication will break.

2. Use-after-free possible in SCardCancel.

   This is probably a low-severity issue, as the deallocated memory will
   be accessed only for reading an int, which would be then used for
   deciding whether to fail fast or to send a request to the daemon
   (the latter is expected to fail anyway).


Suggested solutions:

For #1, suggesting to change the SCARD_CANCEL handler to firstly do
   EHUnregisterClientForEvent, and, only if it returns success, then
   send the SCARD_E_CANCELLED event to the client.

For #2, the suggestion is to move the reading of the
   currentContextMap->cancellable flag under the clientMutex lock.

   Also it may be advisable to change the SCardGetAndLockContext
   function interface so that it's more difficult to use it in thread-unsafe
   manner: remove the second parameter "int lock" and make the
   locking behavior the default one, and introduce another function
   that does no locks and returns a boolean instead of an
   SCONTEXTMAP* pointer.


Regards,
Maksim