[PATCH] fix racecondition between winscard server and clients

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] fix racecondition between winscard server and clients

Florian Kaiser
Hi,

this change fixes a racecondition between a winscard server and clients.
Feel free to message me, if my explanation below is not sufficient. I will try
to provide more Information in this case.


Regards,

Florian Kaiser


Signed-off-by: Florian Kaiser <[hidden email]>
---
 src/winscard_svc.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/src/winscard_svc.c b/src/winscard_svc.c
index b551c20..fc5ec38 100644
--- a/src/winscard_svc.c
+++ b/src/winscard_svc.c
@@ -419,10 +419,31 @@ static void ContextThread(LPVOID newContext)
 
                                READ_BODY(waStr)
 
-                               /* add the client fd to the list */
+                               /* remove the client fd from the list */
                                waStr.rv = EHUnregisterClientForEvent(filedes);
 
-                               WRITE_BODY(waStr)
+                               /* After the client timed out it sends a
+                                * CMD_STOP_WAITING_READER_STATE_CHANGE.
+                                * Sometimes an event occurs which triggers
+                                * MSGSignalClient, which unregisters the
+                                * client from events and sends a
+                                * wait_reader_state_change message to the
+                                * client. The client interprets this message
+                                * as an answer to its
+                                * CMD_STOP_WAITING_READER_STATE_CHANGE.
+                                * If we send another wait_reader_state_change
+                                * message to the client it stays in its buffer
+                                * until another Command is issued and this
+                                * message appears as leading garbage in the
+                                * commands answer.
+                                * Therefore we should not send the
+                                * wait_reader_state_change message here, if
+                                * the client has already been unregisterd
from
+                                * events.
+                                */
+                               if (waStr.rv != SCARD_F_INTERNAL_ERROR) {
+                                       WRITE_BODY(waStr)
+                               }
                        }
                        break;
 
--
2.1.4

_______________________________________________
Pcsclite-muscle mailing list
[hidden email]
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pcsclite-muscle
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] fix racecondition between winscard server and clients

Ludovic Rousseau
2016-11-30 13:26 GMT+01:00 Florian Kaiser <[hidden email]>:
> Hi,

Hello,

> this change fixes a racecondition between a winscard server and clients.
> Feel free to message me, if my explanation below is not sufficient. I will try
> to provide more Information in this case.

Do you have a sample code or an easy way to reproduce the problem?

Thanks

--
 Dr. Ludovic Rousseau

_______________________________________________
Pcsclite-muscle mailing list
[hidden email]
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pcsclite-muscle
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] fix racecondition between winscard server and clients

Ludovic Rousseau
2016-12-03 19:55 GMT+01:00 Ludovic Rousseau <[hidden email]>:

> 2016-11-30 13:26 GMT+01:00 Florian Kaiser <[hidden email]>:
>> Hi,
>
> Hello,
>
>> this change fixes a racecondition between a winscard server and clients.
>> Feel free to message me, if my explanation below is not sufficient. I will try
>> to provide more Information in this case.
>
> Do you have a sample code or an easy way to reproduce the problem?

Fixed in revision
https://github.com/LudovicRousseau/PCSC/commit/4e2a563c8ed4353ad013de85b71aac12ec599f82

Thanks

--
 Dr. Ludovic Rousseau

_______________________________________________
Pcsclite-muscle mailing list
[hidden email]
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pcsclite-muscle
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] fix racecondition between winscard server and clients

Florian Kaiser
Hi,

sorry to bother you again, but there is a small error in your upstream patch.


+ if (waStr.rv != SCARD_F_INTERNAL_ERROR)
+ WRITE_BODY(waStr)

WRITE_BODY is a Macro around two function calls. After the preprocessor ran
the code would look something like this (the parameters are wrong here):

if (waStr.rv != SCARD_F_INTERNAL_ERROR)
log_msg(priority, "%s:%d:%s() " fmt, __FILE__, __LINE__, __FUNCTION__, data1,
data2, data3);
ret = MessageSend(&v, sizeof(v), filedes);

Now the if-statement only affects the log_msg function call, so the curly
brackets are required around this kind of macro...

I have attached a patch, which secures this macro and prevents errors like
this.

Thanks a lot

Florian Kaiser


On Tuesday, December 06, 2016 04:56:09 PM Ludovic Rousseau wrote:

> 2016-12-03 19:55 GMT+01:00 Ludovic Rousseau <[hidden email]>:
> > 2016-11-30 13:26 GMT+01:00 Florian Kaiser <[hidden email]>:
> >> Hi,
> >
> > Hello,
> >
> >> this change fixes a racecondition between a winscard server and clients.
> >> Feel free to message me, if my explanation below is not sufficient. I
> >> will try to provide more Information in this case.
> >
> > Do you have a sample code or an easy way to reproduce the problem?
>
> Fixed in revision
> https://github.com/LudovicRousseau/PCSC/commit/4e2a563c8ed4353ad013de85b71aa
> c12ec599f82
>
> Thanks

_______________________________________________
Pcsclite-muscle mailing list
[hidden email]
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pcsclite-muscle

pcsc-macros.patch (9K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] fix racecondition between winscard server and clients

Ludovic Rousseau
2016-12-07 15:05 GMT+01:00 Florian Kaiser <[hidden email]>:

> Hi,
>
> sorry to bother you again, but there is a small error in your upstream patch.
>
>
> +                               if (waStr.rv != SCARD_F_INTERNAL_ERROR)
> +                                       WRITE_BODY(waStr)
>
> WRITE_BODY is a Macro around two function calls. After the preprocessor ran
> the code would look something like this (the parameters are wrong here):
>
> if (waStr.rv != SCARD_F_INTERNAL_ERROR)
> log_msg(priority, "%s:%d:%s() " fmt, __FILE__, __LINE__, __FUNCTION__, data1,
> data2, data3);
> ret = MessageSend(&v, sizeof(v), filedes);
>
> Now the if-statement only affects the log_msg function call, so the curly
> brackets are required around this kind of macro...
>
> I have attached a patch, which secures this macro and prevents errors like
> this.

You are right. Stupid me.

I tested the fix with an instrumented code using { } and it worked.

Problem fixed in
https://github.com/LudovicRousseau/PCSC/commit/152a53e5151f4f81c572dfeae02d5e0ea8eebccf

Thanks again.

I will release a new version of pcsc-lite soon to make the fix available.

Bye

--
 Dr. Ludovic Rousseau

_______________________________________________
Pcsclite-muscle mailing list
[hidden email]
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pcsclite-muscle