Skip to content

Commit

Permalink
Access readerStates[] in a thread safe way
Browse files Browse the repository at this point in the history 
protect accesses to readerStates[] using a mutex so that 2 threads cannot
interfere for example by calling SCardGetStatusChange() and
SCardStatus() at the same time.

The ThreadSanitizer trace was:
==================
WARNING: ThreadSanitizer: data race (pid=13125)
  Write of size 8 at 0x7fd3afc2b040 by main thread (mutexes: write M0):
    #0 read <null> (pcsc_demo+0x5d8a4) (BuildId: 1ce759cdf5448d9e6401e93f7f56d4c7caa8ede7)
    #1 read /usr/include/x86_64-linux-gnu/bits/unistd.h:38:10 (libpcsclite.so.1+0xab3c) (BuildId: 53195794f51c4d60ff5688d377d82ae2e264554e)
    #2 MessageReceive src/winscard_msg.c:491:17 (libpcsclite.so.1+0xab3c)
    #3 getReaderStates src/winscard_clnt.c:3552:7 (libpcsclite.so.1+0x4109) (BuildId: 53195794f51c4d60ff5688d377d82ae2e264554e)
    #4 SCardStatus src/winscard_clnt.c:1441:7 (libpcsclite.so.1+0x4109)
    #5 main /tmp/x/pcsc-lite-2.0.0/pcsc_demo.c:108:7 (pcsc_demo+0xd1bc4) (BuildId: 1ce759cdf5448d9e6401e93f7f56d4c7caa8ede7)

  Previous write of size 8 at 0x7fd3afc2b040 by thread T1 (mutexes: write M1):
    #0 read <null> (pcsc_demo+0x5d8a4) (BuildId: 1ce759cdf5448d9e6401e93f7f56d4c7caa8ede7)
    #1 read /usr/include/x86_64-linux-gnu/bits/unistd.h:38:10 (libpcsclite.so.1+0xab3c) (BuildId: 53195794f51c4d60ff5688d377d82ae2e264554e)
    #2 MessageReceive src/winscard_msg.c:491:17 (libpcsclite.so.1+0xab3c)
    #3 getReaderStatesAndRegisterForEvents src/winscard_clnt.c:3571:7 (libpcsclite.so.1+0x47b3) (BuildId: 53195794f51c4d60ff5688d377d82ae2e264554e)
    #4 SCardGetStatusChange src/winscard_clnt.c:1747:7 (libpcsclite.so.1+0x47b3)
    #5 do_statuschange /tmp/x/pcsc-lite-2.0.0/pcsc_demo.c:35:10 (pcsc_demo+0xd1e6d) (BuildId: 1ce759cdf5448d9e6401e93f7f56d4c7caa8ede7)
    #6 statuschange_thread /tmp/x/pcsc-lite-2.0.0/pcsc_demo.c:53:2 (pcsc_demo+0xd19fd) (BuildId: 1ce759cdf5448d9e6401e93f7f56d4c7caa8ede7)

  Location is global 'readerStates' of size 2944 at 0x7fd3afc2b040 (libpcsclite.so.1+0xf040)

  Mutex M0 (0x7b3000006010) created at:
    #0 pthread_mutex_init <null> (pcsc_demo+0x5431f) (BuildId: 1ce759cdf5448d9e6401e93f7f56d4c7caa8ede7)
    #1 SCardAddContext src/winscard_clnt.c:3259:8 (libpcsclite.so.1+0x6adc) (BuildId: 53195794f51c4d60ff5688d377d82ae2e264554e)
    #2 SCardEstablishContextTH src/winscard_clnt.c:659:7 (libpcsclite.so.1+0x6adc)
    #3 SCardEstablishContext src/winscard_clnt.c:483:7 (libpcsclite.so.1+0x6adc)
    #4 main /tmp/x/pcsc-lite-2.0.0/pcsc_demo.c:86:7 (pcsc_demo+0xd1aca) (BuildId: 1ce759cdf5448d9e6401e93f7f56d4c7caa8ede7)

  Mutex M1 (0x7b3000000010) created at:
    #0 pthread_mutex_init <null> (pcsc_demo+0x5431f) (BuildId: 1ce759cdf5448d9e6401e93f7f56d4c7caa8ede7)
    #1 SCardAddContext src/winscard_clnt.c:3259:8 (libpcsclite.so.1+0x6adc) (BuildId: 53195794f51c4d60ff5688d377d82ae2e264554e)
    #2 SCardEstablishContextTH src/winscard_clnt.c:659:7 (libpcsclite.so.1+0x6adc)
    #3 SCardEstablishContext src/winscard_clnt.c:483:7 (libpcsclite.so.1+0x6adc)
    #4 do_statuschange /tmp/x/pcsc-lite-2.0.0/pcsc_demo.c:12:7 (pcsc_demo+0xd1d84) (BuildId: 1ce759cdf5448d9e6401e93f7f56d4c7caa8ede7)
    #5 statuschange_thread /tmp/x/pcsc-lite-2.0.0/pcsc_demo.c:53:2 (pcsc_demo+0xd19fd) (BuildId: 1ce759cdf5448d9e6401e93f7f56d4c7caa8ede7)

  Thread T1 (tid=13127, running) created by main thread at:
    #0 pthread_create <null> (pcsc_demo+0x52b4d) (BuildId: 1ce759cdf5448d9e6401e93f7f56d4c7caa8ede7)
    #1 start_thread /tmp/x/pcsc-lite-2.0.0/pcsc_demo.c:61:6 (pcsc_demo+0xd1964) (BuildId: 1ce759cdf5448d9e6401e93f7f56d4c7caa8ede7)
    #2 main /tmp/x/pcsc-lite-2.0.0/pcsc_demo.c:84:8 (pcsc_demo+0xd1ab1) (BuildId: 1ce759cdf5448d9e6401e93f7f56d4c7caa8ede7)

SUMMARY: ThreadSanitizer: data race (/tmp/x/pcsc-lite-2.0.0/pcsc_demo+0x5d8a4) (BuildId: 1ce759cdf5448d9e6401e93f7f56d4c7caa8ede7) in __interceptor_read
==================

Thanks to Stefan Ehmann for the bug report.
  • Loading branch information
@LudovicRousseau
LudovicRousseau committed Nov 19, 2023
1 parent 04a4698 commit e12d9dd
Showing 1 changed file with 26 additions and 0 deletions.
26 changes: 26 additions & 0 deletions src/winscard_clnt.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,7 @@ static pthread_mutex_t clientMutex = PTHREAD_MUTEX_INITIALIZER;
* Area used to read status information about the readers.
*/
static READER_STATE readerStates[PCSCLITE_MAX_READERS_CONTEXTS];
static pthread_mutex_t readerStatesMutex = PTHREAD_MUTEX_INITIALIZER;

/** Protocol Control Information for T=0 */
PCSC_API const SCARD_IO_REQUEST g_rgSCardT0Pci = { SCARD_PROTOCOL_T0, sizeof(SCARD_IO_REQUEST) };
Expand Down Expand Up @@ -1437,6 +1438,9 @@ LONG SCardStatus(SCARDHANDLE hCard, LPSTR szReaderName,
if (rv == -1)
return SCARD_E_INVALID_HANDLE;

/* lock access to readerStates[] */
(void)pthread_mutex_lock(&readerStatesMutex);

/* synchronize reader states with daemon */
rv = getReaderStates(currentContextMap);
if (rv != SCARD_S_SUCCESS)
Expand Down Expand Up @@ -1480,6 +1484,7 @@ LONG SCardStatus(SCARDHANDLE hCard, LPSTR szReaderName,
if (sharing_shall_block && (SCARD_E_SHARING_VIOLATION == rv))
{
(void)pthread_mutex_unlock(&currentContextMap->mMutex);
(void)pthread_mutex_unlock(&readerStatesMutex);
(void)SYS_USleep(PCSCLITE_LOCK_POLL_RATE);
goto retry;
}
Expand Down Expand Up @@ -1562,6 +1567,7 @@ LONG SCardStatus(SCARDHANDLE hCard, LPSTR szReaderName,

end:
(void)pthread_mutex_unlock(&currentContextMap->mMutex);
(void)pthread_mutex_unlock(&readerStatesMutex);

PROFILE_END(rv)

Expand Down Expand Up @@ -1743,10 +1749,17 @@ LONG SCardGetStatusChange(SCARDCONTEXT hContext, DWORD dwTimeout,
goto error;
}

/* lock access to readerStates[] */
(void)pthread_mutex_lock(&readerStatesMutex);

/* synchronize reader states with daemon */
rv = getReaderStatesAndRegisterForEvents(currentContextMap);

if (rv != SCARD_S_SUCCESS)
{
(void)pthread_mutex_unlock(&readerStatesMutex);
goto end;
}

/* check all the readers are already known */
for (j=0; j<cReaders; j++)
Expand All @@ -1768,10 +1781,12 @@ LONG SCardGetStatusChange(SCARDCONTEXT hContext, DWORD dwTimeout,
if (strcasecmp(readerName, "\\\\?PnP?\\Notification") != 0)
{
rv = SCARD_E_UNKNOWN_READER;
(void)pthread_mutex_unlock(&readerStatesMutex);
goto end;
}
}
}
(void)pthread_mutex_unlock(&readerStatesMutex);

/* Clear the event state for all readers */
for (j = 0; j < cReaders; j++)
Expand Down Expand Up @@ -1804,6 +1819,9 @@ LONG SCardGetStatusChange(SCARDCONTEXT hContext, DWORD dwTimeout,
const char *readerName;
int i;

/* lock access to readerStates[] */
(void)pthread_mutex_lock(&readerStatesMutex);

/* Looks for correct readernames */
readerName = currReader->szReader;
for (i = 0; i < PCSCLITE_MAX_READERS_CONTEXTS; i++)
Expand Down Expand Up @@ -2049,6 +2067,8 @@ LONG SCardGetStatusChange(SCARDCONTEXT hContext, DWORD dwTimeout,
dwBreakFlag = 1;
}
} /* End of SCARD_STATE_UNKNOWN */

(void)pthread_mutex_unlock(&readerStatesMutex);
} /* End of SCARD_STATE_IGNORE */

/* Counter and resetter */
Expand Down Expand Up @@ -2105,7 +2125,9 @@ LONG SCardGetStatusChange(SCARDCONTEXT hContext, DWORD dwTimeout,
}

/* synchronize reader states with daemon */
(void)pthread_mutex_lock(&readerStatesMutex);
rv = getReaderStatesAndRegisterForEvents(currentContextMap);
(void)pthread_mutex_unlock(&readerStatesMutex);
if (rv != SCARD_S_SUCCESS)
goto end;

Expand Down Expand Up @@ -2868,6 +2890,9 @@ LONG SCardListReaders(SCARDCONTEXT hContext, /*@unused@*/ LPCSTR mszGroups,
return SCARD_E_INVALID_HANDLE;
}

/* lock access to readerStates[] */
(void)pthread_mutex_lock(&readerStatesMutex);

/* synchronize reader states with daemon */
rv = getReaderStates(currentContextMap);
if (rv != SCARD_S_SUCCESS)
Expand Down Expand Up @@ -2935,6 +2960,7 @@ LONG SCardListReaders(SCARDCONTEXT hContext, /*@unused@*/ LPCSTR mszGroups,
*pcchReaders = dwReadersLen;

(void)pthread_mutex_unlock(&currentContextMap->mMutex);
(void)pthread_mutex_unlock(&readerStatesMutex);

PROFILE_END(rv)
API_TRACE_OUT("%d", *pcchReaders)
Expand Down

0 comments on commit e12d9dd

Please sign in to comment.