Limit the time passed in transport check loop
authorDavid Fort <contact@hardening-consulting.com>
Tue, 3 May 2016 15:24:07 +0000 (17:24 +0200)
committerDavid Fort <contact@hardening-consulting.com>
Fri, 14 Oct 2016 13:12:48 +0000 (15:12 +0200)
This patch make it possible to limit the time that is passed when we call
XXX_check_fds functions. This should smooth the treatment between handling inputs
and handling incoming bitmap updates.
The default maximum time is set to 100 ms.

include/freerdp/peer.h
include/freerdp/settings.h
libfreerdp/core/peer.c
libfreerdp/core/settings.c
libfreerdp/core/transport.c
libfreerdp/core/transport.h

index a08b7f7..26c5703 100644 (file)
 
 #include <winpr/sspi.h>
 
-typedef BOOL (*psPeerContextNew)(freerdp_peer* client, rdpContext* context);
-typedef void (*psPeerContextFree)(freerdp_peer* client, rdpContext* context);
-
-typedef BOOL (*psPeerInitialize)(freerdp_peer* client);
-typedef BOOL (*psPeerGetFileDescriptor)(freerdp_peer* client, void** rfds, int* rcount);
-typedef HANDLE (*psPeerGetEventHandle)(freerdp_peer* client);
-typedef HANDLE (*psPeerGetReceiveEventHandle)(freerdp_peer* client);
-typedef BOOL (*psPeerCheckFileDescriptor)(freerdp_peer* client);
-typedef BOOL (*psPeerIsWriteBlocked)(freerdp_peer* client);
-typedef int (*psPeerDrainOutputBuffer)(freerdp_peer* client);
-typedef BOOL (*psPeerClose)(freerdp_peer* client);
-typedef void (*psPeerDisconnect)(freerdp_peer* client);
-typedef BOOL (*psPeerCapabilities)(freerdp_peer* client);
-typedef BOOL (*psPeerPostConnect)(freerdp_peer* client);
-typedef BOOL (*psPeerActivate)(freerdp_peer* client);
-typedef BOOL (*psPeerLogon)(freerdp_peer* client, SEC_WINNT_AUTH_IDENTITY* identity, BOOL automatic);
-
-typedef int (*psPeerSendChannelData)(freerdp_peer* client, UINT16 channelId, BYTE* data, int size);
-typedef int (*psPeerReceiveChannelData)(freerdp_peer* client, UINT16 channelId, BYTE* data, int size, int flags, int totalSize);
-
-typedef HANDLE (*psPeerVirtualChannelOpen)(freerdp_peer* client, const char* name, UINT32 flags);
-typedef BOOL (*psPeerVirtualChannelClose)(freerdp_peer* client, HANDLE hChannel);
-typedef int (*psPeerVirtualChannelRead)(freerdp_peer* client, HANDLE hChannel, BYTE* buffer, UINT32 length);
-typedef int (*psPeerVirtualChannelWrite)(freerdp_peer* client, HANDLE hChannel, BYTE* buffer, UINT32 length);
-typedef void* (*psPeerVirtualChannelGetData)(freerdp_peer* client, HANDLE hChannel);
-typedef int (*psPeerVirtualChannelSetData)(freerdp_peer* client, HANDLE hChannel, void* data);
+typedef BOOL (*psPeerContextNew)(freerdp_peer* peer, rdpContext* context);
+typedef void (*psPeerContextFree)(freerdp_peer* peer, rdpContext* context);
+
+typedef BOOL (*psPeerInitialize)(freerdp_peer* peer);
+typedef BOOL (*psPeerGetFileDescriptor)(freerdp_peer* peer, void** rfds, int* rcount);
+typedef HANDLE (*psPeerGetEventHandle)(freerdp_peer* peer);
+typedef HANDLE (*psPeerGetReceiveEventHandle)(freerdp_peer* peer);
+typedef BOOL (*psPeerCheckFileDescriptor)(freerdp_peer* peer);
+typedef BOOL (*psPeerIsWriteBlocked)(freerdp_peer* peer);
+typedef int (*psPeerDrainOutputBuffer)(freerdp_peer* peer);
+typedef BOOL (*psPeerHasMoreToRead)(freerdp_peer* peer);
+typedef BOOL (*psPeerClose)(freerdp_peer* peer);
+typedef void (*psPeerDisconnect)(freerdp_peer* peer);
+typedef BOOL (*psPeerCapabilities)(freerdp_peer* peer);
+typedef BOOL (*psPeerPostConnect)(freerdp_peer* peer);
+typedef BOOL (*psPeerActivate)(freerdp_peer* peer);
+typedef BOOL (*psPeerLogon)(freerdp_peer* peer, SEC_WINNT_AUTH_IDENTITY* identity, BOOL automatic);
+
+typedef int (*psPeerSendChannelData)(freerdp_peer* peer, UINT16 channelId, BYTE* data, int size);
+typedef int (*psPeerReceiveChannelData)(freerdp_peer* peer, UINT16 channelId, BYTE* data, int size, int flags, int totalSize);
+
+typedef HANDLE (*psPeerVirtualChannelOpen)(freerdp_peer* peer, const char* name, UINT32 flags);
+typedef BOOL (*psPeerVirtualChannelClose)(freerdp_peer* peer, HANDLE hChannel);
+typedef int (*psPeerVirtualChannelRead)(freerdp_peer* peer, HANDLE hChannel, BYTE* buffer, UINT32 length);
+typedef int (*psPeerVirtualChannelWrite)(freerdp_peer* peer, HANDLE hChannel, BYTE* buffer, UINT32 length);
+typedef void* (*psPeerVirtualChannelGetData)(freerdp_peer* peer, HANDLE hChannel);
+typedef int (*psPeerVirtualChannelSetData)(freerdp_peer* peer, HANDLE hChannel, void* data);
 
 struct rdp_freerdp_peer
 {
@@ -106,6 +107,7 @@ struct rdp_freerdp_peer
 
        psPeerIsWriteBlocked IsWriteBlocked;
        psPeerDrainOutputBuffer DrainOutputBuffer;
+       psPeerHasMoreToRead HasMoreToRead;
 };
 
 #ifdef __cplusplus
index 47d88e8..2d6e035 100644 (file)
@@ -829,7 +829,8 @@ struct rdp_settings
        ALIGN64 char* Domain; /* 23 */
        ALIGN64 char* PasswordHash; /* 24 */
        ALIGN64 BOOL WaitForOutputBufferFlush; /* 25 */
-       UINT64 padding0064[64 - 26]; /* 26 */
+       ALIGN64 UINT32 MaxTimeInCheckLoop; /* 26 */
+       UINT64 padding0064[64 - 27]; /* 27 */
        UINT64 padding0128[128 - 64]; /* 64 */
 
        /**
index 482f855..e1bbe9b 100644 (file)
@@ -674,6 +674,10 @@ static int freerdp_peer_drain_output_buffer(freerdp_peer* peer)
        return transport_drain_output_buffer(transport);
 }
 
+static BOOL freerdp_peer_has_more_to_read(freerdp_peer* peer) {
+       return peer->context->rdp->transport->haveMoreBytesToRead;
+}
+
 BOOL freerdp_peer_context_new(freerdp_peer* client)
 {
        rdpRdp* rdp;
@@ -731,6 +735,7 @@ BOOL freerdp_peer_context_new(freerdp_peer* client)
 
        client->IsWriteBlocked = freerdp_peer_is_write_blocked;
        client->DrainOutputBuffer = freerdp_peer_drain_output_buffer;
+       client->HasMoreToRead = freerdp_peer_has_more_to_read;
 
        IFCALLRET(client->ContextNew, ret, client, client->context);
 
@@ -803,6 +808,7 @@ freerdp_peer* freerdp_peer_new(int sockfd)
                client->SendChannelData = freerdp_peer_send_channel_data;
                client->IsWriteBlocked = freerdp_peer_is_write_blocked;
                client->DrainOutputBuffer = freerdp_peer_drain_output_buffer;
+               client->HasMoreToRead = freerdp_peer_has_more_to_read;
                client->VirtualChannelOpen = freerdp_peer_virtual_channel_open;
                client->VirtualChannelClose = freerdp_peer_virtual_channel_close;
                client->VirtualChannelWrite = freerdp_peer_virtual_channel_write;
index e5070a7..256a128 100644 (file)
@@ -277,6 +277,7 @@ rdpSettings* freerdp_settings_new(DWORD flags)
 
        settings->ServerMode = (flags & FREERDP_SETTINGS_SERVER_MODE) ? TRUE : FALSE;
        settings->WaitForOutputBufferFlush = TRUE;
+       settings->MaxTimeInCheckLoop = 100;
        settings->DesktopWidth = 1024;
        settings->DesktopHeight = 768;
        settings->Workarea = FALSE;
index 8bd610f..776c9a8 100644 (file)
@@ -756,38 +756,58 @@ out_cleanup:
 DWORD transport_get_event_handles(rdpTransport* transport, HANDLE* events,
                                   DWORD count)
 {
-       DWORD nCount = 0;
+       DWORD nCount = 1; /* always the reread Event */
        DWORD tmp;
 
+       if (events)
+       {
+               if (count < 1)
+               {
+                       WLog_ERR(TAG, "%s: provided handles array is too small", __FUNCTION__);
+                       return 0;
+               }
+
+               events[0] = transport->rereadEvent;
+       }
+
        if (!transport->GatewayEnabled)
        {
-               if (events && (nCount < count))
+               nCount++;
+               if (events)
                {
-                       if (BIO_get_event(transport->frontBio, &events[nCount]) != 1)
+                       if (nCount > count)
+                       {
+                               WLog_ERR(TAG, "%s: provided handles array is too small (count=%d nCount=%d)",
+                                               __FUNCTION__, count, nCount);
                                return 0;
+                       }
 
-                       nCount++;
+                       if (BIO_get_event(transport->frontBio, &events[1]) != 1)
+                       {
+                               WLog_ERR(TAG, "%s: error getting the frontBio handle", __FUNCTION__);
+                               return 0;
+                       }
                }
        }
        else
        {
                if (transport->rdg)
                {
-                       tmp = rdg_get_event_handles(transport->rdg, events, nCount - count);
+                       tmp = rdg_get_event_handles(transport->rdg, &events[1], count - 1);
 
                        if (tmp == 0)
                                return 0;
 
-                       nCount = tmp;
+                       nCount += tmp;
                }
                else if (transport->tsg)
                {
-                       tmp = tsg_get_event_handles(transport->tsg, events, nCount - count);
+                       tmp = tsg_get_event_handles(transport->tsg, &events[1], count - 1);
 
                        if (tmp == 0)
                                return 0;
 
-                       nCount = tmp;
+                       nCount += tmp;
                }
        }
 
@@ -800,12 +820,14 @@ void transport_get_fds(rdpTransport* transport, void** rfds, int* rcount)
        DWORD nCount;
        HANDLE events[64];
        nCount = transport_get_event_handles(transport, events, 64);
-       *rcount = nCount;
+       *rcount = nCount + 1;
 
        for (index = 0; index < nCount; index++)
        {
                rfds[index] = GetEventWaitObject(events[index]);
        }
+
+       rfds[nCount + 1] = GetEventWaitObject(transport->rereadEvent);
 }
 
 BOOL transport_is_write_blocked(rdpTransport* transport)
@@ -833,11 +855,19 @@ int transport_check_fds(rdpTransport* transport)
        int status;
        int recv_status;
        wStream* received;
+       DWORD now = GetTickCount();
+       DWORD dueDate = now + transport->settings->MaxTimeInCheckLoop;
 
        if (!transport)
                return -1;
 
-       while (!freerdp_shall_disconnect(transport->context->instance))
+       if (transport->haveMoreBytesToRead)
+       {
+               transport->haveMoreBytesToRead = FALSE;
+               ResetEvent(transport->rereadEvent);
+       }
+
+       while(!freerdp_shall_disconnect(transport->context->instance) && (now < dueDate))
        {
                /**
                 * Note: transport_read_pdu tries to read one PDU from
@@ -883,8 +913,15 @@ int transport_check_fds(rdpTransport* transport)
                                 recv_status);
                        return -1;
                }
+
+               now = GetTickCount();
        }
 
+       if (now > dueDate)
+       {
+               SetEvent(transport->rereadEvent);
+               transport->haveMoreBytesToRead = TRUE;
+       }
        return 0;
 }
 
@@ -1073,12 +1110,17 @@ rdpTransport* transport_new(rdpContext* context)
            || transport->connectedEvent == INVALID_HANDLE_VALUE)
                goto out_free_receivebuffer;
 
+       transport->rereadEvent = CreateEvent(NULL, TRUE, FALSE, NULL);
+       if (!transport->rereadEvent || transport->rereadEvent == INVALID_HANDLE_VALUE)
+               goto out_free_connectedEvent;
+
+       transport->haveMoreBytesToRead = FALSE;
        transport->blocking = TRUE;
        transport->GatewayEnabled = FALSE;
        transport->layer = TRANSPORT_LAYER_TCP;
 
        if (!InitializeCriticalSectionAndSpinCount(&(transport->ReadLock), 4000))
-               goto out_free_connectedEvent;
+               goto out_free_rereadEvent;
 
        if (!InitializeCriticalSectionAndSpinCount(&(transport->WriteLock), 4000))
                goto out_free_readlock;
@@ -1086,6 +1128,8 @@ rdpTransport* transport_new(rdpContext* context)
        return transport;
 out_free_readlock:
        DeleteCriticalSection(&(transport->ReadLock));
+out_free_rereadEvent:
+       CloseHandle(transport->rereadEvent);
 out_free_connectedEvent:
        CloseHandle(transport->connectedEvent);
 out_free_receivebuffer:
index 15c6049..73a733e 100644 (file)
@@ -79,6 +79,8 @@ struct rdp_transport
        CRITICAL_SECTION ReadLock;
        CRITICAL_SECTION WriteLock;
        ULONG written;
+       HANDLE rereadEvent;
+       BOOL haveMoreBytesToRead;
 };
 
 FREERDP_LOCAL wStream* transport_send_stream_init(rdpTransport* transport,