From 38d82612cd7691342c7e6c82e3858fa9249ac224 Mon Sep 17 00:00:00 2001 From: Pete Batard Date: Wed, 10 Apr 2019 12:17:47 +0100 Subject: [PATCH] [core] fix a memory leak when search for process is interrupted --- src/process.c | 28 +++++++++++++++------------- src/rufus.c | 4 ++-- src/rufus.rc | 10 +++++----- src/stdio.c | 4 ++-- 4 files changed, 24 insertions(+), 22 deletions(-) diff --git a/src/process.c b/src/process.c index c4fd70a8..062ea8e0 100644 --- a/src/process.c +++ b/src/process.c @@ -28,6 +28,7 @@ #endif #include +#include #include "rufus.h" #include "process.h" @@ -55,7 +56,7 @@ PF_TYPE_DECL(NTAPI, NTSTATUS, NtClose, (HANDLE)); PF_TYPE_DECL(WINAPI, BOOL, QueryFullProcessImageNameW, (HANDLE, DWORD, LPWSTR, PDWORD)); static PVOID PhHeapHandle = NULL; -static char* _HandleName; +static wchar_t* _wHandleName; static BOOL _bPartialMatch, _bIgnoreSelf, _bQuiet; static BYTE access_mask; extern StrArray BlockingProcess; @@ -432,7 +433,6 @@ static DWORD WINAPI SearchProcessThread(LPVOID param) ULONG_PTR last_access_denied_pid = 0; ULONG bufferSize; USHORT wHandleNameLen; - WCHAR *wHandleName = NULL; HANDLE dupHandle = NULL; HANDLE processHandle = NULL; BOOLEAN bFound = FALSE, bGotCmdLine, verbose = !_bQuiet; @@ -462,8 +462,7 @@ static DWORD WINAPI SearchProcessThread(LPVOID param) pid[0] = (ULONG_PTR)0; cur_pid = 1; - wHandleName = utf8_to_wchar(_HandleName); - wHandleNameLen = (USHORT)wcslen(wHandleName); + wHandleNameLen = (USHORT)wcslen(_wHandleName); bufferSize = 0x200; buffer = PhAllocate(bufferSize); @@ -580,7 +579,7 @@ static DWORD WINAPI SearchProcessThread(LPVOID param) continue; // Match against our target string - if (wcsncmp(wHandleName, buffer->Name.Buffer, wHandleNameLen) != 0) + if (wcsncmp(_wHandleName, buffer->Name.Buffer, wHandleNameLen) != 0) continue; // If we are here, we have a process accessing our target! @@ -595,7 +594,7 @@ static DWORD WINAPI SearchProcessThread(LPVOID param) // If this is the very first process we find, print a header if (cmdline[0] == 0) - vuprintf("WARNING: The following process(es) or service(s) are accessing %s:", _HandleName); + vuprintf("WARNING: The following process(es) or service(s) are accessing %S:", _wHandleName); // Where possible, try to get the full command line bGotCmdLine = FALSE; @@ -638,9 +637,8 @@ out: if (cmdline[0] != 0) vuprintf("You should close these applications before attempting to reformat the drive."); else - vuprintf("NOTE: Could not identify the process(es) or service(s) accessing %s", _HandleName); + vuprintf("NOTE: Could not identify the process(es) or service(s) accessing %S", _wHandleName); - free(wHandleName); PhFree(buffer); PhFree(handles); PhDestroyHeap(); @@ -664,26 +662,30 @@ BYTE SearchProcess(char* HandleName, DWORD dwTimeOut, BOOL bPartialMatch, BOOL b HANDLE handle; DWORD res = 0; - _HandleName = HandleName; + _wHandleName = utf8_to_wchar(HandleName); _bPartialMatch = bPartialMatch; _bIgnoreSelf = bIgnoreSelf; _bQuiet = bQuiet; - access_mask = 0; + access_mask = 0x00; + + assert(_wHandleName != NULL); handle = CreateThread(NULL, 0, SearchProcessThread, NULL, 0, NULL); if (handle == NULL) { uprintf("Warning: Unable to create conflicting process search thread"); - return 0x00; + goto out; } res = WaitForSingleObjectWithMessages(handle, dwTimeOut); if (res == WAIT_TIMEOUT) { // Timeout - kill the thread TerminateThread(handle, 0); - uprintf("Warning: Search for conflicting processes was interrupted due to timeout"); + uprintf("Search for conflicting processes was interrupted due to timeout"); } else if (res != WAIT_OBJECT_0) { TerminateThread(handle, 0); - uprintf("Warning: Failed to wait for conflicting process search thread: %s", WindowsErrorString()); + uprintf("Warning: Failed to wait for conflicting process search thread %s", WindowsErrorString()); } +out: + free(_wHandleName); return access_mask; } diff --git a/src/rufus.c b/src/rufus.c index aa5e8d82..f98bd483 100755 --- a/src/rufus.c +++ b/src/rufus.c @@ -1788,7 +1788,7 @@ static void SaveISO(void) // dwTimeOut is the maximum amount of time we allow for this call to execute (in ms) // If bPrompt is false, the return value is the amount of time remaining before // dwTimeOut would expire (or zero if we spent more than dwTimeout in this procedure). -// If bPrompt is true, the return value is 0 on error, non-zero on success. +// If bPrompt is true, the return value is 0 on error, dwTimeOut on success. DWORD CheckDriveAccess(DWORD dwTimeOut, BOOL bPrompt) { uint32_t i, j; @@ -1848,7 +1848,7 @@ DWORD CheckDriveAccess(DWORD dwTimeOut, BOOL bPrompt) proceed = Notification(MSG_WARNING_QUESTION, NULL, NULL, title, lmprintf(MSG_132)); } if (bPrompt) { - ret = (DWORD)proceed; + ret = proceed ? dwTimeOut : 0; } else { ret = (DWORD)(GetTickCount64() - start_time); ret = (dwTimeOut > ret) ? (dwTimeOut - ret) : 0; diff --git a/src/rufus.rc b/src/rufus.rc index ba8e622f..239aafe0 100644 --- a/src/rufus.rc +++ b/src/rufus.rc @@ -33,7 +33,7 @@ LANGUAGE LANG_NEUTRAL, SUBLANG_NEUTRAL IDD_DIALOG DIALOGEX 12, 12, 232, 326 STYLE DS_SETFONT | DS_MODALFRAME | DS_CENTER | WS_MINIMIZEBOX | WS_POPUP | WS_CAPTION | WS_SYSMENU EXSTYLE WS_EX_ACCEPTFILES -CAPTION "Rufus 3.6.1515" +CAPTION "Rufus 3.6.1516" FONT 9, "Segoe UI Symbol", 400, 0, 0x0 BEGIN LTEXT "Drive Properties",IDS_DRIVE_PROPERTIES_TXT,8,6,53,12,NOT WS_GROUP @@ -394,8 +394,8 @@ END // VS_VERSION_INFO VERSIONINFO - FILEVERSION 3,6,1515,0 - PRODUCTVERSION 3,6,1515,0 + FILEVERSION 3,6,1516,0 + PRODUCTVERSION 3,6,1516,0 FILEFLAGSMASK 0x3fL #ifdef _DEBUG FILEFLAGS 0x1L @@ -413,13 +413,13 @@ BEGIN VALUE "Comments", "https://akeo.ie" VALUE "CompanyName", "Akeo Consulting" VALUE "FileDescription", "Rufus" - VALUE "FileVersion", "3.6.1515" + VALUE "FileVersion", "3.6.1516" VALUE "InternalName", "Rufus" VALUE "LegalCopyright", "© 2011-2019 Pete Batard (GPL v3)" VALUE "LegalTrademarks", "https://www.gnu.org/copyleft/gpl.html" VALUE "OriginalFilename", "rufus-3.6.exe" VALUE "ProductName", "Rufus" - VALUE "ProductVersion", "3.6.1515" + VALUE "ProductVersion", "3.6.1516" END END BLOCK "VarFileInfo" diff --git a/src/stdio.c b/src/stdio.c index 7b4202a2..e0c2b4da 100644 --- a/src/stdio.c +++ b/src/stdio.c @@ -354,7 +354,7 @@ BOOL WriteFileWithRetry(HANDLE hFile, LPCVOID lpBuffer, DWORD nNumberOfBytesToWr // Need to get the current file pointer in case we need to retry readFilePointer = SetFilePointerEx(hFile, liZero, &liFilePointer, FILE_CURRENT); if (!readFilePointer) - uprintf("Warning: Could not read file pointer: %s", WindowsErrorString()); + uprintf("Warning: Could not read file pointer %s", WindowsErrorString()); if (nNumRetries == 0) nNumRetries = 1; @@ -375,7 +375,7 @@ BOOL WriteFileWithRetry(HANDLE hFile, LPCVOID lpBuffer, DWORD nNumberOfBytesToWr } uprintf("Wrote %d bytes but requested %d", *lpNumberOfBytesWritten, nNumberOfBytesToWrite); } else { - uprintf("Write error [0x%08X]", GetLastError()); + uprintf("Write error %s", WindowsErrorString()); LastWriteError = ERROR_SEVERITY_ERROR|FAC(FACILITY_STORAGE)|GetLastError(); } // If we can't reposition for the next run, just abort