[PATCH 0/4] MR4587: kernelbase: Implement flag validation in `LoadLibraryEx()` to work like in Win10
While debugging League of Legends I noticed that `LoadLibraryEx()` is invoked with binary filename (non-text) and non-NULL `hFile`. In Windows such call would return `NULL` with LastError set as `ERROR_INVALID_PARAMETER` but current Wine implementation would try to proceed ahead. I don't know if this is intentional trick to mess up debugging/reversing tools or if we have some memory corruption before that overwrites filename buffer. But then I can't imagine how `hFile` could have gotten corrupted since shouldn't that be simple static value in register... Having this MR causes very noticeable difference in LoL: * without it - process deadlocks * with it - no deadlock Also while looking at this, I tried a lot of different flag combinations on Windows 10 and here I implemented so it works exactly like that. I also implemented test case for non-NULL `hFile` but I didn't bother for other flags since I don't know if we even care about those. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4587
From: Dāvis Mosāns <davispuh(a)gmail.com> When hFile is non-NULL then LoadLibraryEx() should return NULL with LastError set as ERROR_INVALID_PARAMETER --- dlls/kernelbase/tests/file.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/dlls/kernelbase/tests/file.c b/dlls/kernelbase/tests/file.c index 18dff0a32ab..9b922aa7d17 100644 --- a/dlls/kernelbase/tests/file.c +++ b/dlls/kernelbase/tests/file.c @@ -46,6 +46,23 @@ static void test_ioring_caps(void) todo_wine ok(hr == S_OK, "got %#lx.\n", hr); } +static void test_load_library_ex_flags(void) +{ + HMODULE hmod; + DWORD last_error; + + SetLastError(0x33); // Random value so we can see if it was changed + + // Test if LoadLibraryEx can load a dll + hmod = LoadLibraryExW(L"kernelbase.dll", NULL /* hFile */, LOAD_LIBRARY_SEARCH_SYSTEM32 /* dwFlags */); + ok(hmod != NULL, "LoadLibraryEx returned %p with LastError: %#lx.\n", hmod, GetLastError()); + + // When hFile is non-NULL LoadLibraryEx should return NULL and set LastError to ERROR_INVALID_PARAMETER + hmod = LoadLibraryExW(L"kernelbase.dll", (HANDLE)0x000003A0 /* random hFile */, 0 /* dwFlags */); + last_error = GetLastError(); + ok(hmod == NULL && last_error == ERROR_INVALID_PARAMETER, "LoadLibraryEx returned %p with LastError: %#lx.\n", hmod, last_error); +} + START_TEST(file) { HMODULE hmod; @@ -54,4 +71,5 @@ START_TEST(file) pQueryIoRingCapabilities = (void *)GetProcAddress(hmod, "QueryIoRingCapabilities"); test_ioring_caps(); + test_load_library_ex_flags(); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4587
From: Dāvis Mosāns <davispuh(a)gmail.com> --- dlls/kernelbase/loader.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/dlls/kernelbase/loader.c b/dlls/kernelbase/loader.c index 59b91596f13..7aff6bb9412 100644 --- a/dlls/kernelbase/loader.c +++ b/dlls/kernelbase/loader.c @@ -531,11 +531,25 @@ HMODULE WINAPI DECLSPEC_HOTPATCH LoadLibraryExW( LPCWSTR name, HANDLE file, DWOR UNICODE_STRING str; HMODULE module; - if (!name) + if (!name || file) { + // file must be NULL SetLastError( ERROR_INVALID_PARAMETER ); return 0; } + + if ((flags & LOAD_LIBRARY_AS_DATAFILE) && (flags & LOAD_LIBRARY_AS_DATAFILE_EXCLUSIVE)) { + // Mutually exclusive + SetLastError( ERROR_INVALID_PARAMETER ); + return 0; + } + + if ((flags & LOAD_LIBRARY_SEARCH_DEFAULT_DIRS) && (flags & LOAD_WITH_ALTERED_SEARCH_PATH)) { + // Can't mix default dirs with altered path + SetLastError( ERROR_INVALID_PARAMETER ); + return 0; + } + RtlInitUnicodeString( &str, name ); if (str.Buffer[str.Length/sizeof(WCHAR) - 1] != ' ') return load_library( &str, flags ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4587
From: Dāvis Mosāns <davispuh(a)gmail.com> --- dlls/kernelbase/loader.c | 6 ++++++ include/winbase.h | 1 + 2 files changed, 7 insertions(+) diff --git a/dlls/kernelbase/loader.c b/dlls/kernelbase/loader.c index 7aff6bb9412..4d98ee8d309 100644 --- a/dlls/kernelbase/loader.c +++ b/dlls/kernelbase/loader.c @@ -550,6 +550,12 @@ HMODULE WINAPI DECLSPEC_HOTPATCH LoadLibraryExW( LPCWSTR name, HANDLE file, DWOR return 0; } + if (flags & LOAD_PACKAGED_LIBRARY) { + // FIXME: Don't know if it's implemented like this on Windows + // but this does accomplish same result in error case + return LoadPackagedLibrary(name, flags); + } + RtlInitUnicodeString( &str, name ); if (str.Buffer[str.Length/sizeof(WCHAR) - 1] != ' ') return load_library( &str, flags ); diff --git a/include/winbase.h b/include/winbase.h index 16e449f7c2d..3f82d2e0cd4 100644 --- a/include/winbase.h +++ b/include/winbase.h @@ -995,6 +995,7 @@ DECL_WINELIB_TYPE_AW(ENUMRESLANGPROC) /* flags that can be passed to LoadLibraryEx */ #define DONT_RESOLVE_DLL_REFERENCES 0x00000001 #define LOAD_LIBRARY_AS_DATAFILE 0x00000002 +#define LOAD_PACKAGED_LIBRARY 0x00000004 // Undocumented Windows internal #define LOAD_WITH_ALTERED_SEARCH_PATH 0x00000008 #define LOAD_IGNORE_CODE_AUTHZ_LEVEL 0x00000010 #define LOAD_LIBRARY_AS_IMAGE_RESOURCE 0x00000020 -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4587
From: Dāvis Mosāns <davispuh(a)gmail.com> --- dlls/kernelbase/loader.c | 13 +++++++++++++ include/winbase.h | 4 ++++ 2 files changed, 17 insertions(+) diff --git a/dlls/kernelbase/loader.c b/dlls/kernelbase/loader.c index 4d98ee8d309..600b2c967bc 100644 --- a/dlls/kernelbase/loader.c +++ b/dlls/kernelbase/loader.c @@ -550,6 +550,19 @@ HMODULE WINAPI DECLSPEC_HOTPATCH LoadLibraryExW( LPCWSTR name, HANDLE file, DWOR return 0; } + if ((flags & LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR) && + (flags & (LOAD_LIBRARY_AS_DATAFILE | LOAD_LIBRARY_AS_DATAFILE_EXCLUSIVE | LOAD_LIBRARY_AS_IMAGE_RESOURCE))) { + // Don't know why but happens like this on Windows 10 22H2 + SetLastError( ERROR_INVALID_PARAMETER ); + return 0; + } + + if (flags & LOAD_LIBRARY_UNSUPPORTED_MASK) { + WARN( "Pretending to be Windows 10 22H2, unsupported flags: 0x%lx\n, returning ERROR_INVALID_PARAMETER", flags & LOAD_LIBRARY_UNSUPPORTED_MASK ); + SetLastError( ERROR_INVALID_PARAMETER ); + return 0; + } + if (flags & LOAD_PACKAGED_LIBRARY) { // FIXME: Don't know if it's implemented like this on Windows // but this does accomplish same result in error case diff --git a/include/winbase.h b/include/winbase.h index 3f82d2e0cd4..c92d2b9617a 100644 --- a/include/winbase.h +++ b/include/winbase.h @@ -1006,6 +1006,10 @@ DECL_WINELIB_TYPE_AW(ENUMRESLANGPROC) #define LOAD_LIBRARY_SEARCH_USER_DIRS 0x00000400 #define LOAD_LIBRARY_SEARCH_SYSTEM32 0x00000800 #define LOAD_LIBRARY_SEARCH_DEFAULT_DIRS 0x00001000 +#define LOAD_LIBRARY_SAFE_CURRENT_DIRS 0x00002000 +#define LOAD_LIBRARY_SEARCH_SYSTEM32_NO_FORWARDER 0x00004000 +#define LOAD_LIBRARY_OS_INTEGRITY_CONTINUITY 0x00008000 +#define LOAD_LIBRARY_UNSUPPORTED_MASK 0xFFFF0000 // If you add new flags update this mask #define GET_MODULE_HANDLE_EX_FLAG_PIN 1 #define GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT 2 -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4587
btw you should change the comments from // to /* */, since that's the wine convention. Also make the curly braces go on the next line after the if. There also seems to be a lot of repetition, maybe you could do something to get rid of some of it? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4587#note_54814
participants (2)
-
Dāvis Mosāns -
Etaash Mathamsetty (＠etaash.mathamsetty)