www.cendio.com
Bug 5222 - RDPDR channel sometimes stops working when a smart card is inserted / removed on Windows 2012 R2 server
: RDPDR channel sometimes stops working when a smart card is inserted / removed...
Status: CLOSED FIXED
: ThinLinc
| rdesktop (deprecated)
: trunk
: PC Unknown
: P2 Normal
: 4.3.0
Assigned To:
:
:
:
: 4912
  Show dependency treegraph
 
Reported: 2014-08-20 08:40 by
Modified: 2014-10-07 16:52 (History)
Acceptance Criteria:


Attachments


Note

You need to log in before you can comment on or make changes to this bug.


Description From cendio 2014-08-20 08:40:53
Works perfectly fine when doing so against Windows 2012 Server.
------- Comment #1 From cendio 2014-09-04 08:55:53 -------
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 From cendio 2014-09-05 08:03:58 -------
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 From cendio 2014-09-05 14:00:00 -------
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 From cendio 2014-09-09 09:41:58 -------
(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 From cendio 2014-09-11 13:24:29 -------
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 From cendio 2014-09-11 13:25:22 -------
(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 From cendio 2014-09-11 13:49:39 -------
(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 From cendio 2014-09-11 14:02:32 -------
(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 From cendio 2014-09-11 14:09:43 -------
(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 From cendio 2014-09-11 14:20:12 -------
(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 From cendio 2014-09-11 15:53:30 -------
(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 From cendio 2014-09-26 10:50:14 -------
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 From cendio 2014-10-07 16:52:06 -------
(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