Signed-off-by: Craig Schulstad craigaschulstad@gmail.com --- dlls/kernelbase/loader.c | 169 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 164 insertions(+), 5 deletions(-)
diff --git a/dlls/kernelbase/loader.c b/dlls/kernelbase/loader.c index fc9b0ce0083..0fbe9bcd9e8 100644 --- a/dlls/kernelbase/loader.c +++ b/dlls/kernelbase/loader.c @@ -48,6 +48,9 @@ struct exclusive_datafile }; static struct list exclusive_datafile_list = LIST_INIT( exclusive_datafile_list );
+static WCHAR mui_locale[LOCALE_NAME_MAX_LENGTH]; +static BOOL locale_found = 0; +static BOOL recursion_flag = 0;
/*********************************************************************** * Modules @@ -1011,11 +1014,122 @@ BOOL WINAPI DECLSPEC_HOTPATCH EnumResourceTypesExW( HMODULE module, ENUMRESTYPEP return ret; }
+/***********************************************************************/ +/* get_mui - Acquire an MUI module for the associated resource */ +/***********************************************************************/ + +HMODULE get_mui(HMODULE module) + +{ + + HMODULE mui_module = NULL; + + WCHAR module_name[MAX_PATH], mui_name[MAX_PATH]; + + INT i, j, k, l; + + /* Initialize the work strings */ + + for (i = 0; i < MAX_PATH; i++) { + module_name[i] = 0; + mui_name[i] = 0; + } + + /* Note - the reference to the Windows file name for an "MUI" file has a structure such as */ + /* "C:\Program Files\Application Directory\xx-XX\Application.exe.mui"; however, in testing */ + /* out the usage of the "GetModuleFileNameW" function, it was determined that it works with */ + /* a relative Linux file structure such as "xx-XX/Application.exe.mui". */ + + /* Acquire the base resource file name */ + + if (!(GetModuleFileNameW(module, module_name, MAX_PATH))) return module; + + /* Stay with the original module reference if this file is not an executable file. */ + + if (!(wcsstr(module_name, L".exe"))) return module; + + /* Acquire the locale name using LCIDToLocaleName. Since this function utilizes the FindResourceExW function, this */ + /* sets up a recursive call to this function. In order to avoid a stack overflow condition that would be caused by */ + /* repeated calls, a flag will be set on to return back to the FindResourceExW function without again calling the */ + /* locale acquisition function. */ + + if (!(locale_found)) { + + if (recursion_flag) return module; + + recursion_flag = 1; + + LCIDToLocaleName( GetUserDefaultLCID(), mui_locale, LOCALE_NAME_MAX_LENGTH, 0 ); + + recursion_flag = 0; + + locale_found = 1; + + } + + /* Locate the position of the final backslash in the retrieved executable file. */ + + j = 0; + + for (i = 0; i < MAX_PATH; i++) { + + if (module_name[i] == 0) break; + + if (module_name[i] == '\') j = i; + } + + /* Set up the work index that will be used to extract just the executable file from the fully qualified file name. */ + + k = 0; + + for (i = 0; i < MAX_PATH; i++) { + + if (module_name[i] == 0) break; + + /* If work index "j" has been set to -1, then the file portion of the qualified name has been reached and will */ + /* be copied to the "MUI" file reference. */ + + if (j < 0) { + mui_name[k] = module_name[i]; + k++; + } + + /* When the position of the final backslash has been reached, add the locale name as the folder/directory */ + /* containing the "MUI" file and reset work index "j" to -1. */ + + if (i >= j && j > 0) { + for (l = 0; l < 5; l++) { + mui_name[k] = mui_locale[l]; + k++; + } + mui_name[k] = '/'; + k++; + j = -1; + } + } + + /* Finally, append the literal ".mui" onto the file reference. */ + + wcscat(mui_name, L".mui"); + + /* Now, see if there is an associated "MUI" file and if so use its handle for the module handle. */ + + mui_module = LoadLibraryExW(mui_name, 0, 0); + + if (mui_module) { + return mui_module; + } else { + return module; + } + +} + +/***********************************************************************/ +/* get_res_handle - Isolated call of the LdrFindResource function */ +/***********************************************************************/ + +HRSRC get_res_handle(HMODULE module, LPCWSTR type, LPCWSTR name, WORD lang)
-/********************************************************************** - * FindResourceExW (kernelbase.@) - */ -HRSRC WINAPI DECLSPEC_HOTPATCH FindResourceExW( HMODULE module, LPCWSTR type, LPCWSTR name, WORD lang ) { NTSTATUS status; UNICODE_STRING nameW, typeW; @@ -1024,7 +1138,6 @@ HRSRC WINAPI DECLSPEC_HOTPATCH FindResourceExW( HMODULE module, LPCWSTR type, LP
TRACE( "%p %s %s %04x\n", module, debugstr_w(type), debugstr_w(name), lang );
- if (!module) module = GetModuleHandleW( 0 ); nameW.Buffer = typeW.Buffer = NULL;
__TRY @@ -1046,7 +1159,41 @@ HRSRC WINAPI DECLSPEC_HOTPATCH FindResourceExW( HMODULE module, LPCWSTR type, LP
if (!IS_INTRESOURCE(nameW.Buffer)) HeapFree( GetProcessHeap(), 0, nameW.Buffer ); if (!IS_INTRESOURCE(typeW.Buffer)) HeapFree( GetProcessHeap(), 0, typeW.Buffer ); + return (HRSRC)entry; + +} + +/********************************************************************** + * FindResourceExW (kernelbase.@) + */ +HRSRC WINAPI DECLSPEC_HOTPATCH FindResourceExW( HMODULE module, LPCWSTR type, LPCWSTR name, WORD lang ) +{ + + HRSRC rsrc; + + TRACE( "%p %s %s %04x\n", module, debugstr_w(type), debugstr_w(name), lang ); + + if (!module) module = GetModuleHandleW( 0 ); + + rsrc = get_res_handle(module, type, name, lang); + + if (rsrc) { + + return rsrc; + + } else { + + /* If a resource retrieval failed using the initial module value, attempt to */ + /* locate an associated MUI file and retry the resource retrieval. */ + + module = get_mui(module); + + rsrc = get_res_handle(module, type, name, lang); + + return rsrc; + + } }
@@ -1074,11 +1221,23 @@ BOOL WINAPI DECLSPEC_HOTPATCH FreeResource( HGLOBAL handle ) HGLOBAL WINAPI DECLSPEC_HOTPATCH LoadResource( HINSTANCE module, HRSRC rsrc ) { void *ret; + HMODULE mui_module = NULL;
TRACE( "%p %p\n", module, rsrc );
if (!rsrc) return 0; if (!module) module = GetModuleHandleW( 0 ); + + + /* Only check for an MUI reference if the resource handle value is less than the module value, */ + /* or if an MUI reference was found and the MUI reference and handle value are larger than the */ + /* module value for the executable file. That is a signal that the resource handle is to be */ + /* associated with the MUI file instead of the executable file. */ + + mui_module = get_mui(module); + + if (((HMODULE)rsrc < module) || ((mui_module > module) && ((HMODULE)rsrc > mui_module))) module = mui_module; + if (!set_ntstatus( LdrAccessResource( module, (IMAGE_RESOURCE_DATA_ENTRY *)rsrc, &ret, NULL ))) return 0; return ret;
Hello Craig, thanks for the patch, and sorry for the lack of review thus far.
I have a few high-level comments:
(1) It seems to me it should be possible to write some tests for this behaviour, both to prove the implementation is correct and to prevent further regressions. I think the easiest way to do this would be to add a simple empty test DLL (for an example, see fce26e60cc2 and references to "coinst"), to which you can add resources using UpdateResource(). When writing tests, I would recommend to keep in mind my other comments below.
(2) Should this really happen in LoadResource()? Should it instead happen in FindResource(), or even in LdrFindResource_U()?
(3) If yes, how does this interact with the LCID parameter to FindResource()?
On 1/18/21 11:12 AM, Craig Schulstad wrote:
Signed-off-by: Craig Schulstad craigaschulstad@gmail.com
dlls/kernelbase/loader.c | 169 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 164 insertions(+), 5 deletions(-)
diff --git a/dlls/kernelbase/loader.c b/dlls/kernelbase/loader.c index fc9b0ce0083..0fbe9bcd9e8 100644 --- a/dlls/kernelbase/loader.c +++ b/dlls/kernelbase/loader.c @@ -48,6 +48,9 @@ struct exclusive_datafile }; static struct list exclusive_datafile_list = LIST_INIT( exclusive_datafile_list );
+static WCHAR mui_locale[LOCALE_NAME_MAX_LENGTH]; +static BOOL locale_found = 0; +static BOOL recursion_flag = 0;
/***********************************************************************
- Modules
@@ -1011,11 +1014,122 @@ BOOL WINAPI DECLSPEC_HOTPATCH EnumResourceTypesExW( HMODULE module, ENUMRESTYPEP return ret; }
+/***********************************************************************/ +/* get_mui - Acquire an MUI module for the associated resource */ +/***********************************************************************/
+HMODULE get_mui(HMODULE module)
+{
- HMODULE mui_module = NULL;
- WCHAR module_name[MAX_PATH], mui_name[MAX_PATH];
- INT i, j, k, l;
- /* Initialize the work strings */
- for (i = 0; i < MAX_PATH; i++) {
module_name[i] = 0;
mui_name[i] = 0;
- }
- /* Note - the reference to the Windows file name for an "MUI" file has a structure such as */
- /* "C:\Program Files\Application Directory\xx-XX\Application.exe.mui"; however, in testing */
- /* out the usage of the "GetModuleFileNameW" function, it was determined that it works with */
- /* a relative Linux file structure such as "xx-XX/Application.exe.mui". */
- /* Acquire the base resource file name */
This comment, like some others below, feel redundant; the next line tells me as much.
- if (!(GetModuleFileNameW(module, module_name, MAX_PATH))) return module;
- /* Stay with the original module reference if this file is not an executable file. */
- if (!(wcsstr(module_name, L".exe"))) return module;
This is also a good thing to test. It seems at least a little surprising that this would be true.
- /* Acquire the locale name using LCIDToLocaleName. Since this function utilizes the FindResourceExW function, this */
- /* sets up a recursive call to this function. In order to avoid a stack overflow condition that would be caused by */
- /* repeated calls, a flag will be set on to return back to the FindResourceExW function without again calling the */
- /* locale acquisition function. */
- if (!(locale_found)) {
if (recursion_flag) return module;
recursion_flag = 1;
LCIDToLocaleName( GetUserDefaultLCID(), mui_locale, LOCALE_NAME_MAX_LENGTH, 0 );
recursion_flag = 0;
locale_found = 1;
- }
This is not thread safe (it doesn't guarantee ordering); you would need atomics.
I'm wondering if a better solution to this might be to use an internal API in LCIDToLocaleName() [or RtlLcidToLocaleName()], and skip the MUI logic in that case.
- /* Locate the position of the final backslash in the retrieved executable file. */
- j = 0;
- for (i = 0; i < MAX_PATH; i++) {
if (module_name[i] == 0) break;
if (module_name[i] == '\\') j = i;
- }
- /* Set up the work index that will be used to extract just the executable file from the fully qualified file name. */
- k = 0;
- for (i = 0; i < MAX_PATH; i++) {
if (module_name[i] == 0) break;
/* If work index "j" has been set to -1, then the file portion of the qualified name has been reached and will */
/* be copied to the "MUI" file reference. */
if (j < 0) {
mui_name[k] = module_name[i];
k++;
}
/* When the position of the final backslash has been reached, add the locale name as the folder/directory */
/* containing the "MUI" file and reset work index "j" to -1. */
if (i >= j && j > 0) {
for (l = 0; l < 5; l++) {
mui_name[k] = mui_locale[l];
k++;
}
mui_name[k] = '/';
k++;
j = -1;
}
- }
Note that you can use (some) CRT string functions within kernelbase [and ntdll], which may help avoid some of this logic. It's rather difficult to read as-is.
- /* Finally, append the literal ".mui" onto the file reference. */
- wcscat(mui_name, L".mui");
- /* Now, see if there is an associated "MUI" file and if so use its handle for the module handle. */
- mui_module = LoadLibraryExW(mui_name, 0, 0);
- if (mui_module) {
return mui_module;
- } else {
return module;
- }
+}
+/***********************************************************************/ +/* get_res_handle - Isolated call of the LdrFindResource function */ +/***********************************************************************/
+HRSRC get_res_handle(HMODULE module, LPCWSTR type, LPCWSTR name, WORD lang)
-/**********************************************************************
FindResourceExW (kernelbase.@)
- */
-HRSRC WINAPI DECLSPEC_HOTPATCH FindResourceExW( HMODULE module, LPCWSTR type, LPCWSTR name, WORD lang ) { NTSTATUS status; UNICODE_STRING nameW, typeW; @@ -1024,7 +1138,6 @@ HRSRC WINAPI DECLSPEC_HOTPATCH FindResourceExW( HMODULE module, LPCWSTR type, LP
TRACE( "%p %s %s %04x\n", module, debugstr_w(type), debugstr_w(name), lang );
if (!module) module = GetModuleHandleW( 0 ); nameW.Buffer = typeW.Buffer = NULL;
__TRY
@@ -1046,7 +1159,41 @@ HRSRC WINAPI DECLSPEC_HOTPATCH FindResourceExW( HMODULE module, LPCWSTR type, LP
if (!IS_INTRESOURCE(nameW.Buffer)) HeapFree( GetProcessHeap(), 0, nameW.Buffer ); if (!IS_INTRESOURCE(typeW.Buffer)) HeapFree( GetProcessHeap(), 0, typeW.Buffer );
return (HRSRC)entry;
+}
+/**********************************************************************
FindResourceExW (kernelbase.@)
- */
+HRSRC WINAPI DECLSPEC_HOTPATCH FindResourceExW( HMODULE module, LPCWSTR type, LPCWSTR name, WORD lang ) +{
- HRSRC rsrc;
- TRACE( "%p %s %s %04x\n", module, debugstr_w(type), debugstr_w(name), lang );
- if (!module) module = GetModuleHandleW( 0 );
- rsrc = get_res_handle(module, type, name, lang);
- if (rsrc) {
return rsrc;
- } else {
/* If a resource retrieval failed using the initial module value, attempt to */
/* locate an associated MUI file and retry the resource retrieval. */
module = get_mui(module);
Here and below, "module" is effectively leaked. I guess this is necessarily true, but is that module really never unloaded?
rsrc = get_res_handle(module, type, name, lang);
return rsrc;
- } }
@@ -1074,11 +1221,23 @@ BOOL WINAPI DECLSPEC_HOTPATCH FreeResource( HGLOBAL handle ) HGLOBAL WINAPI DECLSPEC_HOTPATCH LoadResource( HINSTANCE module, HRSRC rsrc ) { void *ret;
HMODULE mui_module = NULL;
TRACE( "%p %p\n", module, rsrc );
if (!rsrc) return 0; if (!module) module = GetModuleHandleW( 0 );
/* Only check for an MUI reference if the resource handle value is less than the module value, */
/* or if an MUI reference was found and the MUI reference and handle value are larger than the */
/* module value for the executable file. That is a signal that the resource handle is to be */
/* associated with the MUI file instead of the executable file. */
mui_module = get_mui(module);
if (((HMODULE)rsrc < module) || ((mui_module > module) && ((HMODULE)rsrc > mui_module))) module = mui_module;
This is very confusing. Why would the value of "rsrc" matter?
if (!set_ntstatus( LdrAccessResource( module, (IMAGE_RESOURCE_DATA_ENTRY *)rsrc, &ret, NULL ))) return 0; return ret;