From eecc4b189f31e1ca41982f16adcf586ec07c6450 Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Mon, 10 Oct 2016 10:38:54 +0200 Subject: [PATCH] Fixed leak in bitmap handling. --- libfreerdp/cache/bitmap.c | 62 ++++++++++++++++-------------------- libfreerdp/cache/offscreen.c | 6 ++-- libfreerdp/core/graphics.c | 6 ++-- libfreerdp/core/graphics.h | 30 +++++++++++++++++ libfreerdp/gdi/gdi.c | 9 +++--- 5 files changed, 70 insertions(+), 43 deletions(-) create mode 100644 libfreerdp/core/graphics.h diff --git a/libfreerdp/cache/bitmap.c b/libfreerdp/cache/bitmap.c index dd5b93694..bf6c6f8df 100644 --- a/libfreerdp/cache/bitmap.c +++ b/libfreerdp/cache/bitmap.c @@ -34,14 +34,19 @@ #include #include "../gdi/gdi.h" +#include "../core/graphics.h" #define TAG FREERDP_TAG("cache.bitmap") static rdpBitmap* bitmap_cache_get(rdpBitmapCache* bitmapCache, UINT32 id, UINT32 index); -static void bitmap_cache_put(rdpBitmapCache* bitmap_cache, UINT32 id, +static BOOL bitmap_cache_put(rdpBitmapCache* bitmap_cache, UINT32 id, UINT32 index, rdpBitmap* bitmap); +static void bitmap_free(rdpContext* context, rdpBitmap* bitmap) +{ +} + static BOOL update_gdi_memblt(rdpContext* context, MEMBLT_ORDER* memblt) { @@ -119,25 +124,22 @@ static BOOL update_gdi_cache_bitmap(rdpContext* context, cacheBitmap->bitmapBpp, cacheBitmap->bitmapLength, cacheBitmap->compressed, RDP_CODEC_ID_NONE)) { - bitmap->Free(context, bitmap); + Bitmap_Free(context, bitmap); return FALSE; } if (!bitmap->New(context, bitmap)) { - bitmap->Free(context, bitmap); + Bitmap_Free(context, bitmap); return FALSE; } prevBitmap = bitmap_cache_get(cache->bitmap, cacheBitmap->cacheId, cacheBitmap->cacheIndex); - - if (prevBitmap != NULL) - prevBitmap->Free(context, prevBitmap); - - bitmap_cache_put(cache->bitmap, cacheBitmap->cacheId, cacheBitmap->cacheIndex, - bitmap); - return TRUE; + Bitmap_Free(context, prevBitmap); + return bitmap_cache_put(cache->bitmap, cacheBitmap->cacheId, + cacheBitmap->cacheIndex, + bitmap); } static BOOL update_gdi_cache_bitmap_v2(rdpContext* context, @@ -171,7 +173,7 @@ static BOOL update_gdi_cache_bitmap_v2(rdpContext* context, cacheBitmapV2->compressed, RDP_CODEC_ID_NONE)) { - bitmap->Free(context, bitmap); + Bitmap_Free(context, bitmap); return FALSE; } @@ -180,16 +182,13 @@ static BOOL update_gdi_cache_bitmap_v2(rdpContext* context, if (!bitmap->New(context, bitmap)) { - bitmap->Free(context, bitmap); + Bitmap_Free(context, bitmap); return FALSE; } - if (prevBitmap) - prevBitmap->Free(context, prevBitmap); - - bitmap_cache_put(cache->bitmap, cacheBitmapV2->cacheId, - cacheBitmapV2->cacheIndex, bitmap); - return TRUE; + Bitmap_Free(context, prevBitmap); + return bitmap_cache_put(cache->bitmap, cacheBitmapV2->cacheId, + cacheBitmapV2->cacheIndex, bitmap); } static BOOL update_gdi_cache_bitmap_v3(rdpContext* context, @@ -217,25 +216,21 @@ static BOOL update_gdi_cache_bitmap_v3(rdpContext* context, bitmapData->bpp, bitmapData->length, compressed, bitmapData->codecID)) { - bitmap->Free(context, bitmap); + Bitmap_Free(context, bitmap); return FALSE; } if (!bitmap->New(context, bitmap)) { - bitmap->Free(context, bitmap); + Bitmap_Free(context, bitmap); return FALSE; } prevBitmap = bitmap_cache_get(cache->bitmap, cacheBitmapV3->cacheId, cacheBitmapV3->cacheIndex); - - if (prevBitmap) - prevBitmap->Free(context, prevBitmap); - - bitmap_cache_put(cache->bitmap, cacheBitmapV3->cacheId, - cacheBitmapV3->cacheIndex, bitmap); - return TRUE; + Bitmap_Free(context, prevBitmap); + return bitmap_cache_put(cache->bitmap, cacheBitmapV3->cacheId, + cacheBitmapV3->cacheIndex, bitmap); } rdpBitmap* bitmap_cache_get(rdpBitmapCache* bitmapCache, UINT32 id, @@ -263,13 +258,13 @@ rdpBitmap* bitmap_cache_get(rdpBitmapCache* bitmapCache, UINT32 id, return bitmap; } -void bitmap_cache_put(rdpBitmapCache* bitmapCache, UINT32 id, UINT32 index, +BOOL bitmap_cache_put(rdpBitmapCache* bitmapCache, UINT32 id, UINT32 index, rdpBitmap* bitmap) { if (id > bitmapCache->maxCells) { WLog_ERR(TAG, "put invalid bitmap cell id: %d", id); - return; + return FALSE; } if (index == BITMAP_CACHE_WAITING_LIST_INDEX) @@ -279,10 +274,11 @@ void bitmap_cache_put(rdpBitmapCache* bitmapCache, UINT32 id, UINT32 index, else if (index > bitmapCache->cells[id].number) { WLog_ERR(TAG, "put invalid bitmap index %d in cell id: %d", index, id); - return; + return FALSE; } bitmapCache->cells[id].entries[index] = bitmap; + return TRUE; } void bitmap_cache_register_callbacks(rdpUpdate* update) @@ -335,7 +331,7 @@ fail: { for (i = 0; i < (int) bitmapCache->maxCells; i++) free(bitmapCache->cells[i].entries); - } + } free(bitmapCache); return NULL; @@ -353,9 +349,7 @@ void bitmap_cache_free(rdpBitmapCache* bitmapCache) for (j = 0; j < (int) bitmapCache->cells[i].number + 1; j++) { bitmap = bitmapCache->cells[i].entries[j]; - - if (bitmap) - bitmap->Free(bitmapCache->context, bitmap); + Bitmap_Free(bitmapCache->context, bitmap); } free(bitmapCache->cells[i].entries); diff --git a/libfreerdp/cache/offscreen.c b/libfreerdp/cache/offscreen.c index 13c47e7ed..2321f7f09 100644 --- a/libfreerdp/cache/offscreen.c +++ b/libfreerdp/cache/offscreen.c @@ -30,6 +30,8 @@ #include #include +#include "../core/graphics.h" + #define TAG FREERDP_TAG("cache.offscreen") static void offscreen_cache_put(rdpOffscreenCache* offscreen_cache, @@ -188,9 +190,7 @@ void offscreen_cache_free(rdpOffscreenCache* offscreenCache) for (i = 0; i < (int) offscreenCache->maxEntries; i++) { bitmap = offscreenCache->entries[i]; - - if (bitmap) - bitmap->Free(offscreenCache->update->context, bitmap); + Bitmap_Free(offscreenCache->update->context, bitmap); } free(offscreenCache->entries); diff --git a/libfreerdp/core/graphics.c b/libfreerdp/core/graphics.c index f5a683b5f..fffe2ee54 100644 --- a/libfreerdp/core/graphics.c +++ b/libfreerdp/core/graphics.c @@ -25,6 +25,8 @@ #include +#include "graphics.h" + /* Bitmap Class */ rdpBitmap* Bitmap_Alloc(rdpContext* context) @@ -52,7 +54,7 @@ static BOOL Bitmap_New(rdpContext* context, rdpBitmap* bitmap) return TRUE; } -static void Bitmap_Free(rdpContext* context, rdpBitmap* bitmap) +void Bitmap_Free(rdpContext* context, rdpBitmap* bitmap) { if (bitmap) { @@ -204,7 +206,7 @@ rdpGraphics* graphics_new(rdpContext* context) graphics->Bitmap_Prototype->size = sizeof(rdpBitmap); graphics->Bitmap_Prototype->New = Bitmap_New; - graphics->Bitmap_Prototype->Free = Bitmap_Free; + graphics->Bitmap_Prototype->Free = NULL; graphics->Pointer_Prototype = (rdpPointer*) calloc(1, sizeof(rdpPointer)); if (!graphics->Pointer_Prototype) diff --git a/libfreerdp/core/graphics.h b/libfreerdp/core/graphics.h new file mode 100644 index 000000000..65b94441d --- /dev/null +++ b/libfreerdp/core/graphics.h @@ -0,0 +1,30 @@ +/** + * FreeRDP: A Remote Desktop Protocol Implementation + * Client Channels + * + * Copyright 2016 Armin Novak + * Copyright 2016 Thinast Technologies GmbH + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef FREERDP_CORE_GRAPHICS_H +#define FREERDP_CORE_GRAPHICS_H + +#include +#include +#include + +FREERDP_LOCAL void Bitmap_Free(rdpContext* context, rdpBitmap* bitmap); + +#endif /* FREERDP_CORE_GRAPHICS_H */ diff --git a/libfreerdp/gdi/gdi.c b/libfreerdp/gdi/gdi.c index 7d638ff6b..152afa57e 100644 --- a/libfreerdp/gdi/gdi.c +++ b/libfreerdp/gdi/gdi.c @@ -43,6 +43,7 @@ #include "brush.h" #include "line.h" #include "gdi.h" +#include "../core/graphics.h" #define TAG FREERDP_TAG("gdi") @@ -502,23 +503,23 @@ BOOL gdi_bitmap_update(rdpContext* context, bitmap->bitmapLength, bitmap->compressed, RDP_CODEC_ID_NONE)) { - bmp->Free(context, bmp); + Bitmap_Free(context, bmp); return FALSE; } if (!bmp->New(context, bmp)) { - bmp->Free(context, bmp); + Bitmap_Free(context, bmp); return FALSE; } if (!bmp->Paint(context, bmp)) { - bmp->Free(context, bmp); + Bitmap_Free(context, bmp); return FALSE; } - bmp->Free(context, bmp); + Bitmap_Free(context, bmp); } return TRUE;