Staging WRITECOPY fixes
DodoGTA GT <[email protected]>
| Newsgroups | gmane.comp.emulators.wine.devel |
|---|---|
| Message-ID | <CAKNqjs2A1OBb21Q2QruaaT4yDAjv-dixrch_2JFKwrm0cXnX+A@mail.gmail.com> |
Hello, I found a few issues with the WRITECOPY patch after wine-staging commit ae46f56f2cf20585387ff61cb7b36fd5f3d84837 which I managed to pretty much fix Patch 0003 is a replacement for the previous 0003 patch (apparently very early signal handling is still needed for some Unix-side ntdll functions); the added early argument checks could be replaced with NtCurrentTeb() instead (this would lead to 2 identical signal_init_process() calls though which may look weird) Patch 0004 updates the previous 0004 patch by removing a forgotten i386 compile-time check (which didn't make sense after that wine-staging commit) Patch 0005 is updated to fix conflicts because of the patch 0004 update (so it can be ignored for a manual rebase) Patch 0010 enables the WRITECOPY stuff by default (this is effectively a replacement for patches 0010 and 0011 without their hacky logic which really only works for Chromium-based applications); if this patch addition is too radical, then at least patches 0010 and 0011 should definitely be removed (hopefully there won't be another patch race anytime soon) Please note that patch 0003 also adds an extra ntdll-Syscall_Emulation dependency to fix a bad interaction with that patchset (also I should've sent this email at least a day earlier but hopefully some of these patches can get pushed) Thanks
0010-ntdll-Enable-WRITECOPY-support-by-default.patch
(text/x-patch, 1.5 KB)
From 66aadf72fd7985086f9852975ed5207f116fa9c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aida=20Jonikien=C4=97?= <[email protected]> Date: Fri, 20 Sep 2024 19:26:10 +0300 Subject: [PATCH] ntdll: Enable WRITECOPY support by default. My decent amount of testing shows the experimental WRITECOPY patch is probably good enough for wider usage (but there's a toggle for disabling it just in case wine-staging 9.18 becomes too broken). --- dlls/ntdll/unix/virtual.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dlls/ntdll/unix/virtual.c b/dlls/ntdll/unix/virtual.c index ce103baca77..e828934991f 100644 --- a/dlls/ntdll/unix/virtual.c +++ b/dlls/ntdll/unix/virtual.c @@ -508,15 +508,15 @@ static void reserve_area( void *addr, void *end ) /* This might look like a hack, but it actually isn't - the 'experimental' version * is correct, but it already has revealed a couple of additional Wine bugs, which * were not triggered before, and there are probably some more. - * To avoid breaking Wine for everyone, the new correct implementation has to be - * manually enabled, until it is tested a bit more. */ + * To avoid breaking Wine for everyone, the new correct implementation has a + * toggle for disabling it until it is tested even more. */ static inline BOOL experimental_WRITECOPY( void ) { static int enabled = -1; if (enabled == -1) { const char *str = getenv("STAGING_WRITECOPY"); - enabled = str && (atoi(str) != 0); + enabled = str ? (atoi(str) != 0) : 1; } return enabled; } -- 2.46.1
0003-ntdll-Add-an-early-path-for-the-existing-signal-hand.patch
(text/x-patch, 7.6 KB)
From 4745cca6862e3fad7cef5c7fb2dcea33e1185b18 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aida=20Jonikien=C4=97?= <[email protected]> Date: Wed, 11 Sep 2024 20:23:12 +0300 Subject: [PATCH 3/5] ntdll: Add an early path for the existing signal handler functions. The signal handlers still need to be installed extremely early to handle page faults in some ntdll Unix-side functions (despite the 0003 patch saying otherwise). --- dlls/ntdll/unix/loader.c | 1 + dlls/ntdll/unix/server.c | 2 +- dlls/ntdll/unix/signal_arm.c | 10 +++++++--- dlls/ntdll/unix/signal_arm64.c | 10 +++++++--- dlls/ntdll/unix/signal_i386.c | 14 +++++++++----- dlls/ntdll/unix/signal_x86_64.c | 19 ++++++++++++------- dlls/ntdll/unix/unix_private.h | 2 +- 7 files changed, 38 insertions(+), 20 deletions(-) diff --git a/dlls/ntdll/unix/loader.c b/dlls/ntdll/unix/loader.c index 92f2e2eb3a3..302c6a76e66 100644 --- a/dlls/ntdll/unix/loader.c +++ b/dlls/ntdll/unix/loader.c @@ -2180,6 +2180,7 @@ DECLSPEC_EXPORT void __wine_main( int argc, char *argv[] ) #endif virtual_init(); + signal_init_process( TRUE ); init_environment(); #ifdef __APPLE__ diff --git a/dlls/ntdll/unix/server.c b/dlls/ntdll/unix/server.c index 27dbf1331aa..e65df4dec6a 100644 --- a/dlls/ntdll/unix/server.c +++ b/dlls/ntdll/unix/server.c @@ -1677,7 +1677,7 @@ void server_init_process_done(void) /* Install signal handlers; this cannot be done earlier, since we cannot * send exceptions to the debugger before the create process event that * is sent by init_process_done */ - signal_init_process(); + signal_init_process( FALSE ); /* always send the native TEB */ if (!(teb = NtCurrentTeb64())) teb = NtCurrentTeb(); diff --git a/dlls/ntdll/unix/signal_arm.c b/dlls/ntdll/unix/signal_arm.c index 9cc1a49f6c1..dfcf84f08ce 100644 --- a/dlls/ntdll/unix/signal_arm.c +++ b/dlls/ntdll/unix/signal_arm.c @@ -1078,12 +1078,16 @@ void signal_free_thread( TEB *teb ) /********************************************************************** * signal_init_process */ -void signal_init_process(void) +void signal_init_process( BOOL early ) { struct sigaction sig_act; - void *kernel_stack = (char *)ntdll_get_thread_data()->kernel_stack + kernel_stack_size; + void *kernel_stack; - arm_thread_data()->syscall_frame = (struct syscall_frame *)kernel_stack - 1; + if (!early) + { + kernel_stack = (char *)ntdll_get_thread_data()->kernel_stack + kernel_stack_size; + arm_thread_data()->syscall_frame = (struct syscall_frame *)kernel_stack - 1; + } sig_act.sa_mask = server_block_set; sig_act.sa_flags = SA_RESTART | SA_SIGINFO | SA_ONSTACK; diff --git a/dlls/ntdll/unix/signal_arm64.c b/dlls/ntdll/unix/signal_arm64.c index 24bbdc584cd..118bdb38791 100644 --- a/dlls/ntdll/unix/signal_arm64.c +++ b/dlls/ntdll/unix/signal_arm64.c @@ -1334,12 +1334,16 @@ void signal_free_thread( TEB *teb ) /********************************************************************** * signal_init_process */ -void signal_init_process(void) +void signal_init_process( BOOL early ) { struct sigaction sig_act; - void *kernel_stack = (char *)ntdll_get_thread_data()->kernel_stack + kernel_stack_size; + void *kernel_stack; - arm64_thread_data()->syscall_frame = (struct syscall_frame *)kernel_stack - 1; + if (!early) + { + kernel_stack = (char *)ntdll_get_thread_data()->kernel_stack + kernel_stack_size; + arm64_thread_data()->syscall_frame = (struct syscall_frame *)kernel_stack - 1; + } sig_act.sa_mask = server_block_set; sig_act.sa_flags = SA_SIGINFO | SA_RESTART | SA_ONSTACK; diff --git a/dlls/ntdll/unix/signal_i386.c b/dlls/ntdll/unix/signal_i386.c index b727a4d5d6a..296d13973de 100644 --- a/dlls/ntdll/unix/signal_i386.c +++ b/dlls/ntdll/unix/signal_i386.c @@ -2444,14 +2444,18 @@ void signal_free_thread( TEB *teb ) /********************************************************************** * signal_init_process */ -void signal_init_process(void) +void signal_init_process( BOOL early ) { struct sigaction sig_act; - void *kernel_stack = (char *)ntdll_get_thread_data()->kernel_stack + kernel_stack_size; + void *kernel_stack; - x86_thread_data()->syscall_frame = (struct syscall_frame *)((ULONG_PTR)((char *)kernel_stack - - sizeof(struct syscall_frame) - xstate_features_size) & ~(ULONG_PTR)63); - x86_thread_data()->xstate_features_size = xstate_features_size; + if (!early) + { + kernel_stack = (char *)ntdll_get_thread_data()->kernel_stack + kernel_stack_size; + x86_thread_data()->syscall_frame = (struct syscall_frame *)((ULONG_PTR)((char *)kernel_stack + - sizeof(struct syscall_frame) - xstate_features_size) & ~(ULONG_PTR)63); + x86_thread_data()->xstate_features_size = xstate_features_size; + } if (cpu_info.ProcessorFeatureBits & CPU_FEATURE_FXSR) syscall_flags |= SYSCALL_HAVE_FXSAVE; if (cpu_info.ProcessorFeatureBits & CPU_FEATURE_XSAVE) syscall_flags |= SYSCALL_HAVE_XSAVE; diff --git a/dlls/ntdll/unix/signal_x86_64.c b/dlls/ntdll/unix/signal_x86_64.c index 37e2ed74a08..84cb2219f36 100644 --- a/dlls/ntdll/unix/signal_x86_64.c +++ b/dlls/ntdll/unix/signal_x86_64.c @@ -2610,15 +2610,20 @@ static void *mac_thread_gsbase(void) /********************************************************************** * signal_init_process */ -void signal_init_process(void) +void signal_init_process( BOOL early ) { struct sigaction sig_act; - WOW_TEB *wow_teb = get_wow_teb( NtCurrentTeb() ); - void *ptr, *kernel_stack = (char *)ntdll_get_thread_data()->kernel_stack + kernel_stack_size; + WOW_TEB *wow_teb = NULL; + void *ptr, *kernel_stack; - amd64_thread_data()->syscall_frame = (struct syscall_frame *)((ULONG_PTR)((char *)kernel_stack - - sizeof(struct syscall_frame) - xstate_features_size) & ~(ULONG_PTR)63); - amd64_thread_data()->xstate_features_size = xstate_features_size; + if (!early) + { + wow_teb = get_wow_teb( NtCurrentTeb() ); + kernel_stack = (char *)ntdll_get_thread_data()->kernel_stack + kernel_stack_size; + amd64_thread_data()->syscall_frame = (struct syscall_frame *)((ULONG_PTR)((char *)kernel_stack + - sizeof(struct syscall_frame) - xstate_features_size) & ~(ULONG_PTR)63); + amd64_thread_data()->xstate_features_size = xstate_features_size; + } /* sneak in a syscall dispatcher pointer at a fixed address (7ffe1000) */ ptr = (char *)user_shared_data + page_size; @@ -2690,7 +2695,7 @@ void signal_init_process(void) if (sigaction( SIGSEGV, &sig_act, NULL ) == -1) goto error; if (sigaction( SIGILL, &sig_act, NULL ) == -1) goto error; if (sigaction( SIGBUS, &sig_act, NULL ) == -1) goto error; - install_bpf(&sig_act); + if (!early) install_bpf(&sig_act); return; error: diff --git a/dlls/ntdll/unix/unix_private.h b/dlls/ntdll/unix/unix_private.h index 8ce45dfa0bc..b94fc8e14b2 100644 --- a/dlls/ntdll/unix/unix_private.h +++ b/dlls/ntdll/unix/unix_private.h @@ -311,7 +311,7 @@ extern BOOL get_thread_times( int unix_pid, int unix_tid, LARGE_INTEGER *kernel_ extern void signal_init_threading(void); extern NTSTATUS signal_alloc_thread( TEB *teb ); extern void signal_free_thread( TEB *teb ); -extern void signal_init_process(void); +extern void signal_init_process( BOOL early ); extern void DECLSPEC_NORETURN signal_start_thread( PRTL_THREAD_START_ROUTINE entry, void *arg, BOOL suspend, TEB *teb ); extern SYSTEM_SERVICE_TABLE KeServiceDescriptorTable[4]; -- 2.46.1
0004-ntdll-Properly-handle-PAGE_WRITECOPY-protection.-try.patch
(text/x-patch, 4.1 KB)
From 3b61dd262c84d7ef7b663d9733c8ab973c3495d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20M=C3=BCller?= <[email protected]> Date: Sat, 4 Oct 2014 03:22:09 +0200 Subject: [PATCH 4/5] ntdll: Properly handle PAGE_WRITECOPY protection. (try 5) For now, only enable it when a special environment variable is set. --- dlls/ntdll/unix/virtual.c | 41 ++++++++++++++++++++++++++++++++------- 1 file changed, 34 insertions(+), 7 deletions(-) diff --git a/dlls/ntdll/unix/virtual.c b/dlls/ntdll/unix/virtual.c index e168eed8c37..f155c992b77 100644 --- a/dlls/ntdll/unix/virtual.c +++ b/dlls/ntdll/unix/virtual.c @@ -504,6 +504,21 @@ static void reserve_area( void *addr, void *end ) #endif /* __APPLE__ */ } +/* This might look like a hack, but it actually isn't - the 'experimental' version + * is correct, but it already has revealed a couple of additional Wine bugs, which + * were not triggered before, and there are probably some more. + * To avoid breaking Wine for everyone, the new correct implementation has to be + * manually enabled, until it is tested a bit more. */ +static inline BOOL experimental_WRITECOPY( void ) +{ + static int enabled = -1; + if (enabled == -1) + { + const char *str = getenv("STAGING_WRITECOPY"); + enabled = str && (atoi(str) != 0); + } + return enabled; +} static void mmap_init( const struct preload_info *preload_info ) { @@ -1134,8 +1149,14 @@ static int get_unix_prot( BYTE vprot ) { if (vprot & VPROT_READ) prot |= PROT_READ; if (vprot & VPROT_WRITE) prot |= PROT_WRITE | PROT_READ; - if (vprot & VPROT_WRITECOPY) prot |= PROT_WRITE | PROT_READ; if (vprot & VPROT_EXEC) prot |= PROT_EXEC | PROT_READ; + if (vprot & VPROT_WRITECOPY) + { + if (experimental_WRITECOPY()) + prot = (prot & ~PROT_WRITE) | PROT_READ; + else + prot |= PROT_WRITE | PROT_READ; + } if (vprot & VPROT_WRITEWATCH) prot &= ~PROT_WRITE; } if (!prot) prot = PROT_NONE; @@ -1813,7 +1834,7 @@ static void update_write_watches( void *base, size_t size, size_t accessed_size { TRACE( "updating watch %p-%p-%p\n", base, (char *)base + accessed_size, (char *)base + size ); /* clear write watch flag on accessed pages */ - set_page_vprot_bits( base, accessed_size, 0, VPROT_WRITEWATCH ); + set_page_vprot_bits( base, accessed_size, VPROT_WRITE, VPROT_WRITEWATCH | VPROT_WRITECOPY ); /* restore page protections on the entire range */ mprotect_range( base, size, 0, 0 ); } @@ -4064,12 +4085,13 @@ NTSTATUS virtual_handle_fault( EXCEPTION_RECORD *rec, void *stack ) mprotect_range( page, page_size, 0, 0 ); } } - /* ignore fault if page is writable now */ - if (get_unix_prot( get_page_vprot( page )) & PROT_WRITE) + if (vprot & VPROT_WRITECOPY) { - if ((vprot & VPROT_WRITEWATCH) || is_write_watch_range( page, page_size )) - ret = STATUS_SUCCESS; + set_page_vprot_bits( page, page_size, VPROT_WRITE, VPROT_WRITECOPY ); + mprotect_range( page, page_size, 0, 0 ); } + /* ignore fault if page is writable now */ + if (get_unix_prot( get_page_vprot( page ) ) & PROT_WRITE) ret = STATUS_SUCCESS; } mutex_unlock( &virtual_mutex ); rec->ExceptionCode = ret; @@ -4143,11 +4165,16 @@ static NTSTATUS check_write_access( void *base, size_t size, BOOL *has_write_wat { BYTE vprot = get_page_vprot( addr + i ); if (vprot & VPROT_WRITEWATCH) *has_write_watch = TRUE; + if (vprot & VPROT_WRITECOPY) + { + vprot = (vprot & ~VPROT_WRITECOPY) | VPROT_WRITE; + *has_write_watch = TRUE; + } if (!(get_unix_prot( vprot & ~VPROT_WRITEWATCH ) & PROT_WRITE)) return STATUS_INVALID_USER_BUFFER; } if (*has_write_watch) - mprotect_range( addr, size, 0, VPROT_WRITEWATCH ); /* temporarily enable write access */ + mprotect_range( addr, size, VPROT_WRITE, VPROT_WRITEWATCH | VPROT_WRITECOPY ); /* temporarily enable write access */ return STATUS_SUCCESS; } -- 2.46.1
0005-ntdll-Track-if-a-WRITECOPY-page-has-been-modified.patch
(text/x-patch, 3.7 KB)
From e7225bb0ddf040ab9a5b99b01e0ddf1005c54fa5 Mon Sep 17 00:00:00 2001 From: Andrew Wesie <[email protected]> Date: Fri, 24 Apr 2020 14:55:14 -0500 Subject: [PATCH 5/5] ntdll: Track if a WRITECOPY page has been modified. Once a WRITECOPY page is modified, it should be mapped as if it is a normal read-write page. Signed-off-by: Andrew Wesie <[email protected]> --- dlls/ntdll/unix/virtual.c | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/dlls/ntdll/unix/virtual.c b/dlls/ntdll/unix/virtual.c index f155c992b77..ce103baca77 100644 --- a/dlls/ntdll/unix/virtual.c +++ b/dlls/ntdll/unix/virtual.c @@ -122,6 +122,7 @@ struct file_view #define VPROT_GUARD 0x10 #define VPROT_COMMITTED 0x20 #define VPROT_WRITEWATCH 0x40 +#define VPROT_WRITTEN 0x80 /* per-mapping protection flags */ #define VPROT_ARM64EC 0x0100 /* view may contain ARM64EC code */ #define VPROT_SYSTEM 0x0200 /* system view (underlying mmap not under our control) */ @@ -1152,7 +1153,7 @@ static int get_unix_prot( BYTE vprot ) if (vprot & VPROT_EXEC) prot |= PROT_EXEC | PROT_READ; if (vprot & VPROT_WRITECOPY) { - if (experimental_WRITECOPY()) + if (experimental_WRITECOPY() && !(vprot & VPROT_WRITTEN)) prot = (prot & ~PROT_WRITE) | PROT_READ; else prot |= PROT_WRITE | PROT_READ; @@ -1663,7 +1664,11 @@ static NTSTATUS create_view( struct file_view **view_ret, void *base, size_t siz */ static DWORD get_win32_prot( BYTE vprot, unsigned int map_prot ) { - DWORD ret = VIRTUAL_Win32Flags[vprot & 0x0f]; + DWORD ret; + + if ((vprot & VPROT_WRITECOPY) && (vprot & VPROT_WRITTEN)) + vprot = (vprot & ~VPROT_WRITECOPY) | VPROT_WRITE; + ret = VIRTUAL_Win32Flags[vprot & 0x0f]; if (vprot & VPROT_GUARD) ret |= PAGE_GUARD; if (map_prot & SEC_NOCACHE) ret |= PAGE_NOCACHE; return ret; @@ -1769,16 +1774,29 @@ static void mprotect_range( void *base, size_t size, BYTE set, BYTE clear ) */ static BOOL set_vprot( struct file_view *view, void *base, size_t size, BYTE vprot ) { + int unix_prot; + if (view->protect & VPROT_WRITEWATCH) { /* each page may need different protections depending on write watch flag */ - set_page_vprot_bits( base, size, vprot & ~VPROT_WRITEWATCH, ~vprot & ~VPROT_WRITEWATCH ); + set_page_vprot_bits( base, size, vprot & ~VPROT_WRITEWATCH, ~vprot & ~(VPROT_WRITEWATCH|VPROT_WRITTEN) ); mprotect_range( base, size, 0, 0 ); return TRUE; } + if (enable_write_exceptions && is_vprot_exec_write( vprot )) vprot |= VPROT_WRITEWATCH; - if (mprotect_exec( base, size, get_unix_prot(vprot) )) return FALSE; - set_page_vprot( base, size, vprot ); + unix_prot = get_unix_prot(vprot); + + /* check that we can map this memory with PROT_WRITE since we cannot fail later */ + if (vprot & VPROT_WRITECOPY) + unix_prot |= PROT_WRITE; + + if (mprotect_exec( base, size, unix_prot )) return FALSE; + /* each page may need different protections depending on writecopy */ + set_page_vprot_bits( base, size, vprot, ~vprot & ~VPROT_WRITTEN ); + if (vprot & VPROT_WRITECOPY) + mprotect_range( base, size, 0, 0 ); + return TRUE; } @@ -4087,7 +4105,7 @@ NTSTATUS virtual_handle_fault( EXCEPTION_RECORD *rec, void *stack ) } if (vprot & VPROT_WRITECOPY) { - set_page_vprot_bits( page, page_size, VPROT_WRITE, VPROT_WRITECOPY ); + set_page_vprot_bits( page, page_size, VPROT_WRITE | VPROT_WRITTEN, VPROT_WRITECOPY ); mprotect_range( page, page_size, 0, 0 ); } /* ignore fault if page is writable now */ -- 2.46.1