codec/rfx: error checking and various fixes

- removed some unneeded null checks for free()
- fixed a memory leak in shadow_client
- removed rfx_compose_message_header from API

Changed the following functions to BOOL, check the result
where they are called and handle failures:
- rfx_compose_message
- rfx_compose_message_header
- rfx_write_tile
- rfx_write_message_tileset
- rfx_write_message_frame_begin
- rfx_write_message_region
- rfx_write_message_frame_end
- rfx_write_message

rfx_process_message:
- check memory allocation failures
- verify protocol-conform order of data messages to prevents memory
  leaks caused by repeated allocations
- verify that header messages were parsed/received before the
  data messages
- treat unknown rlgr mode as error
- fixed/added error handling
- fixed all callers to check/handle result

rfx_encode_message:
- fixed incorrect usage of realloc
- missing malloc check
- missing check of CreateThreadpoolWork
- correct cleanup on failure (threadpool, memory)
- check rfx_encode_message result

rfx_encode_messages:
- check rfx_split_message result
- correct cleanup on failure
- prevent memory leak on failure

rfx_write_message_context:
- fixed invalid channelId value (must be 0xFF for WBT_CONTEXT)

rfx_process_message_codec_versions:
- fixed invalid read size of codec_version (it is 16bit)

rfx_process_message_channels:
- verify protocol conform channelId value

rfx_process_message_region:
- replaced invalid reallocs with malloc
- read and verify regionType and numTileSets from stream

rfx_process_message_tileset:
- check allocation results
- fixed incorrect usages of realloc

setupWorkers:
- fixed incorrect usages of realloc

rfx_split_message:
- removed dead code
- missing malloc check

rfx_compose_message:
- fixed a memory leak
- check/handle rfx_encode_message result
This commit is contained in:
Norbert Federa 2015-04-23 15:42:21 +02:00
parent e61620bd8e
commit 84577b1ca7
11 changed files with 427 additions and 227 deletions

View File

@ -670,17 +670,17 @@ void wf_gdi_surface_bits(wfContext* wfc, SURFACE_BITS_COMMAND* surface_bits_comm
{ {
int i, j; int i, j;
int tx, ty; int tx, ty;
char* tile_bitmap;
RFX_MESSAGE* message; RFX_MESSAGE* message;
BITMAPINFO bitmap_info; BITMAPINFO bitmap_info;
tile_bitmap = (char*) malloc(32);
ZeroMemory(tile_bitmap, 32);
if (surface_bits_command->codecID == RDP_CODEC_ID_REMOTEFX) if (surface_bits_command->codecID == RDP_CODEC_ID_REMOTEFX)
{ {
freerdp_client_codecs_prepare(wfc->codecs, FREERDP_CODEC_REMOTEFX); freerdp_client_codecs_prepare(wfc->codecs, FREERDP_CODEC_REMOTEFX);
message = rfx_process_message(wfc->codecs->rfx, surface_bits_command->bitmapData, surface_bits_command->bitmapDataLength); if (!(message = rfx_process_message(wfc->codecs->rfx, surface_bits_command->bitmapData, surface_bits_command->bitmapDataLength)))
{
WLog_ERR(TAG, "Failed to process RemoteFX message");
return;
}
/* blit each tile */ /* blit each tile */
for (i = 0; i < message->numTiles; i++) for (i = 0; i < message->numTiles; i++)
@ -750,9 +750,6 @@ void wf_gdi_surface_bits(wfContext* wfc, SURFACE_BITS_COMMAND* surface_bits_comm
{ {
WLog_ERR(TAG, "Unsupported codecID %d", surface_bits_command->codecID); WLog_ERR(TAG, "Unsupported codecID %d", surface_bits_command->codecID);
} }
if (tile_bitmap != NULL)
free(tile_bitmap);
} }
void wf_gdi_surface_frame_marker(wfContext* wfc, SURFACE_FRAME_MARKER* surface_frame_marker) void wf_gdi_surface_frame_marker(wfContext* wfc, SURFACE_FRAME_MARKER* surface_frame_marker)

View File

@ -1178,7 +1178,11 @@ BOOL xf_gdi_surface_bits(rdpContext* context, SURFACE_BITS_COMMAND* cmd)
{ {
freerdp_client_codecs_prepare(xfc->codecs, FREERDP_CODEC_REMOTEFX); freerdp_client_codecs_prepare(xfc->codecs, FREERDP_CODEC_REMOTEFX);
message = rfx_process_message(xfc->codecs->rfx, cmd->bitmapData, cmd->bitmapDataLength); if (!(message = rfx_process_message(xfc->codecs->rfx, cmd->bitmapData, cmd->bitmapDataLength)))
{
WLog_ERR(TAG, "Failed to process RemoteFX message");
return FALSE;
}
XSetFunction(xfc->display, xfc->gc, GXcopy); XSetFunction(xfc->display, xfc->gc, GXcopy);
XSetFillStyle(xfc->display, xfc->gc, FillSolid); XSetFillStyle(xfc->display, xfc->gc, FillSolid);

View File

@ -270,10 +270,11 @@ int xf_SurfaceCommand_RemoteFX(xfContext* xfc, RdpgfxClientContext* context, RDP
if (!surface) if (!surface)
return -1; return -1;
message = rfx_process_message(xfc->codecs->rfx, cmd->data, cmd->length); if (!(message = rfx_process_message(xfc->codecs->rfx, cmd->data, cmd->length)))
{
if (!message) WLog_ERR(TAG, "Failed to process RemoteFX message");
return -1; return -1;
}
region16_init(&clippingRects); region16_init(&clippingRects);

View File

@ -164,17 +164,16 @@ FREERDP_API UINT16 rfx_message_get_rect_count(RFX_MESSAGE* message);
FREERDP_API RFX_RECT* rfx_message_get_rect(RFX_MESSAGE* message, int index); FREERDP_API RFX_RECT* rfx_message_get_rect(RFX_MESSAGE* message, int index);
FREERDP_API void rfx_message_free(RFX_CONTEXT* context, RFX_MESSAGE* message); FREERDP_API void rfx_message_free(RFX_CONTEXT* context, RFX_MESSAGE* message);
FREERDP_API void rfx_compose_message_header(RFX_CONTEXT* context, wStream* s); FREERDP_API BOOL rfx_compose_message(RFX_CONTEXT* context, wStream* s,
FREERDP_API void rfx_compose_message(RFX_CONTEXT* context, wStream* s,
const RFX_RECT* rects, int num_rects, BYTE* image_data, int width, int height, int rowstride); const RFX_RECT* rects, int num_rects, BYTE* image_data, int width, int height, int rowstride);
FREERDP_API RFX_MESSAGE* rfx_encode_message(RFX_CONTEXT* context, const RFX_RECT* rects, FREERDP_API RFX_MESSAGE* rfx_encode_message(RFX_CONTEXT* context, const RFX_RECT* rects,
int numRects, BYTE* data, int width, int height, int scanline); int numRects, BYTE* data, int width, int height, int scanline);
FREERDP_API RFX_MESSAGE* rfx_encode_messages(RFX_CONTEXT* context, const RFX_RECT* rects, int numRects, FREERDP_API RFX_MESSAGE* rfx_encode_messages(RFX_CONTEXT* context, const RFX_RECT* rects, int numRects,
BYTE* data, int width, int height, int scanline, int* numMessages, int maxDataSize); BYTE* data, int width, int height, int scanline, int* numMessages, int maxDataSize);
FREERDP_API void rfx_write_message(RFX_CONTEXT* context, wStream* s, RFX_MESSAGE* message); FREERDP_API BOOL rfx_write_message(RFX_CONTEXT* context, wStream* s, RFX_MESSAGE* message);
FREERDP_API int rfx_context_reset(RFX_CONTEXT* context); FREERDP_API void rfx_context_reset(RFX_CONTEXT* context);
FREERDP_API RFX_CONTEXT* rfx_context_new(BOOL encoder); FREERDP_API RFX_CONTEXT* rfx_context_new(BOOL encoder);
FREERDP_API void rfx_context_free(RFX_CONTEXT* context); FREERDP_API void rfx_context_free(RFX_CONTEXT* context);

File diff suppressed because it is too large Load Diff

View File

@ -1029,7 +1029,11 @@ static BOOL gdi_surface_bits(rdpContext* context, SURFACE_BITS_COMMAND* cmd)
{ {
freerdp_client_codecs_prepare(gdi->codecs, FREERDP_CODEC_REMOTEFX); freerdp_client_codecs_prepare(gdi->codecs, FREERDP_CODEC_REMOTEFX);
message = rfx_process_message(gdi->codecs->rfx, cmd->bitmapData, cmd->bitmapDataLength); if (!(message = rfx_process_message(gdi->codecs->rfx, cmd->bitmapData, cmd->bitmapDataLength)))
{
WLog_ERR(TAG, "Failed to process RemoteFX message");
return FALSE;
}
/* blit each tile */ /* blit each tile */
for (i = 0; i < message->numTiles; i++) for (i = 0; i < message->numTiles; i++)

View File

@ -199,10 +199,11 @@ int gdi_SurfaceCommand_RemoteFX(rdpGdi* gdi, RdpgfxClientContext* context, RDPGF
if (!surface) if (!surface)
return -1; return -1;
message = rfx_process_message(gdi->codecs->rfx, cmd->data, cmd->length); if (!(message = rfx_process_message(gdi->codecs->rfx, cmd->data, cmd->length)))
{
if (!message) WLog_ERR(TAG, "Failed to process RemoteFX message");
return -1; return -1;
}
region16_init(&clippingRects); region16_init(&clippingRects);

View File

@ -147,8 +147,11 @@ void mf_peer_rfx_update(freerdp_peer* client)
mfp->rfx_context->width = mfi->servscreen_width; mfp->rfx_context->width = mfi->servscreen_width;
mfp->rfx_context->height = mfi->servscreen_height; mfp->rfx_context->height = mfi->servscreen_height;
rfx_compose_message(mfp->rfx_context, s, &rect, 1, if (!(rfx_compose_message(mfp->rfx_context, s, &rect, 1,
(BYTE*) dataBits, rect.width, rect.height, pitch); (BYTE*) dataBits, rect.width, rect.height, pitch)))
{
return;
}
cmd->destLeft = x; cmd->destLeft = x;
cmd->destTop = y; cmd->destTop = y;

View File

@ -157,8 +157,6 @@ static void test_peer_draw_background(freerdp_peer* client)
if (!client->settings->RemoteFxCodec && !client->settings->NSCodec) if (!client->settings->RemoteFxCodec && !client->settings->NSCodec)
return; return;
test_peer_begin_frame(client);
s = test_peer_stream_init(context); s = test_peer_stream_init(context);
rect.x = 0; rect.x = 0;
@ -167,13 +165,18 @@ static void test_peer_draw_background(freerdp_peer* client)
rect.height = client->settings->DesktopHeight; rect.height = client->settings->DesktopHeight;
size = rect.width * rect.height * 3; size = rect.width * rect.height * 3;
rgb_data = malloc(size); if (!(rgb_data = malloc(size)))
return;
memset(rgb_data, 0xA0, size); memset(rgb_data, 0xA0, size);
if (client->settings->RemoteFxCodec) if (client->settings->RemoteFxCodec)
{ {
rfx_compose_message(context->rfx_context, s, if (!rfx_compose_message(context->rfx_context, s,
&rect, 1, rgb_data, rect.width, rect.height, rect.width * 3); &rect, 1, rgb_data, rect.width, rect.height, rect.width * 3))
{
goto out;
}
cmd->codecID = client->settings->RemoteFxCodecId; cmd->codecID = client->settings->RemoteFxCodecId;
} }
else else
@ -192,11 +195,13 @@ static void test_peer_draw_background(freerdp_peer* client)
cmd->height = rect.height; cmd->height = rect.height;
cmd->bitmapDataLength = Stream_GetPosition(s); cmd->bitmapDataLength = Stream_GetPosition(s);
cmd->bitmapData = Stream_Buffer(s); cmd->bitmapData = Stream_Buffer(s);
test_peer_begin_frame(client);
update->SurfaceBits(update->context, cmd); update->SurfaceBits(update->context, cmd);
free(rgb_data);
test_peer_end_frame(client); test_peer_end_frame(client);
out:
free(rgb_data);
} }
static void test_peer_load_icon(freerdp_peer* client) static void test_peer_load_icon(freerdp_peer* client)

View File

@ -129,8 +129,11 @@ void wf_update_encode(wfInfo* wfi)
//WLog_DBG(TAG, "x:%d y:%d w:%d h:%d", wfi->invalid.left, wfi->invalid.top, width, height); //WLog_DBG(TAG, "x:%d y:%d w:%d h:%d", wfi->invalid.left, wfi->invalid.top, width, height);
Stream_Clear(wfi->s); Stream_Clear(wfi->s);
rfx_compose_message(wfi->rfx_context, wfi->s, &rect, 1, if (!(rfx_compose_message(wfi->rfx_context, wfi->s, &rect, 1,
pDataBits, width, height, stride); pDataBits, width, height, stride)))
{
return;
}
wfi->frame_idx = wfi->rfx_context->frameIdx; wfi->frame_idx = wfi->rfx_context->frameIdx;

View File

@ -384,6 +384,7 @@ int shadow_client_send_surface_bits(rdpShadowClient* client, rdpShadowSurface* s
{ {
RFX_RECT rect; RFX_RECT rect;
RFX_MESSAGE* messages; RFX_MESSAGE* messages;
RFX_RECT *messageRects = NULL;
shadow_encoder_prepare(encoder, FREERDP_CODEC_REMOTEFX); shadow_encoder_prepare(encoder, FREERDP_CODEC_REMOTEFX);
@ -394,9 +395,12 @@ int shadow_client_send_surface_bits(rdpShadowClient* client, rdpShadowSurface* s
rect.width = nWidth; rect.width = nWidth;
rect.height = nHeight; rect.height = nHeight;
messages = rfx_encode_messages(encoder->rfx, &rect, 1, pSrcData, if (!(messages = rfx_encode_messages(encoder->rfx, &rect, 1, pSrcData,
surface->width, surface->height, nSrcStep, &numMessages, surface->width, surface->height, nSrcStep, &numMessages,
settings->MultifragMaxRequestSize); settings->MultifragMaxRequestSize)))
{
return 0;
}
cmd.codecID = settings->RemoteFxCodecId; cmd.codecID = settings->RemoteFxCodecId;
@ -409,10 +413,20 @@ int shadow_client_send_surface_bits(rdpShadowClient* client, rdpShadowSurface* s
cmd.width = surface->width; cmd.width = surface->width;
cmd.height = surface->height; cmd.height = surface->height;
if (numMessages > 0)
messageRects = messages[0].rects;
for (i = 0; i < numMessages; i++) for (i = 0; i < numMessages; i++)
{ {
Stream_SetPosition(s, 0); Stream_SetPosition(s, 0);
rfx_write_message(encoder->rfx, s, &messages[i]); if (!rfx_write_message(encoder->rfx, s, &messages[i]))
{
while (i < numMessages)
{
rfx_message_free(encoder->rfx, &messages[i++]);
}
break;
}
rfx_message_free(encoder->rfx, &messages[i]); rfx_message_free(encoder->rfx, &messages[i]);
cmd.bitmapDataLength = Stream_GetPosition(s); cmd.bitmapDataLength = Stream_GetPosition(s);
@ -427,6 +441,7 @@ int shadow_client_send_surface_bits(rdpShadowClient* client, rdpShadowSurface* s
IFCALL(update->SurfaceFrameBits, update->context, &cmd, first, last, frameId); IFCALL(update->SurfaceFrameBits, update->context, &cmd, first, last, frameId);
} }
free(messageRects);
free(messages); free(messages);
} }
else if (settings->NSCodec) else if (settings->NSCodec)