[PATCH 0/3] MR6178: ntdll: Use environ/_NSGetEnviron() directly rather than caching it in main_envp.
Fixes an unpredictable use-after-free crash on launch on macOS. Also, use `_NSGetEnviron()` instead of `environ` in `msv1_0` and `ntdll`. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=57005 -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6178
From: Brendan Shanks <bshanks(a)codeweavers.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=57005 --- dlls/ntdll/unix/env.c | 9 ++++++--- dlls/ntdll/unix/loader.c | 4 +--- dlls/ntdll/unix/unix_private.h | 1 - loader/main.c | 6 ++---- 4 files changed, 9 insertions(+), 11 deletions(-) diff --git a/dlls/ntdll/unix/env.c b/dlls/ntdll/unix/env.c index 36949b905fb..f95bf737f56 100644 --- a/dlls/ntdll/unix/env.c +++ b/dlls/ntdll/unix/env.c @@ -44,6 +44,10 @@ #ifdef __APPLE__ # include <CoreFoundation/CFLocale.h> # include <CoreFoundation/CFString.h> +# include <crt_externs.h> +# define environ (*_NSGetEnviron()) +#else + extern char **environ; #endif #include "ntstatus.h" @@ -70,7 +74,6 @@ static const WCHAR bootstrapW[] = {'W','I','N','E','B','O','O','T','S','T','R',' int main_argc = 0; char **main_argv = NULL; -char **main_envp = NULL; WCHAR **main_wargv = NULL; static LCID user_lcid, system_lcid; @@ -931,12 +934,12 @@ static WCHAR *get_initial_environment( SIZE_T *pos, SIZE_T *size ) /* estimate needed size */ *size = 1; - for (e = main_envp; *e; e++) *size += strlen(*e) + 1; + for (e = environ; *e; e++) *size += strlen(*e) + 1; env = malloc( *size * sizeof(WCHAR) ); ptr = env; end = env + *size - 1; - for (e = main_envp; *e && ptr < end; e++) + for (e = environ; *e && ptr < end; e++) { char *str = *e; diff --git a/dlls/ntdll/unix/loader.c b/dlls/ntdll/unix/loader.c index 1267469bf37..b7dfb5b3528 100644 --- a/dlls/ntdll/unix/loader.c +++ b/dlls/ntdll/unix/loader.c @@ -1953,7 +1953,6 @@ static jstring wine_init_jni( JNIEnv *env, jobject obj, jobjectArray cmdline, jo main_argc = argc; main_argv = argv; - main_envp = environ; init_paths( argv ); virtual_init(); @@ -2149,11 +2148,10 @@ static void check_command_line( int argc, char *argv[] ) * * Main entry point called by the wine loader. */ -DECLSPEC_EXPORT void __wine_main( int argc, char *argv[], char *envp[] ) +DECLSPEC_EXPORT void __wine_main( int argc, char *argv[] ) { main_argc = argc; main_argv = argv; - main_envp = envp; init_paths( argv ); diff --git a/dlls/ntdll/unix/unix_private.h b/dlls/ntdll/unix/unix_private.h index 91267f66e76..ee5630e76f1 100644 --- a/dlls/ntdll/unix/unix_private.h +++ b/dlls/ntdll/unix/unix_private.h @@ -172,7 +172,6 @@ extern SIZE_T startup_info_size; extern BOOL is_prefix_bootstrap; extern int main_argc; extern char **main_argv; -extern char **main_envp; extern WCHAR **main_wargv; extern const WCHAR system_dir[]; extern unsigned int supported_machines_count; diff --git a/loader/main.c b/loader/main.c index e5de4750015..ecb51efa136 100644 --- a/loader/main.c +++ b/loader/main.c @@ -37,8 +37,6 @@ #include "main.h" -extern char **environ; - #if defined(__APPLE__) && defined(__x86_64__) && !defined(HAVE_WINE_PRELOADER) /* Not using the preloader on x86_64: @@ -260,8 +258,8 @@ int main( int argc, char *argv[] ) if ((handle = load_ntdll( argv[0] ))) { - void (*init_func)(int, char **, char **) = dlsym( handle, "__wine_main" ); - if (init_func) init_func( argc, argv, environ ); + void (*init_func)(int, char **) = dlsym( handle, "__wine_main" ); + if (init_func) init_func( argc, argv ); fprintf( stderr, "wine: __wine_main function not found in ntdll.so\n" ); exit(1); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6178
From: Brendan Shanks <bshanks(a)codeweavers.com> --- dlls/ntdll/unix/loader.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dlls/ntdll/unix/loader.c b/dlls/ntdll/unix/loader.c index b7dfb5b3528..92f2e2eb3a3 100644 --- a/dlls/ntdll/unix/loader.c +++ b/dlls/ntdll/unix/loader.c @@ -73,11 +73,13 @@ # ifndef _POSIX_SPAWN_DISABLE_ASLR # define _POSIX_SPAWN_DISABLE_ASLR 0x0100 # endif +# define environ (*_NSGetEnviron()) +#else + extern char **environ; #endif #ifdef __ANDROID__ # include <jni.h> #endif -extern char **environ; #include "ntstatus.h" #define WIN32_NO_STATUS -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6178
From: Brendan Shanks <bshanks(a)codeweavers.com> --- dlls/msv1_0/unixlib.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/dlls/msv1_0/unixlib.c b/dlls/msv1_0/unixlib.c index 807f3778109..95c92233b37 100644 --- a/dlls/msv1_0/unixlib.c +++ b/dlls/msv1_0/unixlib.c @@ -40,7 +40,12 @@ #include "wine/debug.h" #include "unixlib.h" -extern char **environ; +#ifdef __APPLE__ +# include <crt_externs.h> +# define environ (*_NSGetEnviron()) +#else + extern char **environ; +#endif WINE_DEFAULT_DEBUG_CHANNEL(ntlm); WINE_DECLARE_DEBUG_CHANNEL(winediag); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6178
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=147451 Your paranoid android. === debian11 (build log) === error: patch failed: dlls/ntdll/unix/env.c:44 error: patch failed: dlls/ntdll/unix/loader.c:1953 error: patch failed: dlls/ntdll/unix/unix_private.h:172 error: patch failed: loader/main.c:37 error: patch failed: dlls/ntdll/unix/loader.c:73 error: patch failed: dlls/msv1_0/unixlib.c:40 Task: Patch failed to apply === debian11b (64 bit WoW report) === Report validation errors: dxgi:dxgi has unaccounted for todo messages dxgi:dxgi has unaccounted for skip messages The report seems to have been truncated
participants (3)
-
Brendan Shanks -
Brendan Shanks (＠bshanks)
-
Marvin