From c77def304833710927a501eea97dcfd4370780a3 Mon Sep 17 00:00:00 2001 From: Norbert Federa Date: Tue, 5 May 2015 17:25:17 +0200 Subject: [PATCH] Fix unchecked CreateMutex calls --- server/Windows/wf_rdpsnd.c | 9 ++++++--- winpr/libwinpr/interlocked/interlocked.c | 17 +++++++++++------ .../thread/test/TestThreadCreateProcess.c | 6 ++++++ winpr/libwinpr/utils/ssl.c | 11 +++++++---- 4 files changed, 30 insertions(+), 13 deletions(-) diff --git a/server/Windows/wf_rdpsnd.c b/server/Windows/wf_rdpsnd.c index d9d7f465a..3fdffe184 100644 --- a/server/Windows/wf_rdpsnd.c +++ b/server/Windows/wf_rdpsnd.c @@ -142,11 +142,14 @@ int wf_rdpsnd_unlock() BOOL wf_peer_rdpsnd_init(wfPeerContext* context) { - wfInfo* wfi; + wfInfo* wfi = wf_info_get_instance(); - wfi = wf_info_get_instance(); + if (!wfi) + return FALSE; + + if (!(wfi->snd_mutex = CreateMutex(NULL, FALSE, NULL))) + return FALSE; - wfi->snd_mutex = CreateMutex(NULL, FALSE, NULL); context->rdpsnd = rdpsnd_server_context_new(context->vcm); context->rdpsnd->data = context; diff --git a/winpr/libwinpr/interlocked/interlocked.c b/winpr/libwinpr/interlocked/interlocked.c index 6490bb6f1..b5d7eef43 100644 --- a/winpr/libwinpr/interlocked/interlocked.c +++ b/winpr/libwinpr/interlocked/interlocked.c @@ -253,31 +253,36 @@ PVOID InterlockedCompareExchangePointer(PVOID volatile *Destination, PVOID Excha static volatile HANDLE mutex = NULL; -int static_mutex_lock(volatile HANDLE* static_mutex) +BOOL static_mutex_lock(volatile HANDLE* static_mutex) { if (*static_mutex == NULL) { - HANDLE handle = CreateMutex(NULL, FALSE, NULL); + HANDLE handle; + + if (!(handle = CreateMutex(NULL, FALSE, NULL))) + return FALSE; if (InterlockedCompareExchangePointer((PVOID*) static_mutex, (PVOID) handle, NULL) != NULL) CloseHandle(handle); } - return (WaitForSingleObject(*static_mutex, INFINITE) == WAIT_FAILED); + return (WaitForSingleObject(*static_mutex, INFINITE) == WAIT_OBJECT_0); } LONGLONG InterlockedCompareExchange64(LONGLONG volatile *Destination, LONGLONG Exchange, LONGLONG Comperand) { LONGLONG previousValue = 0; - - static_mutex_lock(&mutex); + BOOL locked = static_mutex_lock(&mutex); previousValue = *Destination; if (*Destination == Comperand) *Destination = Exchange; - ReleaseMutex(mutex); + if (locked) + ReleaseMutex(mutex); + else + fprintf(stderr, "WARNING: InterlockedCompareExchange64 operation might have failed\n"); return previousValue; } diff --git a/winpr/libwinpr/thread/test/TestThreadCreateProcess.c b/winpr/libwinpr/thread/test/TestThreadCreateProcess.c index fb30f619b..0ccc405e2 100644 --- a/winpr/libwinpr/thread/test/TestThreadCreateProcess.c +++ b/winpr/libwinpr/thread/test/TestThreadCreateProcess.c @@ -55,6 +55,12 @@ int TestThreadCreateProcess(int argc, char* argv[]) &StartupInfo, &ProcessInformation); + if (!status) + { + printf("CreateProcess failed. error=%d\n", GetLastError()); + return 1; + } + FreeEnvironmentStrings(lpszEnvironmentBlock); WaitForSingleObject(ProcessInformation.hProcess, INFINITE); diff --git a/winpr/libwinpr/utils/ssl.c b/winpr/libwinpr/utils/ssl.c index 44a0397c0..9e63d3af7 100644 --- a/winpr/libwinpr/utils/ssl.c +++ b/winpr/libwinpr/utils/ssl.c @@ -60,12 +60,15 @@ static void _winpr_openssl_locking(int mode, int type, const char* file, int lin static struct CRYPTO_dynlock_value* _winpr_openssl_dynlock_create(const char* file, int line) { - struct CRYPTO_dynlock_value* dynlock = (struct CRYPTO_dynlock_value*) - malloc(sizeof(struct CRYPTO_dynlock_value)); + struct CRYPTO_dynlock_value* dynlock; - if (dynlock) + if (!(dynlock = (struct CRYPTO_dynlock_value*) malloc(sizeof(struct CRYPTO_dynlock_value)))) + return NULL; + + if (!(dynlock->mutex = CreateMutex(NULL, FALSE, NULL))) { - dynlock->mutex = CreateMutex(NULL, FALSE, NULL); + free(dynlock); + return NULL; } return dynlock;