pcsc-lite/0001-Do-not-possibly-lock-a-reader-if-allocating-hCard-fa.patch

79 lines
2.9 KiB
Diff
Raw Normal View History

From 36bc9446b40fa3c6ac12312b934f4d7131659087 Mon Sep 17 00:00:00 2001
From: Ludovic Rousseau <ludovic.rousseau@free.fr>
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