mirrors/FreeRDP
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

[gdi,gfx] Fixed possible memory leaks

Browse Source 
* WINPR_ASSERT all callbacks required to be set
* Unify cache slot creation/destruction
* Destroy cache slot before setting it
This commit is contained in:
akallabeth 2022-11-18 11:32:33 +01:00 committed by akallabeth
parent 7faf13a811
commit febc4b3073
2 changed files with 122 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
channels/rdpgfx/client/rdpgfx_main.c
View File

@ -82,7 +82,7 @@ static void evict_cache_slots(RdpgfxClientContext* context, UINT16 MaxCacheSlots
{
if (CacheSlots[index])
{
RDPGFX_EVICT_CACHE_ENTRY_PDU pdu;
RDPGFX_EVICT_CACHE_ENTRY_PDU pdu = { 0 };
pdu.cacheSlot = (UINT16)index + 1;
if (context && context->EvictCacheEntry)

174
libfreerdp/gdi/gfx.c
View File

@ -88,7 +88,6 @@ static UINT gdi_ResetGraphics(RdpgfxClientContext* context,
UINT16 count;
UINT32 DesktopWidth;
UINT32 DesktopHeight;
gdiGfxSurface* surface;
UINT16* pSurfaceIds = NULL;
rdpGdi* gdi;
rdpUpdate* update;
@ -120,11 +119,14 @@ static UINT gdi_ResetGraphics(RdpgfxClientContext* context,
update->DesktopResize(gdi->context);
}
WINPR_ASSERT(context->GetSurfaceIds);
context->GetSurfaceIds(context, &pSurfaceIds, &count);
for (index = 0; index < count; index++)
{
surface = (gdiGfxSurface*)context->GetSurfaceData(context, pSurfaceIds[index]);
WINPR_ASSERT(context->GetSurfaceData);
gdiGfxSurface* surface =
(gdiGfxSurface*)context->GetSurfaceData(context, pSurfaceIds[index]);
if (!surface)
continue;
@ -239,7 +241,6 @@ static UINT gdi_UpdateSurfaces(RdpgfxClientContext* context)
UINT16 count;
UINT16 index;
UINT status = ERROR_INTERNAL_ERROR;
gdiGfxSurface* surface;
UINT16* pSurfaceIds = NULL;
rdpGdi* gdi;
@ -249,12 +250,16 @@ static UINT gdi_UpdateSurfaces(RdpgfxClientContext* context)
WINPR_ASSERT(gdi);
EnterCriticalSection(&context->mux);
WINPR_ASSERT(context->GetSurfaceIds);
context->GetSurfaceIds(context, &pSurfaceIds, &count);
status = CHANNEL_RC_OK;
for (index = 0; index < count; index++)
{
surface = (gdiGfxSurface*)context->GetSurfaceData(context, pSurfaceIds[index]);
WINPR_ASSERT(context->GetSurfaceData);
gdiGfxSurface* surface =
(gdiGfxSurface*)context->GetSurfaceData(context, pSurfaceIds[index]);
if (!surface)
continue;
@ -335,6 +340,8 @@ static UINT gdi_SurfaceCommand_Uncompressed(rdpGdi* gdi, RdpgfxClientContext* co
WINPR_ASSERT(gdi);
WINPR_ASSERT(context);
WINPR_ASSERT(cmd);
WINPR_ASSERT(context->GetSurfaceData);
surface =
(gdiGfxSurface*)context->GetSurfaceData(context, (UINT16)MIN(UINT16_MAX, cmd->surfaceId));
@ -399,6 +406,8 @@ static UINT gdi_SurfaceCommand_RemoteFX(rdpGdi* gdi, RdpgfxClientContext* contex
WINPR_ASSERT(gdi);
WINPR_ASSERT(context);
WINPR_ASSERT(cmd);
WINPR_ASSERT(context->GetSurfaceData);
surface =
(gdiGfxSurface*)context->GetSurfaceData(context, (UINT16)MIN(UINT16_MAX, cmd->surfaceId));
@ -457,6 +466,8 @@ static UINT gdi_SurfaceCommand_ClearCodec(rdpGdi* gdi, RdpgfxClientContext* cont
WINPR_ASSERT(gdi);
WINPR_ASSERT(context);
WINPR_ASSERT(cmd);
WINPR_ASSERT(context->GetSurfaceData);
surface =
(gdiGfxSurface*)context->GetSurfaceData(context, (UINT16)MIN(UINT16_MAX, cmd->surfaceId));
@ -514,6 +525,8 @@ static UINT gdi_SurfaceCommand_Planar(rdpGdi* gdi, RdpgfxClientContext* context,
WINPR_ASSERT(gdi);
WINPR_ASSERT(context);
WINPR_ASSERT(cmd);
WINPR_ASSERT(context->GetSurfaceData);
surface =
(gdiGfxSurface*)context->GetSurfaceData(context, (UINT16)MIN(UINT16_MAX, cmd->surfaceId));
@ -574,6 +587,7 @@ static UINT gdi_SurfaceCommand_AVC420(rdpGdi* gdi, RdpgfxClientContext* context,
WINPR_ASSERT(context);
WINPR_ASSERT(cmd);
WINPR_ASSERT(context->GetSurfaceData);
surface =
(gdiGfxSurface*)context->GetSurfaceData(context, (UINT16)MIN(UINT16_MAX, cmd->surfaceId));
@ -663,6 +677,8 @@ static UINT gdi_SurfaceCommand_AVC444(rdpGdi* gdi, RdpgfxClientContext* context,
WINPR_ASSERT(gdi);
WINPR_ASSERT(context);
WINPR_ASSERT(cmd);
WINPR_ASSERT(context->GetSurfaceData);
surface =
(gdiGfxSurface*)context->GetSurfaceData(context, (UINT16)MIN(UINT16_MAX, cmd->surfaceId));
@ -802,6 +818,7 @@ static UINT gdi_SurfaceCommand_Alpha(rdpGdi* gdi, RdpgfxClientContext* context,
if (!Stream_CheckAndLogRequiredLength(TAG, s, 4))
return ERROR_INVALID_DATA;
WINPR_ASSERT(context->GetSurfaceData);
surface =
(gdiGfxSurface*)context->GetSurfaceData(context, (UINT16)MIN(UINT16_MAX, cmd->surfaceId));
@ -937,6 +954,8 @@ static UINT gdi_SurfaceCommand_Progressive(rdpGdi* gdi, RdpgfxClientContext* con
WINPR_ASSERT(context);
WINPR_ASSERT(cmd);
const UINT16 surfaceId = (UINT16)MIN(UINT16_MAX, cmd->surfaceId);
WINPR_ASSERT(context->GetSurfaceData);
surface = (gdiGfxSurface*)context->GetSurfaceData(context, surfaceId);
if (!surface)
@ -1151,6 +1170,8 @@ static UINT gdi_CreateSurface(RdpgfxClientContext* context,
memset(surface->data, 0xFF, (size_t)surface->scanline * surface->height);
region16_init(&surface->invalidRegion);
WINPR_ASSERT(context->SetSurfaceData);
rc = context->SetSurfaceData(context, surface->surfaceId, (void*)surface);
fail:
LeaveCriticalSection(&context->mux);
@ -1170,6 +1191,8 @@ static UINT gdi_DeleteSurface(RdpgfxClientContext* context,
rdpCodecs* codecs = NULL;
gdiGfxSurface* surface = NULL;
EnterCriticalSection(&context->mux);
WINPR_ASSERT(context->GetSurfaceData);
surface = (gdiGfxSurface*)context->GetSurfaceData(context, deleteSurface->surfaceId);
if (surface)
@ -1187,6 +1210,7 @@ static UINT gdi_DeleteSurface(RdpgfxClientContext* context,
free(surface);
}
WINPR_ASSERT(context->SetSurfaceData);
res = context->SetSurfaceData(context, deleteSurface->surfaceId, NULL);
if (res)
rc = res;
@ -1215,6 +1239,8 @@ static UINT gdi_SolidFill(RdpgfxClientContext* context, const RDPGFX_SOLID_FILL_
RECTANGLE_16 invalidRect;
rdpGdi* gdi = (rdpGdi*)context->custom;
EnterCriticalSection(&context->mux);
WINPR_ASSERT(context->GetSurfaceData);
surface = (gdiGfxSurface*)context->GetSurfaceData(context, solidFill->surfaceId);
if (!surface)
@ -1284,6 +1310,8 @@ static UINT gdi_SurfaceToSurface(RdpgfxClientContext* context,
rdpGdi* gdi = (rdpGdi*)context->custom;
EnterCriticalSection(&context->mux);
rectSrc = &(surfaceToSurface->rectSrc);
WINPR_ASSERT(context->GetSurfaceData);
surfaceSrc = (gdiGfxSurface*)context->GetSurfaceData(context, surfaceToSurface->surfaceIdSrc);
sameSurface =
(surfaceToSurface->surfaceIdSrc == surfaceToSurface->surfaceIdDest) ? TRUE : FALSE;
@ -1341,6 +1369,40 @@ fail:
return status;
}
static void gdi_GfxCacheEntryFree(gdiGfxCacheEntry* entry)
{
if (!entry)
return;
free(entry->data);
free(entry);
}
static gdiGfxCacheEntry* gdi_GfxCacheEntryNew(UINT64 cacheKey, UINT32 width, UINT32 height,
UINT32 format)
{
gdiGfxCacheEntry* cacheEntry = (gdiGfxCacheEntry*)calloc(1, sizeof(gdiGfxCacheEntry));
if (!cacheEntry)
goto fail;
cacheEntry->cacheKey = cacheKey;
cacheEntry->width = width;
cacheEntry->height = height;
cacheEntry->format = format;
cacheEntry->scanline = gfx_align_scanline(cacheEntry->width * 4, 16);
if ((cacheEntry->width > 0) && (cacheEntry->height > 0))
{
cacheEntry->data = (BYTE*)calloc(cacheEntry->height, cacheEntry->scanline);
if (!cacheEntry->data)
goto fail;
}
return cacheEntry;
fail:
gdi_GfxCacheEntryFree(cacheEntry);
return NULL;
}
/**
* Function description
*
@ -1351,10 +1413,12 @@ static UINT gdi_SurfaceToCache(RdpgfxClientContext* context,
{
const RECTANGLE_16* rect;
gdiGfxSurface* surface;
gdiGfxCacheEntry* cacheEntry;
gdiGfxCacheEntry* cacheEntry = NULL;
UINT rc = ERROR_INTERNAL_ERROR;
EnterCriticalSection(&context->mux);
rect = &(surfaceToCache->rectSrc);
WINPR_ASSERT(context->GetSurfaceData);
surface = (gdiGfxSurface*)context->GetSurfaceData(context, surfaceToCache->surfaceId);
if (!surface)
@ -1363,35 +1427,29 @@ static UINT gdi_SurfaceToCache(RdpgfxClientContext* context,
if (!is_rect_valid(rect, surface->width, surface->height))
goto fail;
cacheEntry = (gdiGfxCacheEntry*)calloc(1, sizeof(gdiGfxCacheEntry));
cacheEntry = gdi_GfxCacheEntryNew(surfaceToCache->cacheKey, (UINT32)(rect->right - rect->left),
(UINT32)(rect->bottom - rect->top), surface->format);
if (!cacheEntry)
goto fail;
cacheEntry->cacheKey = surfaceToCache->cacheKey;
cacheEntry->width = (UINT32)(rect->right - rect->left);
cacheEntry->height = (UINT32)(rect->bottom - rect->top);
cacheEntry->format = surface->format;
cacheEntry->scanline = gfx_align_scanline(cacheEntry->width * 4, 16);
cacheEntry->data = (BYTE*)calloc(cacheEntry->height, cacheEntry->scanline);
if (!cacheEntry->data)
{
free(cacheEntry);
goto fail;
}
if (!freerdp_image_copy(cacheEntry->data, cacheEntry->format, cacheEntry->scanline, 0, 0,
cacheEntry->width, cacheEntry->height, surface->data, surface->format,
surface->scanline, rect->left, rect->top, NULL, FREERDP_FLIP_NONE))
{
free(cacheEntry->data);
free(cacheEntry);
goto fail;
}
RDPGFX_EVICT_CACHE_ENTRY_PDU evict = { surfaceToCache->cacheSlot };
WINPR_ASSERT(context->EvictCacheEntry);
context->EvictCacheEntry(context, &evict);
WINPR_ASSERT(context->SetCacheSlotData);
rc = context->SetCacheSlotData(context, surfaceToCache->cacheSlot, (void*)cacheEntry);
fail:
if (rc != CHANNEL_RC_OK)
gdi_GfxCacheEntryFree(cacheEntry);
LeaveCriticalSection(&context->mux);
return rc;
}
@ -1412,7 +1470,11 @@ static UINT gdi_CacheToSurface(RdpgfxClientContext* context,
rdpGdi* gdi = (rdpGdi*)context->custom;
EnterCriticalSection(&context->mux);
WINPR_ASSERT(context->GetSurfaceData);
surface = (gdiGfxSurface*)context->GetSurfaceData(context, cacheToSurface->surfaceId);
WINPR_ASSERT(context->GetCacheSlotData);
cacheEntry = (gdiGfxCacheEntry*)context->GetCacheSlotData(context, cacheToSurface->cacheSlot);
if (!surface || !cacheEntry)
@ -1471,7 +1533,6 @@ static UINT gdi_CacheImportReply(RdpgfxClientContext* context,
UINT16 index;
UINT16 count;
const UINT16* slots;
gdiGfxCacheEntry* cacheEntry;
UINT error = CHANNEL_RC_OK;
slots = cacheImportReply->cacheSlots;
@ -1484,28 +1545,26 @@ static UINT gdi_CacheImportReply(RdpgfxClientContext* context,
if (cacheSlot == 0)
continue;
cacheEntry = (gdiGfxCacheEntry*)context->GetCacheSlotData(context, cacheSlot);
WINPR_ASSERT(context->GetCacheSlotData);
gdiGfxCacheEntry* cacheEntry =
(gdiGfxCacheEntry*)context->GetCacheSlotData(context, cacheSlot);
if (cacheEntry)
continue;
cacheEntry = (gdiGfxCacheEntry*)calloc(1, sizeof(gdiGfxCacheEntry));
cacheEntry = gdi_GfxCacheEntryNew(cacheSlot, 0, 0, PIXEL_FORMAT_BGRX32);
if (!cacheEntry)
return ERROR_INTERNAL_ERROR;
cacheEntry->width = 0;
cacheEntry->height = 0;
cacheEntry->format = PIXEL_FORMAT_BGRX32;
cacheEntry->scanline = (cacheEntry->width + (cacheEntry->width % 4)) * 4;
cacheEntry->data = NULL;
WINPR_ASSERT(context->SetCacheSlotData);
error = context->SetCacheSlotData(context, cacheSlot, (void*)cacheEntry);
if (error)
{
WLog_ERR(TAG, "CacheImportReply: SetCacheSlotData failed with error %" PRIu32 "",
error);
gdi_GfxCacheEntryFree(cacheEntry);
break;
}
}
@ -1516,41 +1575,38 @@ static UINT gdi_CacheImportReply(RdpgfxClientContext* context,
static UINT gdi_ImportCacheEntry(RdpgfxClientContext* context, UINT16 cacheSlot,
PERSISTENT_CACHE_ENTRY* importCacheEntry)
{
UINT error;
UINT error = ERROR_INTERNAL_ERROR;
gdiGfxCacheEntry* cacheEntry;
if (cacheSlot == 0)
return CHANNEL_RC_OK;
cacheEntry = (gdiGfxCacheEntry*)calloc(1, sizeof(gdiGfxCacheEntry));
cacheEntry = gdi_GfxCacheEntryNew(importCacheEntry->key64, importCacheEntry->width,
importCacheEntry->height, PIXEL_FORMAT_BGRX32);
if (!cacheEntry)
return ERROR_INTERNAL_ERROR;
cacheEntry->cacheKey = importCacheEntry->key64;
cacheEntry->width = (UINT32)importCacheEntry->width;
cacheEntry->height = (UINT32)importCacheEntry->height;
cacheEntry->format = PIXEL_FORMAT_BGRX32;
cacheEntry->scanline = (cacheEntry->width + (cacheEntry->width % 4)) * 4;
cacheEntry->data = (BYTE*)calloc(cacheEntry->height, cacheEntry->scanline);
if (!cacheEntry->data)
{
free(cacheEntry);
return ERROR_INTERNAL_ERROR;
}
goto fail;
if (!freerdp_image_copy(cacheEntry->data, cacheEntry->format, cacheEntry->scanline, 0, 0,
cacheEntry->width, cacheEntry->height, importCacheEntry->data,
PIXEL_FORMAT_BGRX32, 0, 0, 0, NULL, FREERDP_FLIP_NONE))
{
return ERROR_INTERNAL_ERROR;
}
goto fail;
RDPGFX_EVICT_CACHE_ENTRY_PDU evict = { cacheSlot };
WINPR_ASSERT(context->EvictCacheEntry);
error = context->EvictCacheEntry(context, &evict);
if (error != CHANNEL_RC_OK)
goto fail;
WINPR_ASSERT(context->SetCacheSlotData);
error = context->SetCacheSlotData(context, cacheSlot, (void*)cacheEntry);
fail:
if (error)
{
gdi_GfxCacheEntryFree(cacheEntry);
WLog_ERR(TAG, "ImportCacheEntry: SetCacheSlotData failed with error %" PRIu32 "", error);
}
return error;
}
@ -1560,6 +1616,7 @@ static UINT gdi_ExportCacheEntry(RdpgfxClientContext* context, UINT16 cacheSlot,
{
gdiGfxCacheEntry* cacheEntry;
WINPR_ASSERT(context->GetCacheSlotData);
cacheEntry = (gdiGfxCacheEntry*)context->GetCacheSlotData(context, cacheSlot);
if (cacheEntry)
@ -1586,15 +1643,18 @@ static UINT gdi_EvictCacheEntry(RdpgfxClientContext* context,
{
gdiGfxCacheEntry* cacheEntry;
UINT rc = ERROR_NOT_FOUND;
WINPR_ASSERT(context);
WINPR_ASSERT(evictCacheEntry);
EnterCriticalSection(&context->mux);
WINPR_ASSERT(context->GetCacheSlotData);
cacheEntry = (gdiGfxCacheEntry*)context->GetCacheSlotData(context, evictCacheEntry->cacheSlot);
if (cacheEntry)
{
free(cacheEntry->data);
free(cacheEntry);
}
gdi_GfxCacheEntryFree(cacheEntry);
WINPR_ASSERT(context->SetCacheSlotData);
rc = context->SetCacheSlotData(context, evictCacheEntry->cacheSlot, NULL);
LeaveCriticalSection(&context->mux);
return rc;
@ -1611,6 +1671,8 @@ static UINT gdi_MapSurfaceToOutput(RdpgfxClientContext* context,
UINT rc = ERROR_INTERNAL_ERROR;
gdiGfxSurface* surface = NULL;
EnterCriticalSection(&context->mux);
WINPR_ASSERT(context->GetSurfaceData);
surface = (gdiGfxSurface*)context->GetSurfaceData(context, surfaceToOutput->surfaceId);
if (!surface)
@ -1642,6 +1704,8 @@ gdi_MapSurfaceToScaledOutput(RdpgfxClientContext* context,
UINT rc = ERROR_INTERNAL_ERROR;
gdiGfxSurface* surface = NULL;
EnterCriticalSection(&context->mux);
WINPR_ASSERT(context->GetSurfaceData);
surface = (gdiGfxSurface*)context->GetSurfaceData(context, surfaceToOutput->surfaceId);
if (!surface)
@ -1677,6 +1741,8 @@ static UINT gdi_MapSurfaceToWindow(RdpgfxClientContext* context,
UINT rc = ERROR_INTERNAL_ERROR;
gdiGfxSurface* surface;
EnterCriticalSection(&context->mux);
WINPR_ASSERT(context->GetSurfaceData);
surface = (gdiGfxSurface*)context->GetSurfaceData(context, surfaceToWindow->surfaceId);
if (!surface)
@ -1719,6 +1785,8 @@ gdi_MapSurfaceToScaledWindow(RdpgfxClientContext* context,
UINT rc = ERROR_INTERNAL_ERROR;
gdiGfxSurface* surface;
EnterCriticalSection(&context->mux);
WINPR_ASSERT(context->GetSurfaceData);
surface = (gdiGfxSurface*)context->GetSurfaceData(context, surfaceToWindow->surfaceId);
if (!surface)