<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">2017-01-18 21:47 GMT+01:00 Maksim Ivanov <span dir="ltr"><<a target="_blank" href="mailto:emaxx@google.com">emaxx@google.com</a>></span>:<br><blockquote style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex" class="gmail_quote">Hello,<br></blockquote><div><br></div><div>Hi,<br> <br></div><blockquote style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex" class="gmail_quote">
<br>
It seems that there are still some issues with regard to the blocking<br>
requests cancellation functionality in PC/SC-Lite:<br>
<br>
1. Extra SCARD_E_CANCELLED events may be sent by the daemon to the<br>
   client that was previously performing an SCardGetStatusChange call.<br>
<br>
   One scenario is that two concurrent SCardCancel calls succeeding<br>
   simultaneously, and another scenario is an SCardCancel call<br>
   succeeding simultaneously with an event sending from the status<br>
   handler thread.<br>
<br>
   As a result, the app<->daemon communication will break.<br>
<br>
2. Use-after-free possible in SCardCancel.<br>
<br>
   This is probably a low-severity issue, as the deallocated memory will<br>
   be accessed only for reading an int, which would be then used for<br>
   deciding whether to fail fast or to send a request to the daemon<br>
   (the latter is expected to fail anyway).<br>
<br>
<br>
Suggested solutions:<br>
<br>
For #1, suggesting to change the SCARD_CANCEL handler to firstly do<br>
   EHUnregisterClientForEvent, and, only if it returns success, then<br>
   send the SCARD_E_CANCELLED event to the client.<br></blockquote><div><br></div><div>Fixed in <a href="https://github.com/LudovicRousseau/PCSC/commit/b662aa1ae24267150d816a08f757b1c1539411ca">https://github.com/LudovicRousseau/PCSC/commit/b662aa1ae24267150d816a08f757b1c1539411ca</a><br><br> </div><blockquote style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex" class="gmail_quote">
<br>
For #2, the suggestion is to move the reading of the<br>
   currentContextMap->cancellable flag under the clientMutex lock.<br></blockquote><div><br><div>Fixed in  <a href="https://github.com/LudovicRousseau/PCSC/commit/9a21c82330fc4b73ac00faf2c5cf948da5f8ab07">https://github.com/LudovicRousseau/PCSC/commit/9a21c82330fc4b73ac00faf2c5cf948da5f8ab07</a><br clear="all"><br></div></div><blockquote style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex" class="gmail_quote">
   Also it may be advisable to change the SCardGetAndLockContext<br>
   function interface so that it's more difficult to use it in thread-unsafe<br>
   manner: remove the second parameter "int lock" and make the<br>
   locking behavior the default one, and introduce another function<br>
   that does no locks and returns a boolean instead of an<br>
   SCONTEXTMAP* pointer.<br>
<br></blockquote><div><br></div><div>Fixed in <a href="https://github.com/LudovicRousseau/PCSC/commit/8193d6fad84539af03a329e0f6911125f96497d0">https://github.com/LudovicRousseau/PCSC/commit/8193d6fad84539af03a329e0f6911125f96497d0</a><br></div><br></div><div class="gmail_quote">Thanks a lot Maksim for your bug reports and fixes. I appreciate.<br></div><div class="gmail_quote">Regards,<br></div><br>-- <br><div class="gmail_signature"> Dr. Ludovic Rousseau</div>
</div></div>