From 36bc9446b40fa3c6ac12312b934f4d7131659087 Mon Sep 17 00:00:00 2001 From: Ludovic Rousseau Date: Wed, 5 Aug 2020 17:59:41 +0200 Subject: [PATCH 01/13] Do not (possibly) lock a reader if allocating hCard fails In case of SCardConnect() the reader may be locked in SCARD_SHARE_EXCLUSIVE mode if internal SCardConnect() works but MSGAddHandle() fails because the list of handle is full. You need to start pcscd with "--max-card-handle-per-reader n" with n > 200 or the 200 limit (default value) will be hit in internal SCardConnect() and MSGAddHandle() would not be called. Thanks to Maksim Ivanov for the bug report "[Pcsclite-muscle] SCardConnect behavior with invalid contexts" http://lists.infradead.org/pipermail/pcsclite-muscle/2020-July/001095.html " Hello, I believe that there's a potential problem with the SCardConnect implementation that it doesn't check the received SCARDCONTEXT *before* executing the command. This might result in an unexpected state, where the SCardConnect() caller receives an error code meanwhile the connection to the card is actually established (which, for example, might be an exclusive connection that prevents anyone else from connecting to the card). In detail, the ContextThread() function in winscard_svc.c, when receiving the SCARD_CONNECT command, calls first SCardConnect() from winscard.c, and then MSGAddHandle(). The former ignores SCARDCONTEXT and, if possible, establishes a connection to the card. The latter does check the SCARDCONTEXT value, but this happens after the connection is already established, and its error is just returned to the caller (without closing the just-opened connection). Would it make sense to add a check of SCARDCONTEXT before calling SCardConnect(), and/or to call SCardDisconnect() if MSGAddHandle() fails? Regards, Maksim " --- src/winscard_svc.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/winscard_svc.c b/src/winscard_svc.c index cdeac33..c0df008 100644 --- a/src/winscard_svc.c +++ b/src/winscard_svc.c @@ -507,9 +507,15 @@ static void * ContextThread(LPVOID newContext) coStr.dwActiveProtocol = dwActiveProtocol; if (coStr.rv == SCARD_S_SUCCESS) + { coStr.rv = MSGAddHandle(coStr.hContext, coStr.hCard, threadContext); + /* if storing the hCard fails we disconnect */ + if (coStr.rv != SCARD_S_SUCCESS) + SCardDisconnect(coStr.hCard, SCARD_LEAVE_CARD); + } + WRITE_BODY(coStr); } break; @@ -963,7 +969,7 @@ static LONG MSGAddHandle(SCARDCONTEXT hContext, SCARDHANDLE hCard, if (listLength >= contextMaxCardHandles) { Log4(PCSC_LOG_DEBUG, - "Too many card handles for thread context @%p: %d (max is %d)" + "Too many card handles for thread context @%p: %d (max is %d). " "Restart pcscd with --max-card-handle-per-thread value", threadContext, listLength, contextMaxCardHandles); retval = SCARD_E_NO_MEMORY; -- 1.8.3.1