From 873bb26f2e9e77349f8b01630064ae5ec00a86a8 Mon Sep 17 00:00:00 2001 From: weizhenwei Date: Fri, 9 Jun 2017 10:31:00 +0800 Subject: [PATCH 01/20] fix a memroy leak on rdpBitmap free --- libfreerdp/cache/offscreen.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libfreerdp/cache/offscreen.c b/libfreerdp/cache/offscreen.c index 1b7c2b0f1..df79fc822 100644 --- a/libfreerdp/cache/offscreen.c +++ b/libfreerdp/cache/offscreen.c @@ -155,7 +155,7 @@ void offscreen_cache_delete(rdpOffscreenCache* offscreenCache, UINT32 index) prevBitmap = offscreenCache->entries[index]; if (prevBitmap != NULL) - prevBitmap->Free(offscreenCache->update->context, prevBitmap); + Bitmap_Free(offscreenCache->update->context, prevBitmap); offscreenCache->entries[index] = NULL; } From 63c81517b71c424a93d8f12d964c0fa185fbc47d Mon Sep 17 00:00:00 2001 From: weizhenwei Date: Fri, 9 Jun 2017 14:39:38 +0800 Subject: [PATCH 02/20] fix memory leak on update->window->monitored_desktop.windowIds which is realloced at update_read_desktop_actively_monitored_order() --- libfreerdp/core/update.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/libfreerdp/core/update.c b/libfreerdp/core/update.c index 6cc61873e..20c8bf710 100644 --- a/libfreerdp/core/update.c +++ b/libfreerdp/core/update.c @@ -2139,6 +2139,11 @@ void update_free(rdpUpdate* update) free(update->primary); free(update->secondary); free(update->altsec); + if (update->window->monitored_desktop.windowIds) + { + free(update->window->monitored_desktop.windowIds); + update->window->monitored_desktop.windowIds = NULL; + } free(update->window); MessageQueue_Free(update->queue); free(update); From 6a43fdc71af01e9f25a36d53b9762c8c21efb1e0 Mon Sep 17 00:00:00 2001 From: weizhenwei Date: Tue, 13 Jun 2017 14:32:36 +0800 Subject: [PATCH 03/20] code clean on free(update->window->monitored_desktop.windowIds); --- libfreerdp/core/update.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/libfreerdp/core/update.c b/libfreerdp/core/update.c index 20c8bf710..b449f120e 100644 --- a/libfreerdp/core/update.c +++ b/libfreerdp/core/update.c @@ -2139,11 +2139,7 @@ void update_free(rdpUpdate* update) free(update->primary); free(update->secondary); free(update->altsec); - if (update->window->monitored_desktop.windowIds) - { - free(update->window->monitored_desktop.windowIds); - update->window->monitored_desktop.windowIds = NULL; - } + free(update->window->monitored_desktop.windowIds); free(update->window); MessageQueue_Free(update->queue); free(update); From ccdaf15a7599b625a0cf1e6854bb93cf85382ac7 Mon Sep 17 00:00:00 2001 From: weizhenwei Date: Tue, 13 Jun 2017 14:54:58 +0800 Subject: [PATCH 04/20] fix memroy leak on rail_recv_exec_result_order() execResult --- channels/rail/client/rail_orders.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/channels/rail/client/rail_orders.c b/channels/rail/client/rail_orders.c index a7ef11da6..0a6abde77 100644 --- a/channels/rail/client/rail_orders.c +++ b/channels/rail/client/rail_orders.c @@ -650,7 +650,9 @@ UINT rail_order_recv(railPlugin* rail, wStream* s) case RDP_RAIL_ORDER_EXEC_RESULT: { RAIL_EXEC_RESULT_ORDER execResult; - return rail_recv_exec_result_order(rail, &execResult, s); + UINT ret = rail_recv_exec_result_order(rail, &execResult, s); + free(execResult.exeOrFile.string); + return ret; } case RDP_RAIL_ORDER_SYSPARAM: From 7e5b766f783f8d42f112074e101cfba23a628f18 Mon Sep 17 00:00:00 2001 From: weizhenwei Date: Tue, 13 Jun 2017 15:16:09 +0800 Subject: [PATCH 05/20] fix memory leak at rail_client_execute() --- channels/rail/client/rail_main.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/channels/rail/client/rail_main.c b/channels/rail/client/rail_main.c index b9c6a1383..97dd32a73 100644 --- a/channels/rail/client/rail_main.c +++ b/channels/rail/client/rail_main.c @@ -122,7 +122,11 @@ static UINT rail_client_execute(RailClientContext* context, &exec->workingDir); /* ShellWorkingDirectory */ rail_string_to_unicode_string(exec->RemoteApplicationArguments, &exec->arguments); /* RemoteApplicationCmdLine */ - return rail_send_client_exec_order(rail, exec); + UINT ret = rail_send_client_exec_order(rail, exec); + free(exec->exeOrFile.string); + free(exec->workingDir.string); + free(exec->arguments.string); + return ret; } /** From a2ecf403b2628f16f7670a17ecd83b7f558420dd Mon Sep 17 00:00:00 2001 From: weizhenwei Date: Tue, 13 Jun 2017 15:26:01 +0800 Subject: [PATCH 06/20] fix memory leak at drive_register_drive_path() and drive_free() --- channels/drive/client/drive_main.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/channels/drive/client/drive_main.c b/channels/drive/client/drive_main.c index 5bb089b94..f3911fec9 100644 --- a/channels/drive/client/drive_main.c +++ b/channels/drive/client/drive_main.c @@ -757,6 +757,7 @@ static UINT drive_free(DEVICE* device) ListDictionary_Free(drive->files); MessageQueue_Free(drive->IrpQueue); Stream_Free(drive->device.data, TRUE); + free(drive->path); free(drive); return error; } @@ -867,6 +868,7 @@ UINT drive_register_drive_path(PDEVICE_SERVICE_ENTRY_POINTS pEntryPoints, out_error: MessageQueue_Free(drive->IrpQueue); ListDictionary_Free(drive->files); + free(drive->path); free(drive); return error; } From 46b841be1350c88a354adaa5b46e99828477e23d Mon Sep 17 00:00:00 2001 From: weizhenwei Date: Tue, 13 Jun 2017 15:51:14 +0800 Subject: [PATCH 07/20] fix memory leak on rail_virtual_channel_event_data_received() -> StreamNew() --- channels/rail/client/rail_main.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/channels/rail/client/rail_main.c b/channels/rail/client/rail_main.c index 97dd32a73..e24e0fb0f 100644 --- a/channels/rail/client/rail_main.c +++ b/channels/rail/client/rail_main.c @@ -620,9 +620,11 @@ static void* rail_virtual_channel_client_thread(void* arg) if ((error = rail_order_recv(rail, data))) { + Stream_Free(data, TRUE); WLog_ERR(TAG, "rail_order_recv failed with error %"PRIu32"!", error); break; } + Stream_Free(data, TRUE); } } From 5c19318ab57f37d22df3197d1bed8d086faa4b99 Mon Sep 17 00:00:00 2001 From: weizhenwei Date: Tue, 13 Jun 2017 16:08:20 +0800 Subject: [PATCH 08/20] fix memory leak at update->window->window_state.titleInfo.string at update_read_window_state_order() --- libfreerdp/core/update.c | 1 + 1 file changed, 1 insertion(+) diff --git a/libfreerdp/core/update.c b/libfreerdp/core/update.c index b449f120e..0fc5a9151 100644 --- a/libfreerdp/core/update.c +++ b/libfreerdp/core/update.c @@ -2140,6 +2140,7 @@ void update_free(rdpUpdate* update) free(update->secondary); free(update->altsec); free(update->window->monitored_desktop.windowIds); + free(update->window->window_state.titleInfo.string); free(update->window); MessageQueue_Free(update->queue); free(update); From 2f96df25faa1e5551c302a6434c4af3dfd771085 Mon Sep 17 00:00:00 2001 From: weizhenwei Date: Tue, 13 Jun 2017 16:27:28 +0800 Subject: [PATCH 09/20] fix memory leak at update->window->window_state.windowRects/visibilityRects at update_read_window_state_order() --- libfreerdp/core/update.c | 2 ++ libfreerdp/core/window.c | 2 ++ 2 files changed, 4 insertions(+) diff --git a/libfreerdp/core/update.c b/libfreerdp/core/update.c index 0fc5a9151..84e4f8837 100644 --- a/libfreerdp/core/update.c +++ b/libfreerdp/core/update.c @@ -2141,6 +2141,8 @@ void update_free(rdpUpdate* update) free(update->altsec); free(update->window->monitored_desktop.windowIds); free(update->window->window_state.titleInfo.string); + free(update->window->window_state.windowRects); + free(update->window->window_state.visibilityRects); free(update->window); MessageQueue_Free(update->queue); free(update); diff --git a/libfreerdp/core/window.c b/libfreerdp/core/window.c index db5560a32..405e24b5e 100644 --- a/libfreerdp/core/window.c +++ b/libfreerdp/core/window.c @@ -294,6 +294,7 @@ BOOL update_read_window_state_order(wStream* s, WINDOW_ORDER_INFO* orderInfo, WI Stream_Read_UINT16(s, windowState->numWindowRects); /* numWindowRects (2 bytes) */ + free(windowState->windowRects); size = sizeof(RECTANGLE_16) * windowState->numWindowRects; windowState->windowRects = (RECTANGLE_16*) malloc(size); if (!windowState->windowRects) @@ -328,6 +329,7 @@ BOOL update_read_window_state_order(wStream* s, WINDOW_ORDER_INFO* orderInfo, WI Stream_Read_UINT16(s, windowState->numVisibilityRects); /* numVisibilityRects (2 bytes) */ + free(windowState->visibilityRects); size = sizeof(RECTANGLE_16) * windowState->numVisibilityRects; windowState->visibilityRects = (RECTANGLE_16*) malloc(size); if (!windowState->visibilityRects) From 64fce8717fd406004b6cf6d3b57606c794fcbc59 Mon Sep 17 00:00:00 2001 From: weizhenwei Date: Tue, 13 Jun 2017 16:46:37 +0800 Subject: [PATCH 10/20] fix memroy leak of fd at FindFirstFileW() --- winpr/libwinpr/file/generic.c | 1 + 1 file changed, 1 insertion(+) diff --git a/winpr/libwinpr/file/generic.c b/winpr/libwinpr/file/generic.c index 918d8a8e1..51daafe3d 100644 --- a/winpr/libwinpr/file/generic.c +++ b/winpr/libwinpr/file/generic.c @@ -938,6 +938,7 @@ HANDLE FindFirstFileW(LPCWSTR lpFileName, LPWIN32_FIND_DATAW lpFindFileData) } } + free(fd); return h; } From c39aecbdb449e557cac0fa980b306cc6d62145bc Mon Sep 17 00:00:00 2001 From: weizhenwei Date: Tue, 13 Jun 2017 16:49:51 +0800 Subject: [PATCH 11/20] fix memory leak of pKeys at xf_rail_invalidate_region() --- client/X11/xf_rail.c | 1 + 1 file changed, 1 insertion(+) diff --git a/client/X11/xf_rail.c b/client/X11/xf_rail.c index 4928175d5..d4fec546b 100644 --- a/client/X11/xf_rail.c +++ b/client/X11/xf_rail.c @@ -226,6 +226,7 @@ void xf_rail_invalidate_region(xfContext* xfc, REGION16* invalidRegion) } } + free(pKeys); region16_uninit(&windowInvalidRegion); } From d77802d5e9eb6dbbaab4df304590f83218d4a57a Mon Sep 17 00:00:00 2001 From: weizhenwei Date: Tue, 13 Jun 2017 17:12:20 +0800 Subject: [PATCH 12/20] fix memroy leak of window_icon->iconInfo at update_read_window_icon_order() --- libfreerdp/core/update.c | 7 +++++++ libfreerdp/core/window.c | 8 ++++++++ 2 files changed, 15 insertions(+) diff --git a/libfreerdp/core/update.c b/libfreerdp/core/update.c index 84e4f8837..659531d84 100644 --- a/libfreerdp/core/update.c +++ b/libfreerdp/core/update.c @@ -2143,6 +2143,13 @@ void update_free(rdpUpdate* update) free(update->window->window_state.titleInfo.string); free(update->window->window_state.windowRects); free(update->window->window_state.visibilityRects); + if (update->window->window_icon.iconInfo) + { + free(update->window->window_icon.iconInfo->bitsColor); + free(update->window->window_icon.iconInfo->bitsMask); + free(update->window->window_icon.iconInfo->colorTable); + free(update->window->window_icon.iconInfo); + } free(update->window); MessageQueue_Free(update->queue); free(update); diff --git a/libfreerdp/core/window.c b/libfreerdp/core/window.c index 405e24b5e..c70cf6eb9 100644 --- a/libfreerdp/core/window.c +++ b/libfreerdp/core/window.c @@ -352,6 +352,14 @@ BOOL update_read_window_state_order(wStream* s, WINDOW_ORDER_INFO* orderInfo, WI BOOL update_read_window_icon_order(wStream* s, WINDOW_ORDER_INFO* orderInfo, WINDOW_ICON_ORDER* window_icon) { + if (window_icon->iconInfo) + { + free(window_icon->iconInfo->bitsColor); + free(window_icon->iconInfo->bitsMask); + free(window_icon->iconInfo->colorTable); + free(window_icon->iconInfo); + } + window_icon->iconInfo = (ICON_INFO*) calloc(1, sizeof(ICON_INFO)); if (!window_icon->iconInfo) return FALSE; From bcd8ddef5903f78be57106688ad3cb9d00981213 Mon Sep 17 00:00:00 2001 From: weizhenwei Date: Wed, 21 Jun 2017 15:43:41 +0800 Subject: [PATCH 13/20] fix compile error on win64.vs2010 platform --- channels/rail/client/rail_main.c | 5 +++-- channels/rail/client/rail_orders.c | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/channels/rail/client/rail_main.c b/channels/rail/client/rail_main.c index e24e0fb0f..ffcdcd65e 100644 --- a/channels/rail/client/rail_main.c +++ b/channels/rail/client/rail_main.c @@ -104,6 +104,7 @@ static UINT rail_client_execute(RailClientContext* context, RAIL_EXEC_ORDER* exec) { char* exeOrFile; + UINT error; railPlugin* rail = (railPlugin*) context->handle; exeOrFile = exec->RemoteApplicationProgram; @@ -122,11 +123,11 @@ static UINT rail_client_execute(RailClientContext* context, &exec->workingDir); /* ShellWorkingDirectory */ rail_string_to_unicode_string(exec->RemoteApplicationArguments, &exec->arguments); /* RemoteApplicationCmdLine */ - UINT ret = rail_send_client_exec_order(rail, exec); + error = rail_send_client_exec_order(rail, exec); free(exec->exeOrFile.string); free(exec->workingDir.string); free(exec->arguments.string); - return ret; + return error; } /** diff --git a/channels/rail/client/rail_orders.c b/channels/rail/client/rail_orders.c index 0a6abde77..0cb6832a8 100644 --- a/channels/rail/client/rail_orders.c +++ b/channels/rail/client/rail_orders.c @@ -650,9 +650,9 @@ UINT rail_order_recv(railPlugin* rail, wStream* s) case RDP_RAIL_ORDER_EXEC_RESULT: { RAIL_EXEC_RESULT_ORDER execResult; - UINT ret = rail_recv_exec_result_order(rail, &execResult, s); + error = rail_recv_exec_result_order(rail, &execResult, s); free(execResult.exeOrFile.string); - return ret; + return error; } case RDP_RAIL_ORDER_SYSPARAM: From 2d56e22e9eff85afc03f165715620644be5f8760 Mon Sep 17 00:00:00 2001 From: weizhenwei Date: Wed, 21 Jun 2017 22:07:07 +0800 Subject: [PATCH 14/20] refactor on redundant code copy --- channels/rail/client/rail_main.c | 15 ++++++++++++--- libfreerdp/core/update.c | 16 +++++++++------- libfreerdp/core/window.c | 13 +++++++++---- libfreerdp/core/window.h | 1 + 4 files changed, 31 insertions(+), 14 deletions(-) diff --git a/channels/rail/client/rail_main.c b/channels/rail/client/rail_main.c index ffcdcd65e..cfdf7bd0e 100644 --- a/channels/rail/client/rail_main.c +++ b/channels/rail/client/rail_main.c @@ -91,6 +91,17 @@ UINT rail_send_channel_data(railPlugin* rail, void* data, size_t length) return rail_send(rail, s); } +/** + * used by rail_client_execute() to free RAIL_EXEC_ORDER's + * internal malloced memory; + */ +static void rail_client_clean_exec_order(RAIL_EXEC_ORDER* exec) +{ + free(exec->exeOrFile.string); + free(exec->workingDir.string); + free(exec->arguments.string); +} + /** * Callback Interface */ @@ -124,9 +135,7 @@ static UINT rail_client_execute(RailClientContext* context, rail_string_to_unicode_string(exec->RemoteApplicationArguments, &exec->arguments); /* RemoteApplicationCmdLine */ error = rail_send_client_exec_order(rail, exec); - free(exec->exeOrFile.string); - free(exec->workingDir.string); - free(exec->arguments.string); + rail_client_clean_exec_order(exec); return error; } diff --git a/libfreerdp/core/update.c b/libfreerdp/core/update.c index 659531d84..d7f32f569 100644 --- a/libfreerdp/core/update.c +++ b/libfreerdp/core/update.c @@ -2041,6 +2041,13 @@ static void update_free_queued_message(void* obj) update_message_queue_free_message(msg); } +static void update_free_window_state(WINDOW_STATE_ORDER window_state) +{ + free(window_state.titleInfo.string); + free(window_state.windowRects); + free(window_state.visibilityRects); +} + rdpUpdate* update_new(rdpRdp* rdp) { const wObject cb = { NULL, NULL, NULL, update_free_queued_message, NULL }; @@ -2140,15 +2147,10 @@ void update_free(rdpUpdate* update) free(update->secondary); free(update->altsec); free(update->window->monitored_desktop.windowIds); - free(update->window->window_state.titleInfo.string); - free(update->window->window_state.windowRects); - free(update->window->window_state.visibilityRects); + update_free_window_state(update->window->window_state); if (update->window->window_icon.iconInfo) { - free(update->window->window_icon.iconInfo->bitsColor); - free(update->window->window_icon.iconInfo->bitsMask); - free(update->window->window_icon.iconInfo->colorTable); - free(update->window->window_icon.iconInfo); + update_free_window_icon_info(update->window->window_icon.iconInfo); } free(update->window); MessageQueue_Free(update->queue); diff --git a/libfreerdp/core/window.c b/libfreerdp/core/window.c index c70cf6eb9..7e1ed7cba 100644 --- a/libfreerdp/core/window.c +++ b/libfreerdp/core/window.c @@ -354,10 +354,7 @@ BOOL update_read_window_icon_order(wStream* s, WINDOW_ORDER_INFO* orderInfo, WIN { if (window_icon->iconInfo) { - free(window_icon->iconInfo->bitsColor); - free(window_icon->iconInfo->bitsMask); - free(window_icon->iconInfo->colorTable); - free(window_icon->iconInfo); + update_free_window_icon_info(window_icon->iconInfo); } window_icon->iconInfo = (ICON_INFO*) calloc(1, sizeof(ICON_INFO)); @@ -591,6 +588,14 @@ BOOL update_recv_desktop_info_order(rdpUpdate* update, wStream* s, WINDOW_ORDER_ return result; } +void update_free_window_icon_info(ICON_INFO* iconInfo) +{ + free(iconInfo->bitsColor); + free(iconInfo->bitsMask); + free(iconInfo->colorTable); + free(iconInfo); +} + BOOL update_recv_altsec_window_order(rdpUpdate* update, wStream* s) { UINT16 orderSize; diff --git a/libfreerdp/core/window.h b/libfreerdp/core/window.h index 4fdf4bad3..e9e173c93 100644 --- a/libfreerdp/core/window.h +++ b/libfreerdp/core/window.h @@ -27,6 +27,7 @@ #include #include +FREERDP_LOCAL void update_free_window_icon_info(ICON_INFO* iconInfo); FREERDP_LOCAL BOOL update_recv_altsec_window_order(rdpUpdate* update, wStream* s); From fa1c65b65667872bf3dd325652a1c6c387ae91c7 Mon Sep 17 00:00:00 2001 From: weizhenwei Date: Thu, 22 Jun 2017 10:21:20 +0800 Subject: [PATCH 15/20] refactor to remove duplicate code and replace free+malloc with realloc --- channels/rail/client/rail_main.c | 6 +++--- libfreerdp/core/window.c | 21 +++++++++++++++------ winpr/libwinpr/file/generic.c | 5 +++-- 3 files changed, 21 insertions(+), 11 deletions(-) diff --git a/channels/rail/client/rail_main.c b/channels/rail/client/rail_main.c index cfdf7bd0e..061a3a9f3 100644 --- a/channels/rail/client/rail_main.c +++ b/channels/rail/client/rail_main.c @@ -628,13 +628,13 @@ static void* rail_virtual_channel_client_thread(void* arg) { data = (wStream*) message.wParam; - if ((error = rail_order_recv(rail, data))) + error = rail_order_recv(rail, data); + Stream_Free(data, TRUE); + if (error) { - Stream_Free(data, TRUE); WLog_ERR(TAG, "rail_order_recv failed with error %"PRIu32"!", error); break; } - Stream_Free(data, TRUE); } } diff --git a/libfreerdp/core/window.c b/libfreerdp/core/window.c index 7e1ed7cba..3626fe407 100644 --- a/libfreerdp/core/window.c +++ b/libfreerdp/core/window.c @@ -194,6 +194,7 @@ BOOL update_read_window_state_order(wStream* s, WINDOW_ORDER_INFO* orderInfo, WI { int i; int size; + RECTANGLE_16* newRect; if (orderInfo->fieldFlags & WINDOW_ORDER_FIELD_OWNER) { @@ -294,11 +295,15 @@ BOOL update_read_window_state_order(wStream* s, WINDOW_ORDER_INFO* orderInfo, WI Stream_Read_UINT16(s, windowState->numWindowRects); /* numWindowRects (2 bytes) */ - free(windowState->windowRects); size = sizeof(RECTANGLE_16) * windowState->numWindowRects; - windowState->windowRects = (RECTANGLE_16*) malloc(size); - if (!windowState->windowRects) + newRect = (RECTANGLE_16*)realloc(windowState->windowRects, size); + if (!newRect) + { + free(windowState->windowRects); + windowState->windowRects = NULL; return FALSE; + } + windowState->windowRects = newRect; if (Stream_GetRemainingLength(s) < 8 * windowState->numWindowRects) return FALSE; @@ -329,11 +334,15 @@ BOOL update_read_window_state_order(wStream* s, WINDOW_ORDER_INFO* orderInfo, WI Stream_Read_UINT16(s, windowState->numVisibilityRects); /* numVisibilityRects (2 bytes) */ - free(windowState->visibilityRects); size = sizeof(RECTANGLE_16) * windowState->numVisibilityRects; - windowState->visibilityRects = (RECTANGLE_16*) malloc(size); - if (!windowState->visibilityRects) + newRect = (RECTANGLE_16*)realloc(windowState->visibilityRects, size); + if (!newRect) + { + free(windowState->visibilityRects); + windowState->visibilityRects = NULL; return FALSE; + } + windowState->visibilityRects = newRect; if (Stream_GetRemainingLength(s) < windowState->numVisibilityRects * 8) return FALSE; diff --git a/winpr/libwinpr/file/generic.c b/winpr/libwinpr/file/generic.c index 51daafe3d..004656a8b 100644 --- a/winpr/libwinpr/file/generic.c +++ b/winpr/libwinpr/file/generic.c @@ -933,11 +933,12 @@ HANDLE FindFirstFileW(LPCWSTR lpFileName, LPWIN32_FIND_DATAW lpFindFileData) if (!ConvertFindDataAToW(fd, lpFindFileData)) { SetLastError(ERROR_NOT_ENOUGH_MEMORY); - free(fd); - return INVALID_HANDLE_VALUE; + h = INVALID_HANDLE_VALUE; + goto out; } } +out: free(fd); return h; } From 61b24bf0b34865c1665d93f3929dd20f4b28e4b0 Mon Sep 17 00:00:00 2001 From: weizhenwei Date: Thu, 22 Jun 2017 17:53:51 +0800 Subject: [PATCH 16/20] add NULL pointer check and set freed pointers to NULL afterward --- channels/drive/client/drive_main.c | 2 +- channels/rail/client/rail_main.c | 23 ++++++++++++++++++++--- libfreerdp/core/update.c | 28 +++++++++++++++++++++++----- libfreerdp/core/window.c | 22 +++++++++++++++++++--- 4 files changed, 63 insertions(+), 12 deletions(-) diff --git a/channels/drive/client/drive_main.c b/channels/drive/client/drive_main.c index f3911fec9..480a661b8 100644 --- a/channels/drive/client/drive_main.c +++ b/channels/drive/client/drive_main.c @@ -868,7 +868,7 @@ UINT drive_register_drive_path(PDEVICE_SERVICE_ENTRY_POINTS pEntryPoints, out_error: MessageQueue_Free(drive->IrpQueue); ListDictionary_Free(drive->files); - free(drive->path); + drive_free(drive); free(drive); return error; } diff --git a/channels/rail/client/rail_main.c b/channels/rail/client/rail_main.c index 061a3a9f3..08b73bae0 100644 --- a/channels/rail/client/rail_main.c +++ b/channels/rail/client/rail_main.c @@ -97,9 +97,26 @@ UINT rail_send_channel_data(railPlugin* rail, void* data, size_t length) */ static void rail_client_clean_exec_order(RAIL_EXEC_ORDER* exec) { - free(exec->exeOrFile.string); - free(exec->workingDir.string); - free(exec->arguments.string); + if (!exec) + return; + + if (exec->exeOrFile.string) + { + free(exec->exeOrFile.string); + exec->exeOrFile.string = NULL; + } + + if (exec->workingDir.string) + { + free(exec->workingDir.string); + exec->workingDir.string = NULL; + } + + if (exec->arguments.string) + { + free(exec->arguments.string); + exec->arguments.string = NULL; + } } /** diff --git a/libfreerdp/core/update.c b/libfreerdp/core/update.c index d7f32f569..9e0ba99a9 100644 --- a/libfreerdp/core/update.c +++ b/libfreerdp/core/update.c @@ -2041,11 +2041,28 @@ static void update_free_queued_message(void* obj) update_message_queue_free_message(msg); } -static void update_free_window_state(WINDOW_STATE_ORDER window_state) +static void update_free_window_state(WINDOW_STATE_ORDER* window_state) { - free(window_state.titleInfo.string); - free(window_state.windowRects); - free(window_state.visibilityRects); + if (!window_state) + return; + + if (window_state->titleInfo.string) + { + free(window_state->titleInfo.string); + window_state->titleInfo.string = NULL; + } + + if (window_state->windowRects) + { + free(window_state->windowRects); + window_state->windowRects = NULL; + } + + if (window_state->visibilityRects) + { + free(window_state->visibilityRects); + window_state->visibilityRects = NULL; + } } rdpUpdate* update_new(rdpRdp* rdp) @@ -2147,10 +2164,11 @@ void update_free(rdpUpdate* update) free(update->secondary); free(update->altsec); free(update->window->monitored_desktop.windowIds); - update_free_window_state(update->window->window_state); + update_free_window_state(&update->window->window_state); if (update->window->window_icon.iconInfo) { update_free_window_icon_info(update->window->window_icon.iconInfo); + update->window->window_icon.iconInfo = NULL; } free(update->window); MessageQueue_Free(update->queue); diff --git a/libfreerdp/core/window.c b/libfreerdp/core/window.c index 3626fe407..41970db26 100644 --- a/libfreerdp/core/window.c +++ b/libfreerdp/core/window.c @@ -364,6 +364,7 @@ BOOL update_read_window_icon_order(wStream* s, WINDOW_ORDER_INFO* orderInfo, WIN if (window_icon->iconInfo) { update_free_window_icon_info(window_icon->iconInfo); + window_icon->iconInfo = NULL; } window_icon->iconInfo = (ICON_INFO*) calloc(1, sizeof(ICON_INFO)); @@ -599,9 +600,24 @@ BOOL update_recv_desktop_info_order(rdpUpdate* update, wStream* s, WINDOW_ORDER_ void update_free_window_icon_info(ICON_INFO* iconInfo) { - free(iconInfo->bitsColor); - free(iconInfo->bitsMask); - free(iconInfo->colorTable); + if (iconInfo->bitsColor) + { + free(iconInfo->bitsColor); + iconInfo->bitsColor = NULL; + } + + if (iconInfo->bitsMask) + { + free(iconInfo->bitsMask); + iconInfo->bitsMask = NULL; + } + + if (iconInfo->colorTable) + { + free(iconInfo->colorTable); + iconInfo->colorTable = NULL; + } + free(iconInfo); } From 3b52a60d3154485b4e411223bc26db40fe43dced Mon Sep 17 00:00:00 2001 From: weizhenwei Date: Fri, 23 Jun 2017 09:21:16 +0800 Subject: [PATCH 17/20] remove useless NULL pointer check before free --- channels/drive/client/drive_main.c | 1 - channels/rail/client/rail_main.c | 21 ++++++--------------- libfreerdp/core/update.c | 21 ++++++--------------- libfreerdp/core/window.c | 24 +++++++++--------------- 4 files changed, 21 insertions(+), 46 deletions(-) diff --git a/channels/drive/client/drive_main.c b/channels/drive/client/drive_main.c index 480a661b8..a47d001f0 100644 --- a/channels/drive/client/drive_main.c +++ b/channels/drive/client/drive_main.c @@ -869,7 +869,6 @@ out_error: MessageQueue_Free(drive->IrpQueue); ListDictionary_Free(drive->files); drive_free(drive); - free(drive); return error; } diff --git a/channels/rail/client/rail_main.c b/channels/rail/client/rail_main.c index 08b73bae0..6ecaf7a30 100644 --- a/channels/rail/client/rail_main.c +++ b/channels/rail/client/rail_main.c @@ -100,23 +100,14 @@ static void rail_client_clean_exec_order(RAIL_EXEC_ORDER* exec) if (!exec) return; - if (exec->exeOrFile.string) - { - free(exec->exeOrFile.string); - exec->exeOrFile.string = NULL; - } + free(exec->exeOrFile.string); + exec->exeOrFile.string = NULL; - if (exec->workingDir.string) - { - free(exec->workingDir.string); - exec->workingDir.string = NULL; - } + free(exec->workingDir.string); + exec->workingDir.string = NULL; - if (exec->arguments.string) - { - free(exec->arguments.string); - exec->arguments.string = NULL; - } + free(exec->arguments.string); + exec->arguments.string = NULL; } /** diff --git a/libfreerdp/core/update.c b/libfreerdp/core/update.c index 9e0ba99a9..e3064f302 100644 --- a/libfreerdp/core/update.c +++ b/libfreerdp/core/update.c @@ -2046,23 +2046,14 @@ static void update_free_window_state(WINDOW_STATE_ORDER* window_state) if (!window_state) return; - if (window_state->titleInfo.string) - { - free(window_state->titleInfo.string); - window_state->titleInfo.string = NULL; - } + free(window_state->titleInfo.string); + window_state->titleInfo.string = NULL; - if (window_state->windowRects) - { - free(window_state->windowRects); - window_state->windowRects = NULL; - } + free(window_state->windowRects); + window_state->windowRects = NULL; - if (window_state->visibilityRects) - { - free(window_state->visibilityRects); - window_state->visibilityRects = NULL; - } + free(window_state->visibilityRects); + window_state->visibilityRects = NULL; } rdpUpdate* update_new(rdpRdp* rdp) diff --git a/libfreerdp/core/window.c b/libfreerdp/core/window.c index 41970db26..8153c0720 100644 --- a/libfreerdp/core/window.c +++ b/libfreerdp/core/window.c @@ -600,23 +600,17 @@ BOOL update_recv_desktop_info_order(rdpUpdate* update, wStream* s, WINDOW_ORDER_ void update_free_window_icon_info(ICON_INFO* iconInfo) { - if (iconInfo->bitsColor) - { - free(iconInfo->bitsColor); - iconInfo->bitsColor = NULL; - } + if (!iconInfo) + return; - if (iconInfo->bitsMask) - { - free(iconInfo->bitsMask); - iconInfo->bitsMask = NULL; - } + free(iconInfo->bitsColor); + iconInfo->bitsColor = NULL; - if (iconInfo->colorTable) - { - free(iconInfo->colorTable); - iconInfo->colorTable = NULL; - } + free(iconInfo->bitsMask); + iconInfo->bitsMask = NULL; + + free(iconInfo->colorTable); + iconInfo->colorTable = NULL; free(iconInfo); } From bd94dcc6d57b43078832623422b0e59d48f6f3e5 Mon Sep 17 00:00:00 2001 From: weizhenwei Date: Fri, 23 Jun 2017 09:28:42 +0800 Subject: [PATCH 18/20] remove duplicate code when freeing drive --- channels/drive/client/drive_main.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/channels/drive/client/drive_main.c b/channels/drive/client/drive_main.c index a47d001f0..48e5b97c2 100644 --- a/channels/drive/client/drive_main.c +++ b/channels/drive/client/drive_main.c @@ -866,8 +866,6 @@ UINT drive_register_drive_path(PDEVICE_SERVICE_ENTRY_POINTS pEntryPoints, return CHANNEL_RC_OK; out_error: - MessageQueue_Free(drive->IrpQueue); - ListDictionary_Free(drive->files); drive_free(drive); return error; } From 5d8d3b53c586df708a28cc3efd7b3d0135af3bbe Mon Sep 17 00:00:00 2001 From: weizhenwei Date: Fri, 23 Jun 2017 09:44:40 +0800 Subject: [PATCH 19/20] remove redundant NULL pointer check --- libfreerdp/core/update.c | 6 +----- libfreerdp/core/window.c | 7 +------ 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/libfreerdp/core/update.c b/libfreerdp/core/update.c index e3064f302..5478be927 100644 --- a/libfreerdp/core/update.c +++ b/libfreerdp/core/update.c @@ -2156,11 +2156,7 @@ void update_free(rdpUpdate* update) free(update->altsec); free(update->window->monitored_desktop.windowIds); update_free_window_state(&update->window->window_state); - if (update->window->window_icon.iconInfo) - { - update_free_window_icon_info(update->window->window_icon.iconInfo); - update->window->window_icon.iconInfo = NULL; - } + update_free_window_icon_info(update->window->window_icon.iconInfo); free(update->window); MessageQueue_Free(update->queue); free(update); diff --git a/libfreerdp/core/window.c b/libfreerdp/core/window.c index 8153c0720..f1ef234c4 100644 --- a/libfreerdp/core/window.c +++ b/libfreerdp/core/window.c @@ -361,12 +361,7 @@ BOOL update_read_window_state_order(wStream* s, WINDOW_ORDER_INFO* orderInfo, WI BOOL update_read_window_icon_order(wStream* s, WINDOW_ORDER_INFO* orderInfo, WINDOW_ICON_ORDER* window_icon) { - if (window_icon->iconInfo) - { - update_free_window_icon_info(window_icon->iconInfo); - window_icon->iconInfo = NULL; - } - + update_free_window_icon_info(window_icon->iconInfo); window_icon->iconInfo = (ICON_INFO*) calloc(1, sizeof(ICON_INFO)); if (!window_icon->iconInfo) return FALSE; From ef540ee2dfc60ffab376fe51adc879b4433588d5 Mon Sep 17 00:00:00 2001 From: weizhenwei Date: Fri, 23 Jun 2017 09:50:56 +0800 Subject: [PATCH 20/20] code format adjustment --- libfreerdp/core/update.c | 2 +- libfreerdp/core/window.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/libfreerdp/core/update.c b/libfreerdp/core/update.c index 5478be927..b5a742f55 100644 --- a/libfreerdp/core/update.c +++ b/libfreerdp/core/update.c @@ -2156,7 +2156,7 @@ void update_free(rdpUpdate* update) free(update->altsec); free(update->window->monitored_desktop.windowIds); update_free_window_state(&update->window->window_state); - update_free_window_icon_info(update->window->window_icon.iconInfo); + update_free_window_icon_info(update->window->window_icon.iconInfo); free(update->window); MessageQueue_Free(update->queue); free(update); diff --git a/libfreerdp/core/window.c b/libfreerdp/core/window.c index f1ef234c4..eb487e682 100644 --- a/libfreerdp/core/window.c +++ b/libfreerdp/core/window.c @@ -361,7 +361,7 @@ BOOL update_read_window_state_order(wStream* s, WINDOW_ORDER_INFO* orderInfo, WI BOOL update_read_window_icon_order(wStream* s, WINDOW_ORDER_INFO* orderInfo, WINDOW_ICON_ORDER* window_icon) { - update_free_window_icon_info(window_icon->iconInfo); + update_free_window_icon_info(window_icon->iconInfo); window_icon->iconInfo = (ICON_INFO*) calloc(1, sizeof(ICON_INFO)); if (!window_icon->iconInfo) return FALSE;