From 1c86e94de6107e5646efd4f91ef07f24d4425751 Mon Sep 17 00:00:00 2001 From: d_komarov Date: Fri, 6 Jul 2018 15:14:38 +0300 Subject: [PATCH] Fix deadlock when loading libtorrent Dll Definition of `bool file::has_manage_volume_privs` involves a call to `get_manage_volume_privs()`, causing restricted tasks to be performed from within `DllMain` function. They introduce possibility that client application deadlocks or crashes. You should never perform the following tasks from within DllMain: * Call `LoadLibrary` or `LoadLibraryEx` (either directly or indirectly). This can cause a deadlock or a crash. * Call the registry functions. These functions are implemented in 'Advapi32.dll'. If not initialized before your DLL, it can access uninitialized memory and cause the process to crash. --- include/libtorrent/file.hpp | 4 --- src/file.cpp | 49 +++++++++++++++---------------------- 2 files changed, 20 insertions(+), 33 deletions(-) diff --git a/include/libtorrent/file.hpp b/include/libtorrent/file.hpp index 486d4f6ca..640577057 100644 --- a/include/libtorrent/file.hpp +++ b/include/libtorrent/file.hpp @@ -341,10 +341,6 @@ namespace libtorrent #endif int m_open_mode; -#if defined TORRENT_WINDOWS - static bool has_manage_volume_privs; -#endif - #ifdef TORRENT_DEBUG_FILE_LEAKS std::string m_file_path; #endif diff --git a/src/file.cpp b/src/file.cpp index 1b3a88150..308763284 100644 --- a/src/file.cpp +++ b/src/file.cpp @@ -79,6 +79,7 @@ POSSIBILITY OF SUCH DAMAGE. #include #include "libtorrent/assert.hpp" +#include #include #include @@ -1314,9 +1315,6 @@ namespace libtorrent #ifdef TORRENT_WINDOWS bool get_manage_volume_privs(); - - // this needs to be run before CreateFile - bool file::has_manage_volume_privs = get_manage_volume_privs(); #endif file::file() @@ -1956,10 +1954,15 @@ typedef struct _FILE_ALLOCATED_RANGE_BUFFER { static LookupPrivilegeValue_t pLookupPrivilegeValue = NULL; static AdjustTokenPrivileges_t pAdjustTokenPrivileges = NULL; static bool failed_advapi = false; + HMODULE advapi = NULL; + + BOOST_SCOPE_EXIT(&advapi) { + if (advapi) FreeLibrary(advapi); + } BOOST_SCOPE_EXIT_END if (pOpenProcessToken == NULL && !failed_advapi) { - HMODULE advapi = LoadLibraryA("advapi32"); + advapi = LoadLibraryA("advapi32"); if (advapi == NULL) { failed_advapi = true; @@ -1977,16 +1980,19 @@ typedef struct _FILE_ALLOCATED_RANGE_BUFFER { } } - HANDLE token; + HANDLE token = NULL; if (!pOpenProcessToken(GetCurrentProcess() , TOKEN_ADJUST_PRIVILEGES | TOKEN_QUERY, &token)) return false; + BOOST_SCOPE_EXIT(&token) { + CloseHandle(token); + } BOOST_SCOPE_EXIT_END + TOKEN_PRIVILEGES privs; if (!pLookupPrivilegeValue(NULL, "SeManageVolumePrivilege" , &privs.Privileges[0].Luid)) { - CloseHandle(token); return false; } @@ -1996,38 +2002,23 @@ typedef struct _FILE_ALLOCATED_RANGE_BUFFER { bool ret = pAdjustTokenPrivileges(token, FALSE, &privs, 0, NULL, NULL) && GetLastError() == ERROR_SUCCESS; - CloseHandle(token); - return ret; } void set_file_valid_data(HANDLE f, boost::int64_t size) { typedef BOOL (WINAPI *SetFileValidData_t)(HANDLE, LONGLONG); - static SetFileValidData_t pSetFileValidData = NULL; - static bool failed_kernel32 = false; + static SetFileValidData_t const pSetFileValidData = (SetFileValidData_t) + GetProcAddress(GetModuleHandleA("kernel32"), "SetFileValidData"); - if (pSetFileValidData == NULL && !failed_kernel32) + if (pSetFileValidData == NULL) return; + + static bool const has_manage_volume_privs = get_manage_volume_privs(); + + if (has_manage_volume_privs) { - HMODULE k32 = LoadLibraryA("kernel32"); - if (k32 == NULL) - { - failed_kernel32 = true; - return; - } - pSetFileValidData = (SetFileValidData_t)GetProcAddress(k32, "SetFileValidData"); - if (pSetFileValidData == NULL) - { - failed_kernel32 = true; - return; - } + pSetFileValidData(f, size); } - - TORRENT_ASSERT(pSetFileValidData); - - // we don't necessarily expect to have enough - // privilege to do this, so ignore errors. - pSetFileValidData(f, size); } #endif