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

Unified update->BeginPaint and update->EndPaint

Browse Source 
Since these functions were called from 2 different threads
(main thread or dynamic channel) depending on fastpath or
gfx channel use lock between these.
If not locked the invalid region may be accessed from both
threads and lead to crashes as experienced with nightly df280a7ff
This commit is contained in:
Armin Novak 2019-04-11 15:30:05 +02:00
parent df280a7ffd
commit 2a26aa0d87
7 changed files with 102 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
include/freerdp/update.h
View File

@ -252,6 +252,7 @@ struct rdp_update
BOOL combineUpdates;
rdpBounds currentBounds;
rdpBounds previousBounds;
CRITICAL_SECTION mux;
};
#endif /* FREERDP_UPDATE_H */

15
libfreerdp/core/fastpath.c
View File

@ -633,6 +633,7 @@ out_fail:
int fastpath_recv_updates(rdpFastPath* fastpath, wStream* s)
{
int rc = -2;
rdpUpdate* update;
if (!fastpath || !fastpath->rdp || !fastpath->rdp->update || !s)
@ -640,22 +641,26 @@ int fastpath_recv_updates(rdpFastPath* fastpath, wStream* s)
update = fastpath->rdp->update;
if (!IFCALLRESULT(TRUE, update->BeginPaint, update->context))
return -2;
if (!update_begin_paint(update))
goto fail;
while (Stream_GetRemainingLength(s) >= 3)
{
if (fastpath_recv_update_data(fastpath, s) < 0)
{
WLog_ERR(TAG, "fastpath_recv_update_data() fail");
return -3;
rc = -3;
goto fail;
}
}
if (!IFCALLRESULT(FALSE, update->EndPaint, update->context))
rc = 0;
fail:
if (!update_end_paint(update))
return -4;
return 0;
return rc;
}
static BOOL fastpath_read_input_event_header(wStream* s, BYTE* eventFlags, BYTE* eventCode)

10
libfreerdp/core/freerdp.c
View File

@ -270,12 +270,16 @@ BOOL freerdp_connect(freerdp* instance)
Stream_SetLength(s, record.length);
Stream_SetPosition(s, 0);
if (!update->BeginPaint(update->context))
if (!update_begin_paint(update))
status = FALSE;
else if (update_recv_surfcmds(update, s) < 0)
else
{
if (update_recv_surfcmds(update, s) < 0)
status = FALSE;
else if (!update->EndPaint(update->context))
if (!update_end_paint(update))
status = FALSE;
}
Stream_Release(s);
}

77
libfreerdp/core/update.c
View File

@ -636,8 +636,8 @@ BOOL update_recv(rdpUpdate* update, wStream* s)
Stream_Read_UINT16(s, updateType); /* updateType (2 bytes) */
WLog_Print(update->log, WLOG_TRACE, "%s Update Data PDU", UPDATE_TYPE_STRINGS[updateType]);
if (!IFCALLRESULT(TRUE, update->BeginPaint, context))
return FALSE;
if (!update_begin_paint(update))
goto fail;
switch (updateType)
{
@ -652,7 +652,7 @@ BOOL update_recv(rdpUpdate* update, wStream* s)
if (!bitmap_update)
{
WLog_ERR(TAG, "UPDATE_TYPE_BITMAP - update_read_bitmap_update() failed");
return FALSE;
goto fail;
}
rc = IFCALLRESULT(FALSE, update->BitmapUpdate, context, bitmap_update);
@ -667,7 +667,7 @@ BOOL update_recv(rdpUpdate* update, wStream* s)
if (!palette_update)
{
WLog_ERR(TAG, "UPDATE_TYPE_PALETTE - update_read_palette() failed");
return FALSE;
goto fail;
}
rc = IFCALLRESULT(FALSE, update->Palette, context, palette_update);
@ -684,15 +684,17 @@ BOOL update_recv(rdpUpdate* update, wStream* s)
break;
}
fail:
if (!update_end_paint(update))
rc = FALSE;
if (!rc)
{
WLog_ERR(TAG, "UPDATE_TYPE %s [%"PRIu16"] failed", update_type_to_string(updateType), updateType);
return FALSE;
}
if (!IFCALLRESULT(FALSE, update->EndPaint, context))
return FALSE;
return TRUE;
}
@ -764,13 +766,16 @@ void update_post_disconnect(rdpUpdate* update)
update->initialState = TRUE;
}
static BOOL update_begin_paint(rdpContext* context)
static BOOL _update_begin_paint(rdpContext* context)
{
wStream* s;
rdpUpdate* update = context->update;
if (update->us)
update->EndPaint(context);
{
if (!update_end_paint(update))
return FALSE;
}
s = fastpath_update_pdu_init_new(context->rdp->fastpath);
@ -785,7 +790,7 @@ static BOOL update_begin_paint(rdpContext* context)
return TRUE;
}
static BOOL update_end_paint(rdpContext* context)
static BOOL _update_end_paint(rdpContext* context)
{
wStream* s;
int headerLength;
@ -821,20 +826,14 @@ static void update_flush(rdpContext* context)
if (update->numberOrders > 0)
{
update->EndPaint(context);
update->BeginPaint(context);
update_end_paint(update);
update_begin_paint(update);
}
}
static void update_force_flush(rdpContext* context)
{
rdpUpdate* update = context->update;
if (update->numberOrders > 0)
{
update->EndPaint(context);
update->BeginPaint(context);
}
update_flush(context);
}
static BOOL update_check_flush(rdpContext* context, int size)
@ -845,7 +844,7 @@ static BOOL update_check_flush(rdpContext* context, int size)
if (!update->us)
{
update->BeginPaint(context);
update_begin_paint(update);
return FALSE;
}
@ -2080,8 +2079,8 @@ static BOOL update_send_set_keyboard_ime_status(rdpContext* context,
void update_register_server_callbacks(rdpUpdate* update)
{
update->BeginPaint = update_begin_paint;
update->EndPaint = update_end_paint;
update->BeginPaint = _update_begin_paint;
update->EndPaint = _update_end_paint;
update->SetBounds = update_set_bounds;
update->Synchronize = update_send_synchronize;
update->DesktopResize = update_send_desktop_resize;
@ -2095,7 +2094,6 @@ void update_register_server_callbacks(rdpUpdate* update)
update->SetKeyboardImeStatus = update_send_set_keyboard_ime_status;
update->SaveSessionInfo = rdp_send_save_session_info;
update->ServerStatusInfo = rdp_send_server_status_info;
update->primary->DstBlt = update_send_dstblt;
update->primary->PatBlt = update_send_patblt;
update->primary->ScrBlt = update_send_scrblt;
@ -2103,7 +2101,6 @@ void update_register_server_callbacks(rdpUpdate* update)
update->primary->LineTo = update_send_line_to;
update->primary->MemBlt = update_send_memblt;
update->primary->GlyphIndex = update_send_glyph_index;
update->secondary->CacheBitmap = update_send_cache_bitmap;
update->secondary->CacheBitmapV2 = update_send_cache_bitmap_v2;
update->secondary->CacheBitmapV3 = update_send_cache_bitmap_v3;
@ -2111,10 +2108,8 @@ void update_register_server_callbacks(rdpUpdate* update)
update->secondary->CacheGlyph = update_send_cache_glyph;
update->secondary->CacheGlyphV2 = update_send_cache_glyph_v2;
update->secondary->CacheBrush = update_send_cache_brush;
update->altsec->CreateOffscreenBitmap = update_send_create_offscreen_bitmap_order;
update->altsec->SwitchSurface = update_send_switch_surface_order;
update->pointer->PointerSystem = update_send_pointer_system;
update->pointer->PointerPosition = update_send_pointer_position;
update->pointer->PointerColor = update_send_pointer_color;
@ -2164,6 +2159,7 @@ rdpUpdate* update_new(rdpRdp* rdp)
return NULL;
update->log = WLog_Get("com.freerdp.core.update");
InitializeCriticalSection(&(update->mux));
update->pointer = (rdpPointerUpdate*) calloc(1, sizeof(rdpPointerUpdate));
if (!update->pointer)
@ -2241,6 +2237,35 @@ void update_free(rdpUpdate* update)
}
MessageQueue_Free(update->queue);
DeleteCriticalSection(&update->mux);
free(update);
}
}
BOOL update_begin_paint(rdpUpdate* update)
{
if (!update)
return FALSE;
EnterCriticalSection(&update->mux);
if (!update->BeginPaint)
return TRUE;
return update->BeginPaint(update->context);
}
BOOL update_end_paint(rdpUpdate* update)
{
BOOL rc = FALSE;
if (!update)
return FALSE;
if (update->EndPaint)
rc = update->EndPaint(update->context);
LeaveCriticalSection(&update->mux);
return rc;
}

3
libfreerdp/core/update.h
View File

@ -64,4 +64,7 @@ FREERDP_LOCAL void update_register_server_callbacks(rdpUpdate* update);
FREERDP_LOCAL void update_register_client_callbacks(rdpUpdate* update);
FREERDP_LOCAL int update_process_messages(rdpUpdate* update);
FREERDP_LOCAL BOOL update_begin_paint(rdpUpdate* update);
FREERDP_LOCAL BOOL update_end_paint(rdpUpdate* update);
#endif /* FREERDP_LIB_CORE_UPDATE_H */

11
libfreerdp/gdi/gfx.c
View File

@ -23,6 +23,8 @@
#include "config.h"
#endif
#include "../core/update.h"
#include <freerdp/log.h>
#include <freerdp/gdi/gfx.h>
#include <freerdp/gdi/region.h>
@ -124,7 +126,7 @@ static UINT gdi_OutputUpdate(rdpGdi* gdi, gdiGfxSurface* surface)
if (!(rects = region16_rects(&surface->invalidRegion, &nbRects)) || !nbRects)
return CHANNEL_RC_OK;
if (!IFCALLRESULT(TRUE, update->BeginPaint, gdi->context))
if (!update_begin_paint(update))
goto fail;
for (i = 0; i < nbRects; i++)
@ -146,11 +148,12 @@ static UINT gdi_OutputUpdate(rdpGdi* gdi, gdiGfxSurface* surface)
gdi_InvalidateRegion(gdi->primary->hdc, nXDst, nYDst, width, height);
}
if (!IFCALLRESULT(FALSE, update->EndPaint, gdi->context))
goto fail;
rc = CHANNEL_RC_OK;
fail:
if (!update_end_paint(update))
rc = ERROR_INTERNAL_ERROR;
region16_clear(&(surface->invalidRegion));
return rc;
}

29
libfreerdp/gdi/video.c
View File

@ -17,6 +17,7 @@
* limitations under the License.
*/
#include "../core/update.h"
#include <freerdp/client/geometry.h>
#include <freerdp/client/video.h>
@ -82,6 +83,7 @@ static VideoSurface* gdiVideoCreateSurface(VideoClientContext* video, BYTE* data
static BOOL gdiVideoShowSurface(VideoClientContext* video, VideoSurface* surface)
{
BOOL rc = FALSE;
rdpGdi* gdi = (rdpGdi*)video->custom;
gdiVideoSurface* gdiSurface = (gdiVideoSurface*)surface;
RECTANGLE_16 surfaceRect;
@ -90,18 +92,22 @@ static BOOL gdiVideoShowSurface(VideoClientContext* video, VideoSurface* surface
surfaceRect.top = surface->y;
surfaceRect.right = surface->x + surface->w;
surfaceRect.bottom = surface->y + surface->h;
update->BeginPaint(gdi->context);
if (!update_begin_paint(update))
goto fail;
if ((gdi->width < 0) || (gdi->height < 0))
return FALSE;
goto fail;
else
{
const UINT32 nXSrc = surface->x;
const UINT32 nYSrc = surface->y;
const UINT32 nXDst = nXSrc;
const UINT32 nYDst = nYSrc;
const UINT32 width = (surface->w + surface->x < (UINT32)gdi->width) ? surface->w : (UINT32)gdi->width - surface->x;
const UINT32 height = (surface->h + surface->y < (UINT32)gdi->height) ? surface->h : (UINT32)gdi->height -
const UINT32 width = (surface->w + surface->x < (UINT32)gdi->width) ? surface->w :
(UINT32)gdi->width - surface->x;
const UINT32 height = (surface->h + surface->y < (UINT32)gdi->height) ? surface->h :
(UINT32)gdi->height -
surface->y;
if (!freerdp_image_copy(gdi->primary_buffer, gdi->primary->hdc->format,
@ -109,14 +115,21 @@ static BOOL gdiVideoShowSurface(VideoClientContext* video, VideoSurface* surface
nXDst, nYDst, width, height,
surface->data, gdi->primary->hdc->format,
gdiSurface->scanline, 0, 0, NULL, FREERDP_FLIP_NONE))
return FALSE;
goto fail;
if ((nXDst > INT32_MAX) || (nYDst > INT32_MAX) || (width > INT32_MAX) || (height > INT32_MAX))
return FALSE;
goto fail;
gdi_InvalidateRegion(gdi->primary->hdc, (INT32)nXDst, (INT32)nYDst, (INT32)width, (INT32)height);
}
update->EndPaint(gdi->context);
return TRUE;
rc = TRUE;
fail:
if (!update_end_paint(update))
return FALSE;
return rc;
}
static BOOL gdiVideoDeleteSurface(VideoClientContext* video, VideoSurface* surface)