From 3c4bee3d95631624145af9b17d57962f17d193ea Mon Sep 17 00:00:00 2001 From: =?utf8?q?Marc-Andr=C3=A9=20Moreau?= Date: Tue, 17 Mar 2015 16:54:45 -0400 Subject: [PATCH] libfreerdp-core: fix RDG valgrind issues --- libfreerdp/core/gateway/ntlm.c | 2 + libfreerdp/core/gateway/rdg.c | 140 ++++++++++++++++++++--------------------- libfreerdp/core/gateway/rdg.h | 3 +- libfreerdp/core/gateway/rpc.c | 6 +- winpr/libwinpr/handle/handle.c | 4 +- winpr/libwinpr/synch/mutex.c | 18 +++--- 6 files changed, 87 insertions(+), 86 deletions(-) diff --git a/libfreerdp/core/gateway/ntlm.c b/libfreerdp/core/gateway/ntlm.c index 495372a..221258d 100644 --- a/libfreerdp/core/gateway/ntlm.c +++ b/libfreerdp/core/gateway/ntlm.c @@ -302,5 +302,7 @@ void ntlm_free(rdpNtlm* ntlm) ntlm->outputBuffer[0].pvBuffer = NULL; } + ntlm_client_uninit(ntlm); + free(ntlm); } diff --git a/libfreerdp/core/gateway/rdg.c b/libfreerdp/core/gateway/rdg.c index 8aa39c2..f5cf3cf 100644 --- a/libfreerdp/core/gateway/rdg.c +++ b/libfreerdp/core/gateway/rdg.c @@ -204,7 +204,7 @@ BOOL rdg_send_tunnel_authorization(rdpRdg* rdg) int i; wStream* s; BOOL status; - char* clientName = rdg->context->settings->ClientHostname; + char* clientName = rdg->settings->ClientHostname; UINT16 clientNameLen = strlen(clientName) + 1; UINT32 packetSize = 12 + clientNameLen * 2; @@ -229,6 +229,8 @@ BOOL rdg_send_tunnel_authorization(rdpRdg* rdg) status = rdg_write_packet(rdg, s); + Stream_Free(s, TRUE); + if (status) { rdg->state = RDG_CLIENT_STATE_TUNNEL_AUTHORIZE; @@ -244,7 +246,7 @@ BOOL rdg_send_channel_create(rdpRdg* rdg) int i; wStream* s; BOOL status; - char* serverName = rdg->context->settings->ServerHostname; + char* serverName = rdg->settings->ServerHostname; UINT16 serverNameLen = strlen(serverName) + 1; UINT32 packetSize = 16 + serverNameLen * 2; @@ -253,14 +255,14 @@ BOOL rdg_send_channel_create(rdpRdg* rdg) if (!s) return FALSE; - Stream_Write_UINT16(s, PKT_TYPE_CHANNEL_CREATE); /* Type */ - Stream_Write_UINT16(s, 0); /* Reserved */ - Stream_Write_UINT32(s, packetSize); /* Packet length */ + Stream_Write_UINT16(s, PKT_TYPE_CHANNEL_CREATE); /* Type (2 bytes) */ + Stream_Write_UINT16(s, 0); /* Reserved (2 bytes) */ + Stream_Write_UINT32(s, packetSize); /* PacketLength (4 bytes) */ - Stream_Write_UINT8(s, 1); /* Number of resources. */ - Stream_Write_UINT8(s, 0); /* Number of alternative resources. */ - Stream_Write_UINT16(s, 3389); /* Port, this seems to be the standard... ? */ - Stream_Write_UINT16(s, 3); /* Protocol number, set according to an example... ? */ + Stream_Write_UINT8(s, 1); /* Number of resources. (1 byte) */ + Stream_Write_UINT8(s, 0); /* Number of alternative resources. (1 byte) */ + Stream_Write_UINT16(s, 3389); /* Port, this seems to be the standard... ? (2 bytes) */ + Stream_Write_UINT16(s, 3); /* Protocol number, set according to an example... ? (2 bytes) */ Stream_Write_UINT16(s, serverNameLen * 2); for (i = 0; i < serverNameLen; i++) @@ -272,6 +274,8 @@ BOOL rdg_send_channel_create(rdpRdg* rdg) status = rdg_write_packet(rdg, s); + Stream_Free(s, TRUE); + if (status) { rdg->state = RDG_CLIENT_STATE_CHANNEL_CREATE; @@ -307,19 +311,17 @@ wStream* rdg_build_http_request(rdpRdg* rdg, char* method) ntlmToken = rdg->ntlm->outputBuffer; if (ntlmToken) - { base64NtlmToken = crypto_base64_encode(ntlmToken->pvBuffer, ntlmToken->cbBuffer); - } if (base64NtlmToken) { http_request_set_auth_scheme(request, "NTLM"); http_request_set_auth_param(request, base64NtlmToken); + free(base64NtlmToken); + if (!request->AuthScheme || !request->AuthParam) - { return NULL; - } } } @@ -844,7 +846,7 @@ BOOL rdg_tls_out_connect(rdpRdg* rdg, const char* hostname, UINT16 port, int tim int status = 0; BIO* socketBio = NULL; BIO* bufferedBio = NULL; - rdpSettings* settings = rdg->context->settings; + rdpSettings* settings = rdg->settings; assert(rdg != NULL && hostname != NULL); @@ -900,7 +902,7 @@ BOOL rdg_tls_in_connect(rdpRdg* rdg, const char* hostname, UINT16 port, int time int status = 0; BIO* socketBio = NULL; BIO* bufferedBio = NULL; - rdpSettings* settings = rdg->context->settings; + rdpSettings* settings = rdg->settings; assert(rdg != NULL && hostname != NULL); @@ -1196,9 +1198,7 @@ static int rdg_bio_write(BIO* bio, const char* buf, int num) int status; rdpRdg* rdg = (rdpRdg*)bio->ptr; - status = rdg_write_data_packet(rdg, (BYTE*)buf, num); - - printf("write request %d, get %d\r\n", num, status); + status = rdg_write_data_packet(rdg, (BYTE*) buf, num); if (status < 0) { @@ -1225,9 +1225,7 @@ static int rdg_bio_read(BIO* bio, char* buf, int size) int status; rdpRdg* rdg = (rdpRdg*)bio->ptr; - status = rdg_read_data_packet(rdg, (BYTE*)buf, size); - - printf("read request %d, get %d\r\n", size, status); + status = rdg_read_data_packet(rdg, (BYTE*) buf, size); if (status < 0) { @@ -1375,58 +1373,60 @@ rdpRdg* rdg_new(rdpTransport* transport) { rdg->state = RDG_CLIENT_STATE_INITIAL; rdg->context = transport->context; + rdg->settings = rdg->context->settings; + UuidCreate(&rdg->guid); + rpcStatus = UuidToStringA(&rdg->guid, &stringUuid); + if (rpcStatus == RPC_S_OUT_OF_MEMORY) - { goto rdg_alloc_error; - } - sprintf(bracedUuid, "{%s}", stringUuid); + + sprintf_s(bracedUuid, sizeof(bracedUuid), "{%s}", stringUuid); RpcStringFreeA(&stringUuid); - rdg->tlsOut = tls_new(rdg->context->settings); + rdg->tlsOut = tls_new(rdg->settings); + if (!rdg->tlsOut) - { goto rdg_alloc_error; - } - rdg->tlsIn = tls_new(rdg->context->settings); + + rdg->tlsIn = tls_new(rdg->settings); + if (!rdg->tlsIn) - { goto rdg_alloc_error; - } rdg->http = http_context_new(); + if (!rdg->http) - { goto rdg_alloc_error; - } + http_context_set_uri(rdg->http, "/remoteDesktopGateway/"); http_context_set_accept(rdg->http, "*/*"); http_context_set_cache_control(rdg->http, "no-cache"); http_context_set_pragma(rdg->http, "no-cache"); http_context_set_connection(rdg->http, "Keep-Alive"); http_context_set_user_agent(rdg->http, "MS-RDGateway/1.0"); - http_context_set_host(rdg->http, rdg->context->settings->GatewayHostname); + http_context_set_host(rdg->http, rdg->settings->GatewayHostname); http_context_set_rdg_connection_id(rdg->http, bracedUuid); - if (!rdg->http->URI || !rdg->http->Accept || !rdg->http->CacheControl || !rdg->http->Pragma || !rdg->http->Connection - || !rdg->http->UserAgent || !rdg->http->Host || !rdg->http->RdgConnectionId) + if (!rdg->http->URI || !rdg->http->Accept || !rdg->http->CacheControl || + !rdg->http->Pragma || !rdg->http->Connection || !rdg->http->UserAgent + || !rdg->http->Host || !rdg->http->RdgConnectionId) { goto rdg_alloc_error; } rdg->frontBio = BIO_new(BIO_s_rdg()); + if (!rdg->frontBio) - { goto rdg_alloc_error; - } + rdg->frontBio->ptr = rdg; rdg->readEvent = CreateEvent(NULL, TRUE, FALSE, NULL); + if (!rdg->readEvent) - { goto rdg_alloc_error; - } } return rdg; @@ -1438,44 +1438,38 @@ rdg_alloc_error: void rdg_free(rdpRdg* rdg) { - if (rdg) - { - if (rdg->tlsOut) - { - tls_free(rdg->tlsOut); - rdg->tlsOut = NULL; - } - - if (rdg->tlsIn) - { - tls_free(rdg->tlsIn); - rdg->tlsIn = NULL; - } + if (!rdg) + return; - if (rdg->http) - { - http_context_free(rdg->http); - rdg->http = NULL; - } + if (rdg->tlsOut) + { + tls_free(rdg->tlsOut); + rdg->tlsOut = NULL; + } - if (rdg->ntlm) - { - ntlm_free(rdg->ntlm); - rdg->ntlm = NULL; - } + if (rdg->tlsIn) + { + tls_free(rdg->tlsIn); + rdg->tlsIn = NULL; + } - if (rdg->frontBio) - { - BIO_free(rdg->frontBio); - rdg->frontBio = NULL; - } + if (rdg->http) + { + http_context_free(rdg->http); + rdg->http = NULL; + } - if (rdg->readEvent) - { - CloseHandle(rdg->readEvent); - rdg->readEvent = NULL; - } + if (rdg->ntlm) + { + ntlm_free(rdg->ntlm); + rdg->ntlm = NULL; + } - free(rdg); + if (rdg->readEvent) + { + CloseHandle(rdg->readEvent); + rdg->readEvent = NULL; } + + free(rdg); } diff --git a/libfreerdp/core/gateway/rdg.h b/libfreerdp/core/gateway/rdg.h index 78ed3e3..3c5a9bf 100644 --- a/libfreerdp/core/gateway/rdg.h +++ b/libfreerdp/core/gateway/rdg.h @@ -125,7 +125,8 @@ typedef struct rdp_transport rdpTransport; typedef struct rdp_rdg rdpRdg; struct rdp_rdg { - rdpContext* context; /* Shortcut to parent context. */ + rdpContext* context; + rdpSettings* settings; BIO* bioIn; BIO* bioOut; BIO* frontBio; diff --git a/libfreerdp/core/gateway/rpc.c b/libfreerdp/core/gateway/rpc.c index 3a211de..e976816 100644 --- a/libfreerdp/core/gateway/rpc.c +++ b/libfreerdp/core/gateway/rpc.c @@ -554,6 +554,10 @@ int rpc_out_channel_transition_to_state(RpcOutChannel* outChannel, CLIENT_OUT_CH str = "CLIENT_OUT_CHANNEL_STATE_OPENED_B3W"; break; + case CLIENT_OUT_CHANNEL_STATE_RECYCLED: + str = "CLIENT_OUT_CHANNEL_STATE_RECYCLED"; + break; + case CLIENT_OUT_CHANNEL_STATE_FINAL: str = "CLIENT_OUT_CHANNEL_STATE_FINAL"; break; @@ -866,8 +870,6 @@ int rpc_out_channel_connect(RpcOutChannel* outChannel, int timeout) int rpc_out_channel_replacement_connect(RpcOutChannel* outChannel, int timeout) { - int status = 0; - HttpResponse* response = NULL; rdpRpc* rpc = outChannel->rpc; /* Connect OUT Channel */ diff --git a/winpr/libwinpr/handle/handle.c b/winpr/libwinpr/handle/handle.c index 5ffce91..5cc5e41 100644 --- a/winpr/libwinpr/handle/handle.c +++ b/winpr/libwinpr/handle/handle.c @@ -48,7 +48,7 @@ BOOL CloseHandle(HANDLE hObject) ULONG Type; WINPR_HANDLE *Object; - if (!winpr_Handle_GetInfo(hObject, &Type, (PVOID*)&Object)) + if (!winpr_Handle_GetInfo(hObject, &Type, (PVOID*) &Object)) return FALSE; if (!Object) @@ -57,7 +57,7 @@ BOOL CloseHandle(HANDLE hObject) if (!Object->ops) return FALSE; - if(Object->ops->CloseHandle) + if (Object->ops->CloseHandle) return Object->ops->CloseHandle(hObject); return FALSE; diff --git a/winpr/libwinpr/synch/mutex.c b/winpr/libwinpr/synch/mutex.c index 4afbb95..a6c8ac6 100644 --- a/winpr/libwinpr/synch/mutex.c +++ b/winpr/libwinpr/synch/mutex.c @@ -54,13 +54,14 @@ static int MutexGetFd(HANDLE handle) return -1; } -BOOL MutexCloseHandle(HANDLE handle) { - WINPR_MUTEX *mutex = (WINPR_MUTEX *) handle; +BOOL MutexCloseHandle(HANDLE handle) +{ + WINPR_MUTEX* mutex = (WINPR_MUTEX*) handle; if (!MutexIsHandled(handle)) return FALSE; - if(pthread_mutex_destroy(&mutex->mutex)) + if (!pthread_mutex_destroy(&mutex->mutex)) return FALSE; free(handle); @@ -68,11 +69,12 @@ BOOL MutexCloseHandle(HANDLE handle) { return TRUE; } -static HANDLE_OPS ops = { - MutexIsHandled, - MutexCloseHandle, - MutexGetFd, - NULL /* CleanupHandle */ +static HANDLE_OPS ops = +{ + MutexIsHandled, + MutexCloseHandle, + MutexGetFd, + NULL /* CleanupHandle */ }; HANDLE CreateMutexW(LPSECURITY_ATTRIBUTES lpMutexAttributes, BOOL bInitialOwner, LPCWSTR lpName) -- 2.7.4