Comment 1 Henrik Andersson 2014-09-04 08:55:53 CEST

There is a bug in pcsclite in call chain SCardGetStatusChange()->WaitForPcscdEvent() where a timeout value of 0 will block the call instead of beeing a poll.

On rdesktop side I can make a workaround to change all timeouts from 0 to 1ms for polling functionality, this however will introduce a hang of a few ms. but thats better then hanging forever.

Comment 2 Henrik Andersson 2014-09-05 08:03:58 CEST

Another bug with pcsclite discovered:

SCardGetStatusChange() locks a context which results with other operations using the same context like SCardListReaders() will hang until SCardGetStatusChange() finished.

A workaround on rdesktop side is to create a new internal context for each dispatch of SCardGetStatusChange() which seems to work as expected.

Comment 3 Henrik Andersson 2014-09-05 14:00:00 CEST

Identified a bug a potential memory leak in handling of IRP_MJ_DEVICE_CONTROL requests. Each dispatched request is put into list using add_async_iorequest()
which is processed using select and built around the use of a fd, serial/file io. Smartcard doesn't use a fd and therefor it's never removed form this list.

Simple fix, don't put smartcard request into async_iorequest(), scard.c have internal handling of request and their response.

Comment 4 Henrik Andersson 2014-09-09 09:41:58 CEST

(In reply to comment #3)
> Identified a bug a potential memory leak in handling of IRP_MJ_DEVICE_CONTROL
> requests. Each dispatched request is put into list using add_async_iorequest()
> which is processed using select and built around the use of a fd, serial/file
> io. Smartcard doesn't use a fd and therefor it's never removed form this list.
> 
> Simple fix, don't put smartcard request into async_iorequest(), scard.c have
> internal handling of request and their response.

This is not an issue, scard.c:thread_wrapper(), the fns->device_control() implementation adds 0xc0000000 to status code which is used to skip adding pending request to list.

Comment 5 Henrik Andersson 2014-09-11 13:24:29 CEST

The problem is now pinpointed to:

During log on to a session, the RDPDR channel is initialize so that smartcard can be used. After the log on there is a reinitialize of RDPDR channel that also implies that all pending iorequest is abdonend. If rdesktop have pending responses to iorequest received on initial RDPDR channel which will be sent to the server on the reinitialized RDPDR channel, the server will shutdown the RDPDR channel and no further packets will be sent/received over RDPDR.

Comment 6 Henrik Andersson 2014-09-11 13:25:22 CEST

(In reply to comment #1)
> There is a bug in pcsclite in call chain
> SCardGetStatusChange()->WaitForPcscdEvent() where a timeout value of 0 will
> block the call instead of beeing a poll.
> 
> On rdesktop side I can make a workaround to change all timeouts from 0 to 1ms
> for polling functionality, this however will introduce a hang of a few ms. but
> thats better then hanging forever.

Workaround in upstream commit 1835.

Comment 7 Henrik Andersson 2014-09-11 13:49:39 CEST

(In reply to comment #0)
> Works perfectly fine when doing so against Windows 2012 Server.

Testcase:

The issue appeared once you inserted/removed the card from the reader. The issue is 50% reproducible with following apporach:

Connect with CredSSP + Kerberos, local drive, smartcard redirection and provide username/password on commandline for rdesktop.

1. *Reconnect* to a session with smartcard insterted
2. Open the local drive redirection in Explorer so you can
   see the files...
3. Remove card from reader
4. Refresh explorer window and you will be prompted with a
   message '\\tsclient\xxx is not accessible...'

Comment 8 Henrik Andersson 2014-09-11 14:02:32 CEST

(In reply to comment #5)
> The problem is now pinpointed to:
> 
> During log on to a session, the RDPDR channel is initialize so that smartcard
> can be used. After the log on there is a reinitialize of RDPDR channel that
> also implies that all pending iorequest is abdonend. If rdesktop have pending
> responses to iorequest received on initial RDPDR channel which will be sent to
> the server on the reinitialized RDPDR channel, the server will shutdown the
> RDPDR channel and no further packets will be sent/received over RDPDR.

Workaround for the issue is commited upstream in r1836.

The workaround is to prevent sending iorequest responses to abandoned iorequests on a reinitialized RDPDR channel by tagging iorequest to a specific epoch.

Comment 9 Henrik Andersson 2014-09-11 14:09:43 CEST

(In reply to comment #8)
> (In reply to comment #5)
> > The problem is now pinpointed to:
> > 
> > During log on to a session, the RDPDR channel is initialize so that smartcard
> > can be used. After the log on there is a reinitialize of RDPDR channel that
> > also implies that all pending iorequest is abdonend. If rdesktop have pending
> > responses to iorequest received on initial RDPDR channel which will be sent to
> > the server on the reinitialized RDPDR channel, the server will shutdown the
> > RDPDR channel and no further packets will be sent/received over RDPDR.
> 
> Workaround for the issue is commited upstream in r1836.
> 
> The workaround is to prevent sending iorequest responses to abandoned
> iorequests on a reinitialized RDPDR channel by tagging iorequest to a specific
> epoch.

Created bug #5252 for introducing resource leak in the workaround.

Comment 10 Henrik Andersson 2014-09-11 14:20:12 CEST

(In reply to comment #8)
> (In reply to comment #5)
> > The problem is now pinpointed to:
> > 
> > During log on to a session, the RDPDR channel is initialize so that smartcard
> > can be used. After the log on there is a reinitialize of RDPDR channel that
> > also implies that all pending iorequest is abdonend. If rdesktop have pending
> > responses to iorequest received on initial RDPDR channel which will be sent to
> > the server on the reinitialized RDPDR channel, the server will shutdown the
> > RDPDR channel and no further packets will be sent/received over RDPDR.
> 
> Workaround for the issue is commited upstream in r1836.
> 
> The workaround is to prevent sending iorequest responses to abandoned
> iorequests on a reinitialized RDPDR channel by tagging iorequest to a specific
> epoch.

Brought to ctc in commit 29346.

Comment 11 Henrik Andersson 2014-09-11 15:53:30 CEST

(In reply to comment #1)
> There is a bug in pcsclite in call chain
> SCardGetStatusChange()->WaitForPcscdEvent() where a timeout value of 0 will
> block the call instead of beeing a poll.
> 
> On rdesktop side I can make a workaround to change all timeouts from 0 to 1ms
> for polling functionality, this however will introduce a hang of a few ms. but
> thats better then hanging forever.

Posted this issue to the maling list due to i didn't find any issue tracker for the project.

Comment 12 Samuel Mannehed 2014-09-26 10:50:14 CEST

Was not able to reproduce the issue on Windows 2012 R2 Server. Will verify that nothing is broken after the changes on Windows 2008 R2 server before closing.

Comment 13 Samuel Mannehed 2014-10-07 16:52:06 CEST

(In reply to comment #12)
> Was not able to reproduce the issue on Windows 2012 R2 Server. Will verify that
> nothing is broken after the changes on Windows 2008 R2 server before closing.

Looks good