[Pcsclite-git-commit] [PCSC] 02/03: Threading: lock the PC/SC context in a safe way

[Ludovic Rousseau]

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

rousseau pushed a commit to branch master
in repository PCSC.

commit 9681246742b812ba1b8acbe8892d8aa755215059
Author: Ludovic Rousseau <ludovic.rousseau at gmail.com>
Date:   Tue Jun 23 09:21:25 2015 +0200

    Threading: lock the PC/SC context in a safe way
    
    Functions working on hContext now acquire the context lock while the
    global thread lock is held.
    
    This will avoid the context to be released between the context retrieval
    and context lock. The code was ugly with a new call to
    SCardGetContext()to check that the context was still valid.
    
    Thanks to Luc Mazardo for the bug report.
    http://lists.alioth.debian.org/pipermail/pcsclite-muscle/Week-of-Mon-20150608/000395.html
---
 src/winscard_clnt.c | 81 ++++++++++-------------------------------------------
 1 file changed, 15 insertions(+), 66 deletions(-)

diff --git a/src/winscard_clnt.c b/src/winscard_clnt.c
index 97f6b7f..589d14f 100644
--- a/src/winscard_clnt.c
+++ b/src/winscard_clnt.c
@@ -368,7 +368,7 @@ PCSC_API const SCARD_IO_REQUEST g_rgSCardRawPci = { SCARD_PROTOCOL_RAW, sizeof(S
 
 
 static LONG SCardAddContext(SCARDCONTEXT, DWORD);
-static SCONTEXTMAP * SCardGetContext(SCARDCONTEXT);
+static SCONTEXTMAP * SCardGetAndLockContext(SCARDCONTEXT, int);
 static SCONTEXTMAP * SCardGetContextTH(SCARDCONTEXT);
 static LONG SCardRemoveContext(SCARDCONTEXT);
 static LONG SCardCleanContext(SCONTEXTMAP *);
@@ -661,26 +661,13 @@ LONG SCardReleaseContext(SCARDCONTEXT hContext)
 	 * Make sure this context has been opened
 	 * and get currentContextMap
 	 */
-	currentContextMap = SCardGetContext(hContext);
+	currentContextMap = SCardGetAndLockContext(hContext, TRUE);
 	if (NULL == currentContextMap)
 	{
 		rv = SCARD_E_INVALID_HANDLE;
 		goto error;
 	}
 
-	(void)pthread_mutex_lock(&currentContextMap->mMutex);
-
-	/* check the context is still opened */
-	currentContextMap = SCardGetContext(hContext);
-	if (NULL == currentContextMap)
-		/* the hContext context is now invalid
-		 * -> another thread may have called SCardReleaseContext
-		 *    so the mMutex has been unlocked */
-	{
-		rv = SCARD_E_INVALID_HANDLE;
-		goto error;
-	}
-
 	scReleaseStruct.hContext = hContext;
 	scReleaseStruct.rv = SCARD_S_SUCCESS;
 
@@ -805,18 +792,8 @@ LONG SCardConnect(SCARDCONTEXT hContext, LPCSTR szReader,
 	/*
 	 * Make sure this context has been opened
 	 */
-	currentContextMap = SCardGetContext(hContext);
-	if (NULL == currentContextMap)
-		return SCARD_E_INVALID_HANDLE;
-
-	(void)pthread_mutex_lock(&currentContextMap->mMutex);
-
-	/* check the context is still opened */
-	currentContextMap = SCardGetContext(hContext);
+	currentContextMap = SCardGetAndLockContext(hContext, TRUE);
 	if (NULL == currentContextMap)
-		/* the hContext context is now invalid
-		 * -> another thread may have called SCardReleaseContext
-		 *    so the mMutex has been unlocked */
 		return SCARD_E_INVALID_HANDLE;
 
 	strncpy(scConnectStruct.szReader, szReader, sizeof scConnectStruct.szReader);
@@ -1798,26 +1775,13 @@ LONG SCardGetStatusChange(SCARDCONTEXT hContext, DWORD dwTimeout,
 	/*
 	 * Make sure this context has been opened
 	 */
-	currentContextMap = SCardGetContext(hContext);
+	currentContextMap = SCardGetAndLockContext(hContext, TRUE);
 	if (NULL == currentContextMap)
 	{
 		rv = SCARD_E_INVALID_HANDLE;
 		goto error;
 	}
 
-	(void)pthread_mutex_lock(&currentContextMap->mMutex);
-
-	/* check the context is still opened */
-	currentContextMap = SCardGetContext(hContext);
-	if (NULL == currentContextMap)
-		/* the hContext context is now invalid
-		 * -> another thread may have called SCardReleaseContext
-		 *    so the mMutex has been unlocked */
-	{
-		rv = SCARD_E_INVALID_HANDLE;
-		goto error;
-	}
-
 	/* synchronize reader states with daemon */
 	rv = getReaderStates(currentContextMap);
 	if (rv != SCARD_S_SUCCESS)
@@ -2954,23 +2918,13 @@ LONG SCardListReaders(SCARDCONTEXT hContext, /*@unused@*/ LPCSTR mszGroups,
 	/*
 	 * Make sure this context has been opened
 	 */
-	currentContextMap = SCardGetContext(hContext);
+	currentContextMap = SCardGetAndLockContext(hContext, TRUE);
 	if (NULL == currentContextMap)
 	{
 		PROFILE_END(SCARD_E_INVALID_HANDLE)
 		return SCARD_E_INVALID_HANDLE;
 	}
 
-	(void)pthread_mutex_lock(&currentContextMap->mMutex);
-
-	/* check the context is still opened */
-	currentContextMap = SCardGetContext(hContext);
-	if (NULL == currentContextMap)
-		/* the hContext context is now invalid
-		 * -> another thread may have called SCardReleaseContext
-		 * -> so the mMutex has been unlocked */
-		return SCARD_E_INVALID_HANDLE;
-
 	/* synchronize reader states with daemon */
 	rv = getReaderStates(currentContextMap);
 	if (rv != SCARD_S_SUCCESS)
@@ -3068,7 +3022,7 @@ LONG SCardFreeMemory(SCARDCONTEXT hContext, LPCVOID pvMem)
 	/*
 	 * Make sure this context has been opened
 	 */
-	currentContextMap = SCardGetContext(hContext);
+	currentContextMap = SCardGetAndLockContext(hContext, FALSE);
 	if (NULL == currentContextMap)
 		return SCARD_E_INVALID_HANDLE;
 
@@ -3146,20 +3100,10 @@ LONG SCardListReaderGroups(SCARDCONTEXT hContext, LPSTR mszGroups,
 	/*
 	 * Make sure this context has been opened
 	 */
-	currentContextMap = SCardGetContext(hContext);
+	currentContextMap = SCardGetAndLockContext(hContext, TRUE);
 	if (NULL == currentContextMap)
 		return SCARD_E_INVALID_HANDLE;
 
-	(void)pthread_mutex_lock(&currentContextMap->mMutex);
-
-	/* check the context is still opened */
-	currentContextMap = SCardGetContext(hContext);
-	if (NULL == currentContextMap)
-		/* the hContext context is now invalid
-		 * -> another thread may have called SCardReleaseContext
-		 * -> so the mMutex has been unlocked */
-		return SCARD_E_INVALID_HANDLE;
-
 	if (SCARD_AUTOALLOCATE == *pcchGroups)
 	{
 		if (NULL == mszGroups)
@@ -3241,7 +3185,7 @@ LONG SCardCancel(SCARDCONTEXT hContext)
 	/*
 	 * Make sure this context has been opened
 	 */
-	currentContextMap = SCardGetContext(hContext);
+	currentContextMap = SCardGetAndLockContext(hContext, FALSE);
 	if (NULL == currentContextMap)
 	{
 		rv = SCARD_E_INVALID_HANDLE;
@@ -3325,7 +3269,7 @@ LONG SCardIsValidContext(SCARDCONTEXT hContext)
 	/*
 	 * Make sure this context has been opened
 	 */
-	currentContextMap = SCardGetContext(hContext);
+	currentContextMap = SCardGetAndLockContext(hContext, FALSE);
 	if (currentContextMap == NULL)
 		rv = SCARD_E_INVALID_HANDLE;
 
@@ -3415,12 +3359,17 @@ error:
  * @return Index corresponding to the Application Context or -1 if it is
  * not found.
  */
-static SCONTEXTMAP * SCardGetContext(SCARDCONTEXT hContext)
+static SCONTEXTMAP * SCardGetAndLockContext(SCARDCONTEXT hContext, int lock)
 {
 	SCONTEXTMAP * currentContextMap;
 
 	(void)SCardLockThread();
 	currentContextMap = SCardGetContextTH(hContext);
+
+	/* lock the context (if available) */
+	if (lock && NULL != currentContextMap)
+		(void)pthread_mutex_lock(&currentContextMap->mMutex);
+
 	(void)SCardUnlockThread();
 
 	return currentContextMap;

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