From 51554ff16c700cd9d51a324a16c4d49f11f6acd0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Moreau?= Date: Sun, 28 Dec 2014 14:56:13 -0500 Subject: [PATCH] channels: patch rdpdr/smartcard valgrind leaks, fix hang on disconnect --- channels/rdpdr/client/irp.c | 4 ++ channels/rdpdr/client/rdpdr_main.c | 60 ++++++++++--------- channels/rdpdr/client/rdpdr_main.h | 2 +- channels/smartcard/client/smartcard_main.c | 35 ++++++++--- .../smartcard/client/smartcard_operations.c | 21 ++++--- docs/valgrind.supp | 10 ++++ libfreerdp/core/redirection.c | 31 +++++----- winpr/libwinpr/smartcard/smartcard_pcsc.c | 26 ++++---- 8 files changed, 114 insertions(+), 75 deletions(-) diff --git a/channels/rdpdr/client/irp.c b/channels/rdpdr/client/irp.c index 150109e8b..f687b04b9 100644 --- a/channels/rdpdr/client/irp.c +++ b/channels/rdpdr/client/irp.c @@ -75,6 +75,10 @@ IRP* irp_new(DEVMAN* devman, wStream* s) return NULL; irp = (IRP*) _aligned_malloc(sizeof(IRP), MEMORY_ALLOCATION_ALIGNMENT); + + if (!irp) + return NULL; + ZeroMemory(irp, sizeof(IRP)); irp->input = s; diff --git a/channels/rdpdr/client/rdpdr_main.c b/channels/rdpdr/client/rdpdr_main.c index 66fa9d5fc..31c8aee75 100644 --- a/channels/rdpdr/client/rdpdr_main.c +++ b/channels/rdpdr/client/rdpdr_main.c @@ -452,7 +452,7 @@ static void* drive_hotplug_thread_func(void* arg) if (mfd < 0) { - WLog_ERR(TAG, "ERROR: Unable to open /proc/mounts."); + WLog_ERR(TAG, "ERROR: Unable to open /proc/mounts."); return NULL; } @@ -539,8 +539,8 @@ static void rdpdr_send_client_announce_reply(rdpdrPlugin* rdpdr) s = Stream_New(NULL, 12); - Stream_Write_UINT16(s, RDPDR_CTYP_CORE); - Stream_Write_UINT16(s, PAKID_CORE_CLIENTID_CONFIRM); + Stream_Write_UINT16(s, RDPDR_CTYP_CORE); /* Component (2 bytes) */ + Stream_Write_UINT16(s, PAKID_CORE_CLIENTID_CONFIRM); /* PacketId (2 bytes) */ Stream_Write_UINT16(s, rdpdr->versionMajor); Stream_Write_UINT16(s, rdpdr->versionMinor); @@ -562,8 +562,8 @@ static void rdpdr_send_client_name_request(rdpdrPlugin* rdpdr) s = Stream_New(NULL, 16 + computerNameLenW + 2); - Stream_Write_UINT16(s, RDPDR_CTYP_CORE); - Stream_Write_UINT16(s, PAKID_CORE_CLIENT_NAME); + Stream_Write_UINT16(s, RDPDR_CTYP_CORE); /* Component (2 bytes) */ + Stream_Write_UINT16(s, PAKID_CORE_CLIENT_NAME); /* PacketId (2 bytes) */ Stream_Write_UINT32(s, 1); /* unicodeFlag, 0 for ASCII and 1 for Unicode */ Stream_Write_UINT32(s, 0); /* codePage, must be set to zero */ @@ -614,8 +614,8 @@ static void rdpdr_send_device_list_announce_request(rdpdrPlugin* rdpdr, BOOL use s = Stream_New(NULL, 256); - Stream_Write_UINT16(s, RDPDR_CTYP_CORE); - Stream_Write_UINT16(s, PAKID_CORE_DEVICELIST_ANNOUNCE); + Stream_Write_UINT16(s, RDPDR_CTYP_CORE); /* Component (2 bytes) */ + Stream_Write_UINT16(s, PAKID_CORE_DEVICELIST_ANNOUNCE); /* PacketId (2 bytes) */ count_pos = (int) Stream_GetPosition(s); count = 0; @@ -662,7 +662,7 @@ static void rdpdr_send_device_list_announce_request(rdpdrPlugin* rdpdr, BOOL use Stream_Write(s, Stream_Buffer(device->data), data_len); count++; - WLog_INFO(TAG, "registered device #%d: %s (type=%d id=%d)", + WLog_INFO(TAG, "registered device #%d: %s (type=%d id=%d)", count, device->name, device->type, device->id); } } @@ -716,16 +716,16 @@ static void rdpdr_process_init(rdpdrPlugin* rdpdr) static void rdpdr_process_receive(rdpdrPlugin* rdpdr, wStream* s) { UINT16 component; - UINT16 packetID; - UINT32 deviceID; + UINT16 packetId; + UINT32 deviceId; UINT32 status; - Stream_Read_UINT16(s, component); - Stream_Read_UINT16(s, packetID); + Stream_Read_UINT16(s, component); /* Component (2 bytes) */ + Stream_Read_UINT16(s, packetId); /* PacketId (2 bytes) */ if (component == RDPDR_CTYP_CORE) { - switch (packetID) + switch (packetId) { case PAKID_CORE_SERVER_ANNOUNCE: rdpdr_process_server_announce_request(rdpdr, s); @@ -750,7 +750,7 @@ static void rdpdr_process_receive(rdpdrPlugin* rdpdr, wStream* s) case PAKID_CORE_DEVICE_REPLY: /* connect to a specific resource */ - Stream_Read_UINT32(s, deviceID); + Stream_Read_UINT32(s, deviceId); Stream_Read_UINT32(s, status); break; @@ -760,17 +760,19 @@ static void rdpdr_process_receive(rdpdrPlugin* rdpdr, wStream* s) break; default: + WLog_ERR(TAG, "rdpdr_process_receive: RDPDR_CTYP_CORE unknown PacketId: 0x%04X", packetId); break; } } else if (component == RDPDR_CTYP_PRN) { - + WLog_ERR(TAG, "rdpdr_process_receive: RDPDR_CTYP_PRN unknown PacketId: 0x%04X", packetId); } else { - + WLog_ERR(TAG, "rdpdr_process_receive: unknown message: Component: 0x%04X PacketId: 0x%04X", + component, packetId); } Stream_Free(s, TRUE); @@ -845,7 +847,7 @@ int rdpdr_send(rdpdrPlugin* rdpdr, wStream* s) if (status != CHANNEL_RC_OK) { Stream_Free(s, TRUE); - WLog_ERR(TAG, "rdpdr_send: VirtualChannelWrite failed %d", status); + WLog_ERR(TAG, "rdpdr_send: VirtualChannelWrite failed %d", status); } return status; @@ -869,7 +871,7 @@ static void rdpdr_virtual_channel_event_data_received(rdpdrPlugin* rdpdr, if (dataFlags & CHANNEL_FLAG_FIRST) { - if (rdpdr->data_in != NULL) + if (rdpdr->data_in) Stream_Free(rdpdr->data_in, TRUE); rdpdr->data_in = Stream_New(NULL, totalLength); @@ -883,14 +885,14 @@ static void rdpdr_virtual_channel_event_data_received(rdpdrPlugin* rdpdr, { if (Stream_Capacity(data_in) != Stream_GetPosition(data_in)) { - WLog_ERR(TAG, "svc_plugin_process_received: read error\n"); + WLog_ERR(TAG, "rdpdr_virtual_channel_event_data_received: read error\n"); } rdpdr->data_in = NULL; Stream_SealLength(data_in); Stream_SetPosition(data_in, 0); - MessageQueue_Post(rdpdr->MsgPipe->In, NULL, 0, (void*) data_in, NULL); + MessageQueue_Post(rdpdr->queue, NULL, 0, (void*) data_in, NULL); } } @@ -903,7 +905,7 @@ static VOID VCAPITYPE rdpdr_virtual_channel_open_event(DWORD openHandle, UINT ev if (!rdpdr) { - WLog_ERR(TAG, "rdpdr_virtual_channel_open_event: error no match\n"); + WLog_ERR(TAG, "rdpdr_virtual_channel_open_event: error no match\n"); return; } @@ -929,10 +931,10 @@ static void* rdpdr_virtual_channel_client_thread(void* arg) while (1) { - if (!MessageQueue_Wait(rdpdr->MsgPipe->In)) + if (!MessageQueue_Wait(rdpdr->queue)) break; - if (MessageQueue_Peek(rdpdr->MsgPipe->In, &message, TRUE)) + if (MessageQueue_Peek(rdpdr->queue, &message, TRUE)) { if (message.id == WMQ_QUIT) break; @@ -960,11 +962,11 @@ static void rdpdr_virtual_channel_event_connected(rdpdrPlugin* rdpdr, LPVOID pDa if (status != CHANNEL_RC_OK) { - WLog_ERR(TAG, "rdpdr_virtual_channel_event_connected: open failed: status: %d\n", status); + WLog_ERR(TAG, "rdpdr_virtual_channel_event_connected: open failed: status: %d\n", status); return; } - rdpdr->MsgPipe = MessagePipe_New(); + rdpdr->queue = MessageQueue_New(NULL); rdpdr->thread = CreateThread(NULL, 0, (LPTHREAD_START_ROUTINE) rdpdr_virtual_channel_client_thread, (void*) rdpdr, 0, NULL); @@ -972,13 +974,13 @@ static void rdpdr_virtual_channel_event_connected(rdpdrPlugin* rdpdr, LPVOID pDa static void rdpdr_virtual_channel_event_terminated(rdpdrPlugin* rdpdr) { - if (rdpdr->MsgPipe) + if (rdpdr->queue) { - MessagePipe_PostQuit(rdpdr->MsgPipe, 0); + MessageQueue_PostQuit(rdpdr->queue, 0); WaitForSingleObject(rdpdr->thread, INFINITE); - MessagePipe_Free(rdpdr->MsgPipe); - rdpdr->MsgPipe = NULL; + MessageQueue_Free(rdpdr->queue); + rdpdr->queue = NULL; CloseHandle(rdpdr->thread); rdpdr->thread = NULL; diff --git a/channels/rdpdr/client/rdpdr_main.h b/channels/rdpdr/client/rdpdr_main.h index f3b6c1aff..2be0c67f0 100644 --- a/channels/rdpdr/client/rdpdr_main.h +++ b/channels/rdpdr/client/rdpdr_main.h @@ -47,7 +47,7 @@ struct rdpdr_plugin wStream* data_in; void* InitHandle; DWORD OpenHandle; - wMessagePipe* MsgPipe; + wMessageQueue* queue; DEVMAN* devman; diff --git a/channels/smartcard/client/smartcard_main.c b/channels/smartcard/client/smartcard_main.c index 131802aea..5ed802291 100644 --- a/channels/smartcard/client/smartcard_main.c +++ b/channels/smartcard/client/smartcard_main.c @@ -105,6 +105,9 @@ void smartcard_context_free(SMARTCARD_CONTEXT* pContext) if (!pContext) return; + /* cancel blocking calls like SCardGetStatusChange */ + SCardCancel(pContext->hContext); + MessageQueue_PostQuit(pContext->IrpQueue, 0); WaitForSingleObject(pContext->thread, INFINITE); CloseHandle(pContext->thread); @@ -118,14 +121,24 @@ static void smartcard_free(DEVICE* device) { SMARTCARD_DEVICE* smartcard = (SMARTCARD_DEVICE*) device; - MessageQueue_PostQuit(smartcard->IrpQueue, 0); - WaitForSingleObject(smartcard->thread, INFINITE); + if (smartcard->IrpQueue) + { + MessageQueue_PostQuit(smartcard->IrpQueue, 0); + WaitForSingleObject(smartcard->thread, INFINITE); - CloseHandle(smartcard->thread); + MessageQueue_Free(smartcard->IrpQueue); + smartcard->IrpQueue = NULL; - Stream_Free(smartcard->device.data, TRUE); + CloseHandle(smartcard->thread); + smartcard->thread = NULL; + } + + if (smartcard->device.data) + { + Stream_Free(smartcard->device.data, TRUE); + smartcard->device.data = NULL; + } - MessageQueue_Free(smartcard->IrpQueue); ListDictionary_Free(smartcard->rgSCardContextList); ListDictionary_Free(smartcard->rgOutstandingMessages); Queue_Free(smartcard->CompletedIrpQueue); @@ -414,6 +427,7 @@ static void* smartcard_thread_func(void* arg) { WaitForSingleObject(irp->thread, INFINITE); CloseHandle(irp->thread); + irp->thread = NULL; } smartcard_complete_irp(smartcard, irp); @@ -441,6 +455,7 @@ static void* smartcard_thread_func(void* arg) { WaitForSingleObject(irp->thread, INFINITE); CloseHandle(irp->thread); + irp->thread = NULL; } smartcard_complete_irp(smartcard, irp); @@ -509,10 +524,16 @@ int DeviceServiceEntry(PDEVICE_SERVICE_ENTRY_POINTS pEntryPoints) smartcard->log = WLog_Get("com.freerdp.channel.smartcard.client"); smartcard->IrpQueue = MessageQueue_New(NULL); - smartcard->rgSCardContextList = ListDictionary_New(TRUE); - smartcard->rgOutstandingMessages = ListDictionary_New(TRUE); + smartcard->CompletedIrpQueue = Queue_New(TRUE, -1, -1); + smartcard->rgSCardContextList = ListDictionary_New(TRUE); + + ListDictionary_ValueObject(smartcard->rgSCardContextList)->fnObjectFree = + (OBJECT_FREE_FN) smartcard_context_free; + + smartcard->rgOutstandingMessages = ListDictionary_New(TRUE); + smartcard->thread = CreateThread(NULL, 0, (LPTHREAD_START_ROUTINE) smartcard_thread_func, smartcard, CREATE_SUSPENDED, NULL); diff --git a/channels/smartcard/client/smartcard_operations.c b/channels/smartcard/client/smartcard_operations.c index 86db78145..c0e80e5ce 100644 --- a/channels/smartcard/client/smartcard_operations.c +++ b/channels/smartcard/client/smartcard_operations.c @@ -555,8 +555,8 @@ static UINT32 smartcard_ConnectA_Decode(SMARTCARD_DEVICE* smartcard, SMARTCARD_O static UINT32 smartcard_ConnectA_Call(SMARTCARD_DEVICE* smartcard, SMARTCARD_OPERATION* operation, ConnectA_Call* call) { LONG status; - SCARDHANDLE hCard; - Connect_Return ret; + SCARDHANDLE hCard = 0; + Connect_Return ret = { 0 }; IRP* irp = operation->irp; if ((call->Common.dwPreferredProtocols == SCARD_PROTOCOL_UNDEFINED) && @@ -604,8 +604,8 @@ static UINT32 smartcard_ConnectW_Decode(SMARTCARD_DEVICE* smartcard, SMARTCARD_O static UINT32 smartcard_ConnectW_Call(SMARTCARD_DEVICE* smartcard, SMARTCARD_OPERATION* operation, ConnectW_Call* call) { LONG status; - SCARDHANDLE hCard; - Connect_Return ret; + SCARDHANDLE hCard = 0; + Connect_Return ret = { 0 }; IRP* irp = operation->irp; if ((call->Common.dwPreferredProtocols == SCARD_PROTOCOL_UNDEFINED) && @@ -1020,9 +1020,11 @@ static UINT32 smartcard_GetAttrib_Call(SMARTCARD_DEVICE* smartcard, SMARTCARD_OP if (ret.ReturnCode) { WLog_Print(smartcard->log, WLOG_WARN, - "SCardGetAttrib: %s (0x%08X) cbAttrLen: %d\n", + "SCardGetAttrib: %s (0x%08X) cbAttrLen: %d", SCardGetAttributeString(call->dwAttrId), call->dwAttrId, call->cbAttrLen); Stream_Zero(irp->output, 256); + + free(ret.pbAttr); return ret.ReturnCode; } @@ -1198,7 +1200,7 @@ UINT32 smartcard_irp_device_control_decode(SMARTCARD_DEVICE* smartcard, SMARTCAR if (Stream_Length(irp->input) != (Stream_GetPosition(irp->input) + inputBufferLength)) { WLog_Print(smartcard->log, WLOG_WARN, - "InputBufferLength mismatch: Actual: %d Expected: %d\n", + "InputBufferLength mismatch: Actual: %d Expected: %d", Stream_Length(irp->input), Stream_GetPosition(irp->input) + inputBufferLength); return SCARD_F_INTERNAL_ERROR; } @@ -1206,7 +1208,7 @@ UINT32 smartcard_irp_device_control_decode(SMARTCARD_DEVICE* smartcard, SMARTCAR WLog_Print(smartcard->log, WLOG_DEBUG, "%s (0x%08X) FileId: %d CompletionId: %d", smartcard_get_ioctl_string(ioControlCode, TRUE), ioControlCode, irp->FileId, irp->CompletionId); #if 0 - WLog_DBG(TAG, "%s (0x%08X) FileId: %d CompletionId: %d\n", + WLog_DBG(TAG, "%s (0x%08X) FileId: %d CompletionId: %d", smartcard_get_ioctl_string(ioControlCode, TRUE), ioControlCode, irp->FileId, irp->CompletionId); #endif @@ -1727,8 +1729,7 @@ UINT32 smartcard_irp_device_control_call(SMARTCARD_DEVICE* smartcard, SMARTCARD_ (ioControlCode != SCARD_IOCTL_RELEASESTARTEDEVENT)) { offset = (RDPDR_DEVICE_IO_RESPONSE_LENGTH + RDPDR_DEVICE_IO_CONTROL_RSP_HDR_LENGTH); - smartcard_pack_write_size_align(smartcard, irp->output, - Stream_GetPosition(irp->output) - offset, 8); + smartcard_pack_write_size_align(smartcard, irp->output, Stream_GetPosition(irp->output) - offset, 8); } if ((result != SCARD_S_SUCCESS) && (result != SCARD_E_TIMEOUT) && @@ -1756,12 +1757,14 @@ UINT32 smartcard_irp_device_control_call(SMARTCARD_DEVICE* smartcard, SMARTCARD_ outputBufferLength = Stream_Length(irp->output) - RDPDR_DEVICE_IO_RESPONSE_LENGTH - 4; objectBufferLength = outputBufferLength - RDPDR_DEVICE_IO_RESPONSE_LENGTH; Stream_SetPosition(irp->output, RDPDR_DEVICE_IO_RESPONSE_LENGTH); + /* Device Control Response */ Stream_Write_UINT32(irp->output, outputBufferLength); /* OutputBufferLength (4 bytes) */ smartcard_pack_common_type_header(smartcard, irp->output); /* CommonTypeHeader (8 bytes) */ smartcard_pack_private_type_header(smartcard, irp->output, objectBufferLength); /* PrivateTypeHeader (8 bytes) */ Stream_Write_UINT32(irp->output, result); /* Result (4 bytes) */ Stream_SetPosition(irp->output, Stream_Length(irp->output)); + return SCARD_S_SUCCESS; } diff --git a/docs/valgrind.supp b/docs/valgrind.supp index 6fd68e54a..563efe0cc 100644 --- a/docs/valgrind.supp +++ b/docs/valgrind.supp @@ -12,6 +12,16 @@ fun:getaddrinfo } +{ + ignore pcsc-lite SCardConnect + Memcheck:Param + socketcall.sendto(msg) + fun:send + fun:MessageSend + fun:MessageSendWithHeader + fun:SCardConnect +} + { ignore openssl malloc Memcheck:Leak diff --git a/libfreerdp/core/redirection.c b/libfreerdp/core/redirection.c index 242d001d4..5c2048d66 100644 --- a/libfreerdp/core/redirection.c +++ b/libfreerdp/core/redirection.c @@ -32,48 +32,48 @@ void rdp_print_redirection_flags(UINT32 flags) { - WLog_INFO(TAG, "redirectionFlags = {"); + WLog_INFO(TAG, "redirectionFlags = {"); if (flags & LB_TARGET_NET_ADDRESS) - WLog_INFO(TAG, "\tLB_TARGET_NET_ADDRESS"); + WLog_INFO(TAG, "\tLB_TARGET_NET_ADDRESS"); if (flags & LB_LOAD_BALANCE_INFO) - WLog_INFO(TAG, "\tLB_LOAD_BALANCE_INFO"); + WLog_INFO(TAG, "\tLB_LOAD_BALANCE_INFO"); if (flags & LB_USERNAME) - WLog_INFO(TAG, "\tLB_USERNAME"); + WLog_INFO(TAG, "\tLB_USERNAME"); if (flags & LB_DOMAIN) - WLog_INFO(TAG, "\tLB_DOMAIN"); + WLog_INFO(TAG, "\tLB_DOMAIN"); if (flags & LB_PASSWORD) - WLog_INFO(TAG, "\tLB_PASSWORD"); + WLog_INFO(TAG, "\tLB_PASSWORD"); if (flags & LB_DONTSTOREUSERNAME) - WLog_INFO(TAG, "\tLB_DONTSTOREUSERNAME"); + WLog_INFO(TAG, "\tLB_DONTSTOREUSERNAME"); if (flags & LB_SMARTCARD_LOGON) - WLog_INFO(TAG, "\tLB_SMARTCARD_LOGON"); + WLog_INFO(TAG, "\tLB_SMARTCARD_LOGON"); if (flags & LB_NOREDIRECT) - WLog_INFO(TAG, "\tLB_NOREDIRECT"); + WLog_INFO(TAG, "\tLB_NOREDIRECT"); if (flags & LB_TARGET_FQDN) - WLog_INFO(TAG, "\tLB_TARGET_FQDN"); + WLog_INFO(TAG, "\tLB_TARGET_FQDN"); if (flags & LB_TARGET_NETBIOS_NAME) - WLog_INFO(TAG, "\tLB_TARGET_NETBIOS_NAME"); + WLog_INFO(TAG, "\tLB_TARGET_NETBIOS_NAME"); if (flags & LB_TARGET_NET_ADDRESSES) - WLog_INFO(TAG, "\tLB_TARGET_NET_ADDRESSES"); + WLog_INFO(TAG, "\tLB_TARGET_NET_ADDRESSES"); if (flags & LB_CLIENT_TSV_URL) - WLog_INFO(TAG, "\tLB_CLIENT_TSV_URL"); + WLog_INFO(TAG, "\tLB_CLIENT_TSV_URL"); if (flags & LB_SERVER_TSV_CAPABLE) - WLog_INFO(TAG, "\tLB_SERVER_TSV_CAPABLE"); + WLog_INFO(TAG, "\tLB_SERVER_TSV_CAPABLE"); - WLog_INFO(TAG, "}"); + WLog_INFO(TAG, "}"); } BOOL rdp_redirection_read_string(wStream* s, char** str) @@ -200,6 +200,7 @@ BOOL rdp_recv_server_redirection_pdu(rdpRdp* rdp, wStream* s) Stream_Read_UINT16(s, length); /* length (2 bytes) */ Stream_Read_UINT32(s, redirection->sessionID); /* sessionID (4 bytes) */ Stream_Read_UINT32(s, redirection->flags); /* redirFlags (4 bytes) */ + WLog_Print(redirection->log, WLOG_DEBUG, "flags: 0x%04X, redirFlags: 0x%04X length: %d, sessionID: 0x%08X", flags, redirection->flags, length, redirection->sessionID); #ifdef WITH_DEBUG_REDIR diff --git a/winpr/libwinpr/smartcard/smartcard_pcsc.c b/winpr/libwinpr/smartcard/smartcard_pcsc.c index 5b7d0c31b..e6fa0f4e0 100644 --- a/winpr/libwinpr/smartcard/smartcard_pcsc.c +++ b/winpr/libwinpr/smartcard/smartcard_pcsc.c @@ -1410,6 +1410,9 @@ WINSCARDAPI LONG WINAPI PCSC_SCardGetStatusChange_Internal(SCARDCONTEXT hContext if (!cReaders) return SCARD_S_SUCCESS; + /* pcsc-lite interprets value 0 as INFINITE, work around the problem by using value 1 */ + pcsc_dwTimeout = pcsc_dwTimeout ? pcsc_dwTimeout : 1; + /** * Apple's SmartCard Services (not vanilla pcsc-lite) appears to have trouble with the * "\\\\?PnP?\\Notification" reader name. I am always getting EXC_BAD_ACCESS with it. @@ -1463,15 +1466,10 @@ WINSCARDAPI LONG WINAPI PCSC_SCardGetStatusChange_Internal(SCARDCONTEXT hContext cMappedReaders = j; - /** - * pcsc-lite interprets dwTimeout value 0 as INFINITE, use value 1 as a workaround - */ - if (cMappedReaders > 0) { status = (LONG) g_PCSC.pfnSCardGetStatusChange(hContext, - pcsc_dwTimeout ? pcsc_dwTimeout : 1, - states, cMappedReaders); + pcsc_dwTimeout, states, cMappedReaders); status = PCSC_MapErrorCodeToWinSCard(status); } else @@ -2244,21 +2242,18 @@ WINSCARDAPI LONG WINAPI PCSC_SCardGetAttrib_FriendlyName(SCARDHANDLE hCard, DWOR cbAttrLen = *pcbAttrLen; *pcbAttrLen = SCARD_AUTOALLOCATE; - status = PCSC_SCardGetAttrib_Internal(hCard, SCARD_ATTR_DEVICE_FRIENDLY_NAME_A, - (LPBYTE) &pbAttrA, pcbAttrLen); + status = PCSC_SCardGetAttrib_Internal(hCard, SCARD_ATTR_DEVICE_FRIENDLY_NAME_A, (LPBYTE) &pbAttrA, pcbAttrLen); if (status != SCARD_S_SUCCESS) { pbAttrA = NULL; *pcbAttrLen = SCARD_AUTOALLOCATE; - status = PCSC_SCardGetAttrib_Internal(hCard, SCARD_ATTR_DEVICE_FRIENDLY_NAME_W, - (LPBYTE) &pbAttrW, pcbAttrLen); + status = PCSC_SCardGetAttrib_Internal(hCard, SCARD_ATTR_DEVICE_FRIENDLY_NAME_W, (LPBYTE) &pbAttrW, pcbAttrLen); if (status != SCARD_S_SUCCESS) return status; - length = ConvertFromUnicode(CP_UTF8, 0, (WCHAR*) pbAttrW, - *pcbAttrLen, (char**) &pbAttrA, 0, NULL, NULL); + length = ConvertFromUnicode(CP_UTF8, 0, (WCHAR*) pbAttrW, *pcbAttrLen, (char**) &pbAttrA, 0, NULL, NULL); namePCSC = pbAttrA; PCSC_SCardFreeMemory_Internal(hContext, pbAttrW); } @@ -2290,6 +2285,7 @@ WINSCARDAPI LONG WINAPI PCSC_SCardGetAttrib_FriendlyName(SCARDHANDLE hCard, DWOR if (dwAttrId == SCARD_ATTR_DEVICE_FRIENDLY_NAME_W) { + /* length here includes null terminator */ length = ConvertToUnicode(CP_UTF8, 0, (char*) friendlyNameA, -1, &friendlyNameW, 0); free(friendlyNameA); @@ -2304,14 +2300,14 @@ WINSCARDAPI LONG WINAPI PCSC_SCardGetAttrib_FriendlyName(SCARDHANDLE hCard, DWOR } else { - if (((length + 1) * 2) > cbAttrLen) + if ((length * 2) > cbAttrLen) { free(friendlyNameW); return SCARD_E_INSUFFICIENT_BUFFER; } else { - CopyMemory(pbAttr, (BYTE*) friendlyNameW, ((length + 1) * 2)); + CopyMemory(pbAttr, (BYTE*) friendlyNameW, (length * 2)); *pcbAttrLen = length * 2; free(friendlyNameW); } @@ -2341,6 +2337,8 @@ WINSCARDAPI LONG WINAPI PCSC_SCardGetAttrib_FriendlyName(SCARDHANDLE hCard, DWOR } } + free(namePCSC); + return status; }