[Pcsclite-muscle] Race condition with SCardGetStatusChange() when USB Reader is removed

[Ludovic Rousseau]

2017-05-08 12:47 GMT+02:00 Maximilian Stein <maximilian.stein at secunet.com>:

> > Yes. It is possible.
> > pcsc-lite may contain bugs.
>
> I'm sorry if that sounded offensive. Personally I appreciate the work
> and effort you put into this project very much and merely want to
> contribute to its further improvement.
>

No offense.
I was making joke of myself :-)
I should have added a smiley.

> I tried to reproduce the problem with the attached sample code but
> > without success.
> > I tried using the special reader "\\?PnP?\Notification" and also using
> > the current reader name but could not reproduce the problem. Yes, I
> > first applied your patch and I get the extra sleep() in pcscd.
> >
> > You can change line 52 of my sample code to use the PnP reader or the
> > normal one.
>
> On my testing machine (Xubuntu 16.04 python-pyscard installed from
> repos) I can reproduce the problem with the python Unit Tests and your
> sample code, with any value in line 52. But on another installation of
> Ubuntu 16.04 (self compiled pyscard) I can't reproduce it either.
>
> My patch includes a change in the lipcsclite client library which is
> very important to provoke the race condition. This change should produce
> debug output ("Waiting 2s to provoke...") when executing the test
> programs with env variable PCSCLITE_DEBUG=0.
>
> Did you get this additional client side debug output?
>

I was not using the correct libpcsclite.
Now I can reproduce the problem using your C code.

The client will not be blocked during an INFINITE time but for 60 seconds
https://anonscm.debian.org/cgit/pcsclite/PCSC.git/tree/
src/winscard_clnt.c#n1783
But that is still a bug.


My proposed patch is do modify removeReader() [readerfactory.c] to call
EHDestroyEventHandler() _after_ RFUnInitializeReader() instead of before.

Something like:
--- /tmp/paqtOc_readerfactory.c 2017-05-09 15:15:23.885862634 +0200
+++ src/readerfactory.c 2017-05-09 15:15:21.461801253 +0200
@@ -615,6 +615,8 @@ LONG RFRemoveReader(const char *readerNa

 LONG removeReader(READER_CONTEXT * sContext)
 {
+   RFUnInitializeReader(sContext);
+
    /* Try to destroy the thread */
    if (sContext -> pthThread)
        (void)EHDestroyEventHandler(sContext);
@@ -626,8 +628,6 @@ LONG removeReader(READER_CONTEXT * sCont
        return SCARD_E_INVALID_VALUE;
    }

-   RFUnInitializeReader(sContext);
-
    *sContext->pMutex -= 1;


With this patch, the problem is fixed on my side.
Do you confirm it also fixes the problem for you?

Thanks

-- 
 Dr. Ludovic Rousseau
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.alioth.debian.org/pipermail/pcsclite-muscle/attachments/20170509/12e69433/attachment.html>