[Pcsclite-git-commit] [PCSC] 01/01: SCardGetStatusChange(): Fix the fix for a race condition

Ludovic Rousseau rousseau at moszumanska.debian.org
Thu Dec 8 19:16:19 UTC 2016


This is an automated email from the git hooks/post-receive script.

rousseau pushed a commit to branch master
in repository PCSC.

commit 152a53e5151f4f81c572dfeae02d5e0ea8eebccf
Author: Ludovic Rousseau <ludovic.rousseau at free.fr>
Date:   Thu Dec 8 14:48:21 2016 +0100

    SCardGetStatusChange(): Fix the fix for a race condition
    
    The change in 4e2a563c8ed4353ad013de85b71aac12ec599f82 is not correct.
    
    WRITE_BODY() is a macro composed of 2 lines of C so we can't use:
    "if (test)
        WRITE_BODY()"
    
    As is is interpreted as:
    "if (test)
        instruction1;
    instruction2;"
    
    For that to work we need to:
    - use { } around WRITE_BODY()
    or
    - define WRITE_BODY() as a C instruction
    
    Defining WRITE_BODY() as a C instruction (a "do {} while(0)") is the
    best option as it will prevent the same bug to appear again.
    
    Thanks to Florian Kaiser (again) for the patch
    "[Pcsclite-muscle] [PATCH] fix racecondition between winscard server and
    clients"
    http://lists.alioth.debian.org/pipermail/pcsclite-muscle/Week-of-Mon-20161205/000775.html
---
 src/winscard_svc.c | 82 ++++++++++++++++++++++++++++++------------------------
 1 file changed, 45 insertions(+), 37 deletions(-)

diff --git a/src/winscard_svc.c b/src/winscard_svc.c
index 1ddbc5a..20f2976 100644
--- a/src/winscard_svc.c
+++ b/src/winscard_svc.c
@@ -304,15 +304,23 @@ static const char *CommandsText[] = {
 #endif
 
 #define READ_BODY(v) \
-	if (header.size != sizeof(v)) { goto wrong_length; } \
-	ret = MessageReceive(&v, sizeof(v), filedes); \
-	if (ret != SCARD_S_SUCCESS) { Log2(PCSC_LOG_DEBUG, "Client die: %d", filedes); goto exit; }
+	do { \
+		if (header.size != sizeof(v)) \
+			goto wrong_length;  \
+		ret = MessageReceive(&v, sizeof(v), filedes); \
+		if (ret != SCARD_S_SUCCESS) { \
+			Log2(PCSC_LOG_DEBUG, "Client die: %d", filedes); \
+			goto exit; \
+		} \
+	} while (0)
 
 #define WRITE_BODY(v) \
 	WRITE_BODY_WITH_COMMAND(CommandsText[header.command], v)
 #define WRITE_BODY_WITH_COMMAND(command, v) \
-	Log4(PCSC_LOG_DEBUG, "%s rv=0x%X for client %d", command, v.rv, filedes); \
-	ret = MessageSend(&v, sizeof(v), filedes);
+	do { \
+		Log4(PCSC_LOG_DEBUG, "%s rv=0x%X for client %d", command, v.rv, filedes); \
+		ret = MessageSend(&v, sizeof(v), filedes); \
+	} while (0)
 
 static void ContextThread(LPVOID newContext)
 {
@@ -357,7 +365,7 @@ static void ContextThread(LPVOID newContext)
 			{
 				struct version_struct veStr;
 
-				READ_BODY(veStr)
+				READ_BODY(veStr);
 
 				Log3(PCSC_LOG_DEBUG, "Client is protocol version %d:%d",
 					veStr.major, veStr.minor);
@@ -380,7 +388,7 @@ static void ContextThread(LPVOID newContext)
 				veStr.minor = PROTOCOL_VERSION_MINOR;
 
 				/* send back the response */
-				WRITE_BODY(veStr)
+				WRITE_BODY(veStr);
 			}
 			break;
 
@@ -402,7 +410,7 @@ static void ContextThread(LPVOID newContext)
 			{
 				struct wait_reader_state_change waStr;
 
-				READ_BODY(waStr)
+				READ_BODY(waStr);
 
 				/* add the client fd to the list */
 				EHRegisterClientForEvent(filedes);
@@ -417,7 +425,7 @@ static void ContextThread(LPVOID newContext)
 			{
 				struct wait_reader_state_change waStr;
 
-				READ_BODY(waStr)
+				READ_BODY(waStr);
 
 				/* remove the client fd from the list */
 				waStr.rv = EHUnregisterClientForEvent(filedes);
@@ -425,7 +433,7 @@ static void ContextThread(LPVOID newContext)
 				/* send the response only if the client was still in the
 				 * list */
 				if (waStr.rv != SCARD_F_INTERNAL_ERROR)
-					WRITE_BODY(waStr)
+					WRITE_BODY(waStr);
 			}
 			break;
 
@@ -434,7 +442,7 @@ static void ContextThread(LPVOID newContext)
 				struct establish_struct esStr;
 				SCARDCONTEXT hContext;
 
-				READ_BODY(esStr)
+				READ_BODY(esStr);
 
 				hContext = esStr.hContext;
 				esStr.rv = SCardEstablishContext(esStr.dwScope, 0, 0,
@@ -444,7 +452,7 @@ static void ContextThread(LPVOID newContext)
 				if (esStr.rv == SCARD_S_SUCCESS)
 					esStr.rv = MSGAddContext(esStr.hContext, threadContext);
 
-				WRITE_BODY(esStr)
+				WRITE_BODY(esStr);
 			}
 			break;
 
@@ -452,14 +460,14 @@ static void ContextThread(LPVOID newContext)
 			{
 				struct release_struct reStr;
 
-				READ_BODY(reStr)
+				READ_BODY(reStr);
 
 				reStr.rv = SCardReleaseContext(reStr.hContext);
 
 				if (reStr.rv == SCARD_S_SUCCESS)
 					reStr.rv = MSGRemoveContext(reStr.hContext, threadContext);
 
-				WRITE_BODY(reStr)
+				WRITE_BODY(reStr);
 			}
 			break;
 
@@ -469,7 +477,7 @@ static void ContextThread(LPVOID newContext)
 				SCARDHANDLE hCard;
 				DWORD dwActiveProtocol;
 
-				READ_BODY(coStr)
+				READ_BODY(coStr);
 
 				coStr.szReader[sizeof(coStr.szReader)-1] = 0;
 				hCard = coStr.hCard;
@@ -496,7 +504,7 @@ static void ContextThread(LPVOID newContext)
 					coStr.rv = MSGAddHandle(coStr.hContext, coStr.hCard,
 						threadContext);
 
-				WRITE_BODY(coStr)
+				WRITE_BODY(coStr);
 			}
 			break;
 
@@ -505,7 +513,7 @@ static void ContextThread(LPVOID newContext)
 				struct reconnect_struct rcStr;
 				DWORD dwActiveProtocol;
 
-				READ_BODY(rcStr)
+				READ_BODY(rcStr);
 
 				if (MSGCheckHandleAssociation(rcStr.hCard, threadContext))
 					goto exit;
@@ -515,7 +523,7 @@ static void ContextThread(LPVOID newContext)
 					&dwActiveProtocol);
 				rcStr.dwActiveProtocol = dwActiveProtocol;
 
-				WRITE_BODY(rcStr)
+				WRITE_BODY(rcStr);
 			}
 			break;
 
@@ -523,7 +531,7 @@ static void ContextThread(LPVOID newContext)
 			{
 				struct disconnect_struct diStr;
 
-				READ_BODY(diStr)
+				READ_BODY(diStr);
 
 				if (MSGCheckHandleAssociation(diStr.hCard, threadContext))
 					goto exit;
@@ -533,7 +541,7 @@ static void ContextThread(LPVOID newContext)
 				if (SCARD_S_SUCCESS == diStr.rv)
 					diStr.rv = MSGRemoveHandle(diStr.hCard, threadContext);
 
-				WRITE_BODY(diStr)
+				WRITE_BODY(diStr);
 			}
 			break;
 
@@ -541,14 +549,14 @@ static void ContextThread(LPVOID newContext)
 			{
 				struct begin_struct beStr;
 
-				READ_BODY(beStr)
+				READ_BODY(beStr);
 
 				if (MSGCheckHandleAssociation(beStr.hCard, threadContext))
 					goto exit;
 
 				beStr.rv = SCardBeginTransaction(beStr.hCard);
 
-				WRITE_BODY(beStr)
+				WRITE_BODY(beStr);
 			}
 			break;
 
@@ -556,7 +564,7 @@ static void ContextThread(LPVOID newContext)
 			{
 				struct end_struct enStr;
 
-				READ_BODY(enStr)
+				READ_BODY(enStr);
 
 				if (MSGCheckHandleAssociation(enStr.hCard, threadContext))
 					goto exit;
@@ -564,7 +572,7 @@ static void ContextThread(LPVOID newContext)
 				enStr.rv = SCardEndTransaction(enStr.hCard,
 					enStr.dwDisposition);
 
-				WRITE_BODY(enStr)
+				WRITE_BODY(enStr);
 			}
 			break;
 
@@ -572,7 +580,7 @@ static void ContextThread(LPVOID newContext)
 			{
 				struct cancel_struct caStr;
 				SCONTEXT * psTargetContext = NULL;
-				READ_BODY(caStr)
+				READ_BODY(caStr);
 
 				/* find the client */
 				(void)pthread_mutex_lock(&contextsList_lock);
@@ -591,7 +599,7 @@ static void ContextThread(LPVOID newContext)
 				else
 					caStr.rv = SCARD_E_INVALID_HANDLE;
 
-				WRITE_BODY(caStr)
+				WRITE_BODY(caStr);
 			}
 			break;
 
@@ -599,7 +607,7 @@ static void ContextThread(LPVOID newContext)
 			{
 				struct status_struct stStr;
 
-				READ_BODY(stStr)
+				READ_BODY(stStr);
 
 				if (MSGCheckHandleAssociation(stStr.hCard, threadContext))
 					goto exit;
@@ -608,7 +616,7 @@ static void ContextThread(LPVOID newContext)
 				stStr.rv = SCardStatus(stStr.hCard, NULL, NULL, NULL,
 					NULL, 0, NULL);
 
-				WRITE_BODY(stStr)
+				WRITE_BODY(stStr);
 			}
 			break;
 
@@ -621,7 +629,7 @@ static void ContextThread(LPVOID newContext)
 				SCARD_IO_REQUEST ioRecvPci;
 				DWORD cbRecvLength;
 
-				READ_BODY(trStr)
+				READ_BODY(trStr);
 
 				if (MSGCheckHandleAssociation(trStr.hCard, threadContext))
 					goto exit;
@@ -661,7 +669,7 @@ static void ContextThread(LPVOID newContext)
 				trStr.ioRecvPciLength = ioRecvPci.cbPciLength;
 				trStr.pcbRecvLength = cbRecvLength;
 
-				WRITE_BODY(trStr)
+				WRITE_BODY(trStr);
 
 				/* write received buffer */
 				if (SCARD_S_SUCCESS == trStr.rv)
@@ -676,7 +684,7 @@ static void ContextThread(LPVOID newContext)
 				unsigned char pbRecvBuffer[MAX_BUFFER_SIZE_EXTENDED];
 				DWORD dwBytesReturned;
 
-				READ_BODY(ctStr)
+				READ_BODY(ctStr);
 
 				if (MSGCheckHandleAssociation(ctStr.hCard, threadContext))
 					goto exit;
@@ -705,7 +713,7 @@ static void ContextThread(LPVOID newContext)
 
 				ctStr.dwBytesReturned = dwBytesReturned;
 
-				WRITE_BODY(ctStr)
+				WRITE_BODY(ctStr);
 
 				/* write received buffer */
 				if (SCARD_S_SUCCESS == ctStr.rv)
@@ -718,7 +726,7 @@ static void ContextThread(LPVOID newContext)
 				struct getset_struct gsStr;
 				DWORD cbAttrLen;
 
-				READ_BODY(gsStr)
+				READ_BODY(gsStr);
 
 				if (MSGCheckHandleAssociation(gsStr.hCard, threadContext))
 					goto exit;
@@ -734,7 +742,7 @@ static void ContextThread(LPVOID newContext)
 
 				gsStr.cbAttrLen = cbAttrLen;
 
-				WRITE_BODY(gsStr)
+				WRITE_BODY(gsStr);
 			}
 			break;
 
@@ -742,7 +750,7 @@ static void ContextThread(LPVOID newContext)
 			{
 				struct getset_struct gsStr;
 
-				READ_BODY(gsStr)
+				READ_BODY(gsStr);
 
 				if (MSGCheckHandleAssociation(gsStr.hCard, threadContext))
 					goto exit;
@@ -754,7 +762,7 @@ static void ContextThread(LPVOID newContext)
 				gsStr.rv = SCardSetAttrib(gsStr.hCard, gsStr.dwAttrId,
 					gsStr.pbAttr, gsStr.cbAttrLen);
 
-				WRITE_BODY(gsStr)
+				WRITE_BODY(gsStr);
 			}
 			break;
 
@@ -791,7 +799,7 @@ LONG MSGSignalClient(uint32_t filedes, LONG rv)
 	Log2(PCSC_LOG_DEBUG, "Signal client: %d", filedes);
 
 	waStr.rv = rv;
-	WRITE_BODY_WITH_COMMAND("SIGNAL", waStr)
+	WRITE_BODY_WITH_COMMAND("SIGNAL", waStr);
 
 	return ret;
 } /* MSGSignalClient */

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pcsclite/PCSC.git



More information about the Pcsclite-cvs-commit mailing list