Hello Martin,
I think I fixed the pattern A crash in revision 6388. Can you try
again with an up-to-date subversion version of pcsc-lite?
I also got the patern B endless loop. I suspect a bug in libudev so
the pcscd do not receive the device remove event. But maybe it is a
bug in pcscd. I don't know how to reproduce the bug.
Bye
2012/5/28 Ludovic Rousseau <[email protected]>:
> 2012/5/27 Martin Vogt <[email protected]>:
>> On Sun, May 27, 2012 at 3:50 PM, Ludovic Rousseau
>> <[email protected]> wrote:
>
>>> I have not yet fixed this bug.
>>> And yes. It is a bit more complex to fix. I don't know yet how I will fix
>>> it.
>>
>> I looked at the code and added a few printfs.
>> The sequence seems to be:
>> HOTPLUG UDEV calls RFRemoveReader -s
>> [....]
>> 00000010 readerfactory.c:1009:RFUnInitializeReader() Attempting
>> shutdown of Aladdin eToken PRO USB 72K Java [Main Interface] 00 00.
>> 00000013 ifdhandler.c:244:IFDHCloseChannel()
>> usb:0529/0620:libudev:0:/dev/bus/usb/002/018 (lun: 0)
>> 00000021 ccid_usb.c:649:WriteUSB() write failed (2/18): -4 No such device
>> RFUnInitializeReader-1
>> 00000026 readerfactory.c:880:RFUnloadReader() Unloading reader driver.
>>
>> In between this call starts a new ContexThread:
>>
>> 00000105 winscard_svc.c:315:ContextThread() Received command:
>> DISCONNECT from client 9
>>
>> HOTPLU UDEV ends call RFRemoveReader -e
>>
>> Then I have:
>>
>> 00073910 winscard.c:796:SCardDisconnect() Lock released
>> 00000010 winscard.c:820:SCardDisconnect() Active Contexts: 0
>> 00000006 winscard.c:821:SCardDisconnect() dwDisposition: 2
>> Segmentation fault
>>
>> Maybe the hotplug thread can deliver Add/Remove over the SVCServiceRunLoop?
>> (As far as I understand it, the hotplug mechanism works independently,
>> and this may be the problem?)
>
> You can considerably reduce the time before the crash by adding a
> sleep(1) in ifdwrapper.c at line 269, just before the call to
> IFDStatusICC();
>
> The problem is that the reader is removed while another function using
> the reader is running and after this another function has checked the
> reader is present.
>
> It is a classic TOCTTOU bug [1].
>
> Bye
>
> [1] http://en.wikipedia.org/wiki/Time_of_check_to_time_of_use
--
Dr. Ludovic Rousseau
Index: src/winscard.c
===================================================================
--- src/winscard.c (révision 6387)
+++ src/winscard.c (copie de travail)
@@ -487,6 +487,8 @@ LONG SCardConnect(/*@unused@*/ SCARDCONT
rContext->readerState->readerSharing = rContext->contexts;
exit:
+ UNREF_READER(rContext)
+
PROFILE_END
return rv;
@@ -789,6 +791,8 @@ LONG SCardReconnect(SCARDHANDLE hCard, D
rv = SCARD_S_SUCCESS;
exit:
+ UNREF_READER(rContext)
+
return rv;
}
@@ -1037,6 +1041,8 @@ LONG SCardDisconnect(SCARDHANDLE hCard,
rv = SCARD_S_SUCCESS;
exit:
+ UNREF_READER(rContext)
+
return rv;
}
@@ -1077,6 +1083,8 @@ LONG SCardBeginTransaction(SCARDHANDLE h
Log2(PCSC_LOG_DEBUG, "Status: 0x%08lX", rv);
exit:
+ UNREF_READER(rContext)
+
return rv;
}
@@ -1211,6 +1219,8 @@ LONG SCardEndTransaction(SCARDHANDLE hCa
Log2(PCSC_LOG_DEBUG, "Status: 0x%08lX", rv);
exit:
+ UNREF_READER(rContext)
+
return rv;
}
@@ -1272,6 +1282,8 @@ LONG SCardStatus(SCARDHANDLE hCard, LPST
goto exit;
exit:
+ UNREF_READER(rContext)
+
return rv;
}
@@ -1329,6 +1341,8 @@ LONG SCardControl(SCARDHANDLE hCard, DWO
rv = SCARD_E_UNSUPPORTED_FEATURE;
exit:
+ UNREF_READER(rContext)
+
return rv;
}
@@ -1403,6 +1417,8 @@ LONG SCardGetAttrib(SCARDHANDLE hCard, D
}
exit:
+ UNREF_READER(rContext)
+
return rv;
}
@@ -1451,6 +1467,8 @@ LONG SCardSetAttrib(SCARDHANDLE hCard, D
rv = SCARD_E_NOT_TRANSACTED;
exit:
+ UNREF_READER(rContext)
+
return rv;
}
@@ -1605,6 +1623,8 @@ LONG SCardTransmit(SCARDHANDLE hCard, co
*pcbRecvLength = dwRxLength;
exit:
+ UNREF_READER(rContext)
+
return rv;
}
Index: src/readerfactory.c
===================================================================
--- src/readerfactory.c (révision 6387)
+++ src/readerfactory.c (copie de travail)
@@ -58,6 +58,7 @@ static int ConfigFileCRC = 0;
static pthread_mutex_t LockMutex = PTHREAD_MUTEX_INITIALIZER;
#define IDENTITY_SHIFT 16
+static LONG removeReader(READER_CONTEXT * sReader);
static int RDR_CLIHANDLES_seeker(const void *el, const void *key)
{
@@ -78,6 +79,33 @@ static int RDR_CLIHANDLES_seeker(const v
}
+LONG _RefReader(READER_CONTEXT * sReader)
+{
+ if (0 == sReader->reference)
+ return SCARD_E_READER_UNAVAILABLE;
+
+ pthread_mutex_lock(&sReader->reference_lock);
+ sReader->reference += 1;
+ pthread_mutex_unlock(&sReader->reference_lock);
+
+ return SCARD_S_SUCCESS;
+}
+
+LONG _UnrefReader(READER_CONTEXT * sReader)
+{
+ if (0 == sReader->reference)
+ return SCARD_E_READER_UNAVAILABLE;
+
+ pthread_mutex_lock(&sReader->reference_lock);
+ sReader->reference -= 1;
+ pthread_mutex_unlock(&sReader->reference_lock);
+
+ if (0 == sReader->reference)
+ removeReader(sReader);
+
+ return SCARD_S_SUCCESS;
+}
+
LONG RFAllocateReaderSpace(unsigned int customMaxReaderHandles)
{
int i; /* Counter */
@@ -218,6 +246,11 @@ LONG RFAddReader(const char *readerNameL
NULL);
sReadersContexts[dwContext]->powerState = POWER_STATE_UNPOWERED;
+ /* reference count */
+ (void)pthread_mutex_init(&sReadersContexts[dwContext]->reference_lock,
+ NULL);
+ sReadersContexts[dwContext]->reference = 1;
+
/* If a clone to this reader exists take some values from that clone */
if (parentNode >= 0 && parentNode < PCSCLITE_MAX_READERS_CONTEXTS)
{
@@ -411,6 +444,11 @@ LONG RFAddReader(const char *readerNameL
NULL);
sReadersContexts[dwContextB]->powerState =
POWER_STATE_UNPOWERED;
+ /* reference count */
+
(void)pthread_mutex_init(&sReadersContexts[dwContextB]->reference_lock,
+ NULL);
+ sReadersContexts[dwContextB]->reference = 1;
+
/* Call on the parent driver to see if the slots are thread
safe */
dwGetSize = sizeof(ucThread);
rv = IFDGetCapabilities((sReadersContexts[dwContext]),
@@ -477,6 +515,21 @@ LONG RFRemoveReader(const char *readerNa
rv = RFReaderInfoNamePort(port, readerName, &sContext);
if (SCARD_S_SUCCESS == rv)
{
+ /* first to derefence RFReaderInfoNamePort() */
+ UNREF_READER(sContext)
+
+ /* second to remove the reader */
+ UNREF_READER(sContext)
+ }
+
+ return SCARD_S_SUCCESS;
+}
+
+LONG removeReader(READER_CONTEXT * sContext)
+{
+ LONG rv;
+
+ {
/* Try to destroy the thread */
if (sContext -> pthThread)
(void)EHDestroyEventHandler(sContext);
@@ -673,6 +726,9 @@ LONG RFReaderInfo(const char *readerName
if (strcmp(readerName,
sReadersContexts[i]->readerState->readerName)
== 0)
{
+ /* Increase reference count */
+ REF_READER(sReadersContexts[i])
+
*sReader = sReadersContexts[i];
return SCARD_S_SUCCESS;
}
@@ -704,6 +760,10 @@ LONG RFReaderInfoNamePort(int port, cons
if ((strncmp(readerName, lpcStripReader, MAX_READERNAME
- sizeof(" 00 00")) == 0)
&& (port == sReadersContexts[i]->port))
{
+ Log3(PCSC_LOG_DEBUG, "name: %s, port: %d",
readerName, port);
+ /* Increase reference count */
+ REF_READER(sReadersContexts[i])
+
*sReader = sReadersContexts[i];
return SCARD_S_SUCCESS;
}
@@ -728,6 +788,9 @@ LONG RFReaderInfoById(SCARDHANDLE hCard,
(void)pthread_mutex_unlock(&sReadersContexts[i]->handlesList_lock);
if (currentHandle != NULL)
{
+ /* Increase reference count */
+ REF_READER(sReadersContexts[i])
+
*sReader = sReadersContexts[i];
return SCARD_S_SUCCESS;
}
Index: src/readerfactory.h
===================================================================
--- src/readerfactory.h (révision 6387)
+++ src/readerfactory.h (copie de travail)
@@ -109,6 +109,8 @@
int * pMutex; /**< Number of client to mutex
*/
int powerState; /**< auto power off state */
pthread_mutex_t powerState_lock; /**< powerState mutex */
+ int reference; /**< number of users of the
structure */
+ pthread_mutex_t reference_lock; /**< reference mutex */
struct pubReaderStatesList *readerState; /**< link to the
reader state */
/* we can't use READER_STATE * here since eventhandler.h can't
be
@@ -117,6 +119,12 @@
typedef struct ReaderContext READER_CONTEXT;
+ LONG _RefReader(READER_CONTEXT * sReader);
+ LONG _UnrefReader(READER_CONTEXT * sReader);
+
+#define REF_READER(reader) { LONG rv; Log2(PCSC_LOG_CRITICAL, "RefReader()
count was: %d", reader->reference); rv = _RefReader(reader); if (rv !=
SCARD_S_SUCCESS) return rv; }
+#define UNREF_READER(reader) {Log2(PCSC_LOG_CRITICAL, "UnrefReader() count
was: %d", reader->reference); _UnrefReader(reader);}
+
LONG RFAllocateReaderSpace(unsigned int);
LONG RFAddReader(const char *, int, const char *, const char *);
LONG RFRemoveReader(const char *, int);
_______________________________________________
Muscle mailing list
[email protected]
http://lists.drizzle.com/mailman/listinfo/muscle
