From ffa55dcc0033ee0c366ff43fdbd4b98a367f511b Mon Sep 17 00:00:00 2001 From: Sebastian Lackner Date: Thu, 10 Dec 2015 06:03:53 +0100 Subject: [PATCH] Added patch to avoid holding reference on parent process in wineserver. --- README.md | 2 +- patches/patchinstall.sh | 20 ++-- ...ror-when-opening-a-terminating-proce.patch | 57 --------- patches/server-OpenProcess/definition | 4 - ...not-hold-reference-on-parent-process.patch | 113 ++++++++++++++++++ ...size-of-PID-table-to-512-to-reduce-r.patch | 26 ++++ patches/server-Parent_Process/definition | 1 + staging/changelog | 3 + 8 files changed, 155 insertions(+), 71 deletions(-) delete mode 100644 patches/server-OpenProcess/0001-server-Return-error-when-opening-a-terminating-proce.patch delete mode 100644 patches/server-OpenProcess/definition create mode 100644 patches/server-Parent_Process/0001-server-Do-not-hold-reference-on-parent-process.patch create mode 100644 patches/server-Parent_Process/0002-server-Increase-size-of-PID-table-to-512-to-reduce-r.patch create mode 100644 patches/server-Parent_Process/definition diff --git a/README.md b/README.md index b42d44a4..174747e7 100644 --- a/README.md +++ b/README.md @@ -120,6 +120,7 @@ for more details.* * Do not allow interruption of system APC in server_select ([Wine Bug #14697](https://bugs.winehq.org/show_bug.cgi?id=14697)) * Do not allow to deallocate thread stack for current thread * Do not fail when a used context is passed to wglShareLists ([Wine Bug #11436](https://bugs.winehq.org/show_bug.cgi?id=11436)) +* Do not hold reference on parent process in wineserver ([Wine Bug #37087](https://bugs.winehq.org/show_bug.cgi?id=37087)) * Do not signal threads until they are really gone * Do not use unixfs for devices without mountpoint * Do not wait for hook thread startup in IDirectInput8::Initialize ([Wine Bug #21403](https://bugs.winehq.org/show_bug.cgi?id=21403)) @@ -247,7 +248,6 @@ for more details.* * Return STATUS_INVALID_DEVICE_REQUEST when trying to call NtReadFile on directory * Return WN_NOT_CONNECTED from WNetGetUniversalName REMOTE_NAME_INFO_LEVEL stub ([Wine Bug #39452](https://bugs.winehq.org/show_bug.cgi?id=39452)) * Return a valid mesh in D3DXCreateTeapot ([Wine Bug #36884](https://bugs.winehq.org/show_bug.cgi?id=36884)) -* Return an error when trying to open a terminated process ([Wine Bug #37087](https://bugs.winehq.org/show_bug.cgi?id=37087)) * Return correct IMediaSeeking stream positions in quartz * Return correct values for GetThreadTimes function ([Wine Bug #20230](https://bugs.winehq.org/show_bug.cgi?id=20230)) * Return dummy ID3DXSkinInfo interface when skinning info not present ([Wine Bug #33904](https://bugs.winehq.org/show_bug.cgi?id=33904)) diff --git a/patches/patchinstall.sh b/patches/patchinstall.sh index 3c40d751..000e5c8d 100755 --- a/patches/patchinstall.sh +++ b/patches/patchinstall.sh @@ -245,7 +245,7 @@ patch_enable_all () enable_server_Key_State="$1" enable_server_Map_EXDEV_Error="$1" enable_server_Misc_ACL="$1" - enable_server_OpenProcess="$1" + enable_server_Parent_Process="$1" enable_server_PeekMessage="$1" enable_server_Pipe_ObjectName="$1" enable_server_Realtime_Priority="$1" @@ -848,8 +848,8 @@ patch_enable () server-Misc_ACL) enable_server_Misc_ACL="$2" ;; - server-OpenProcess) - enable_server_OpenProcess="$2" + server-Parent_Process) + enable_server_Parent_Process="$2" ;; server-PeekMessage) enable_server_PeekMessage="$2" @@ -4995,18 +4995,20 @@ if test "$enable_server_Map_EXDEV_Error" -eq 1; then ) >> "$patchlist" fi -# Patchset server-OpenProcess +# Patchset server-Parent_Process # | # | This patchset fixes the following Wine bugs: -# | * [#37087] Return an error when trying to open a terminated process +# | * [#37087] Do not hold reference on parent process in wineserver # | # | Modified files: -# | * server/process.c, server/process.h +# | * server/console.c, server/process.c, server/process.h, server/snapshot.c, server/thread.c # | -if test "$enable_server_OpenProcess" -eq 1; then - patch_apply server-OpenProcess/0001-server-Return-error-when-opening-a-terminating-proce.patch +if test "$enable_server_Parent_Process" -eq 1; then + patch_apply server-Parent_Process/0001-server-Do-not-hold-reference-on-parent-process.patch + patch_apply server-Parent_Process/0002-server-Increase-size-of-PID-table-to-512-to-reduce-r.patch ( - echo '+ { "Michael Müller", "server: Return error when opening a terminating process.", 3 },'; + echo '+ { "Sebastian Lackner", "server: Do not hold reference on parent process.", 1 },'; + echo '+ { "Sebastian Lackner", "server: Increase size of PID table to 512 to reduce risk of collisions.", 1 },'; ) >> "$patchlist" fi diff --git a/patches/server-OpenProcess/0001-server-Return-error-when-opening-a-terminating-proce.patch b/patches/server-OpenProcess/0001-server-Return-error-when-opening-a-terminating-proce.patch deleted file mode 100644 index eb42ce84..00000000 --- a/patches/server-OpenProcess/0001-server-Return-error-when-opening-a-terminating-proce.patch +++ /dev/null @@ -1,57 +0,0 @@ -From bc7cb674c2b15f6dc603b9271a684d6aefe3a7d8 Mon Sep 17 00:00:00 2001 -From: =?UTF-8?q?Michael=20M=C3=BCller?= -Date: Thu, 14 Aug 2014 03:05:52 +0200 -Subject: server: Return error when opening a terminating process. (try 3) - ---- - server/process.c | 7 ++++++- - server/process.h | 1 + - 2 files changed, 7 insertions(+), 1 deletion(-) - -diff --git a/server/process.c b/server/process.c -index 5be3b00..f9738c0 100644 ---- a/server/process.c -+++ b/server/process.c -@@ -394,6 +394,7 @@ void shutdown_master_socket(void) - /* final cleanup once we are sure a process is really dead */ - static void process_died( struct process *process ) - { -+ process->is_terminated = 1; - if (debug_level) fprintf( stderr, "%04x: *process killed*\n", process->id ); - if (!process->is_system) - { -@@ -450,6 +451,7 @@ struct thread *create_process( int fd, struct thread *parent_thread, int inherit - process->is_system = 0; - process->debug_children = 0; - process->is_terminating = 0; -+ process->is_terminated = 0; - process->job = NULL; - process->console = NULL; - process->startup_state = STARTUP_IN_PROGRESS; -@@ -1278,7 +1280,10 @@ DECL_HANDLER(open_process) - reply->handle = 0; - if (process) - { -- reply->handle = alloc_handle( current->process, process, req->access, req->attributes ); -+ if (!process->is_terminated) -+ reply->handle = alloc_handle( current->process, process, req->access, req->attributes ); -+ else -+ set_error( STATUS_INVALID_PARAMETER ); - release_object( process ); - } - } -diff --git a/server/process.h b/server/process.h -index 58c313a..0cf9514 100644 ---- a/server/process.h -+++ b/server/process.h -@@ -75,6 +75,7 @@ struct process - unsigned int is_system:1; /* is it a system process? */ - unsigned int debug_children:1;/* also debug all child processes */ - unsigned int is_terminating:1;/* is process terminating? */ -+ unsigned int is_terminated:1; /* is process terminated? */ - struct job *job; /* job object ascoicated with this process */ - struct list job_entry; /* list entry for job object */ - struct list locks; /* list of file locks owned by the process */ --- -2.3.3 - diff --git a/patches/server-OpenProcess/definition b/patches/server-OpenProcess/definition deleted file mode 100644 index 3adda39f..00000000 --- a/patches/server-OpenProcess/definition +++ /dev/null @@ -1,4 +0,0 @@ -Fixes: [37087] Return an error when trying to open a terminated process - -# Causes regression? -# https://bugs.wine-staging.com/show_bug.cgi?id=446 diff --git a/patches/server-Parent_Process/0001-server-Do-not-hold-reference-on-parent-process.patch b/patches/server-Parent_Process/0001-server-Do-not-hold-reference-on-parent-process.patch new file mode 100644 index 00000000..85af9b03 --- /dev/null +++ b/patches/server-Parent_Process/0001-server-Do-not-hold-reference-on-parent-process.patch @@ -0,0 +1,113 @@ +From b71aa6b6bdf71f3a5d006a5d2b1c109a29af75af Mon Sep 17 00:00:00 2001 +From: Sebastian Lackner +Date: Thu, 10 Dec 2015 05:55:41 +0100 +Subject: server: Do not hold reference on parent process. + +--- + server/console.c | 3 +-- + server/process.c | 7 +++---- + server/process.h | 2 +- + server/snapshot.c | 2 +- + server/thread.c | 2 +- + 5 files changed, 7 insertions(+), 9 deletions(-) + +diff --git a/server/console.c b/server/console.c +index a57b2fe..3d0e0e0 100644 +--- a/server/console.c ++++ b/server/console.c +@@ -1425,13 +1425,12 @@ DECL_HANDLER(alloc_console) + case 0: + /* renderer is current, console to be attached to parent process */ + renderer = current; +- if (!(process = current->process->parent)) ++ if (!(process = get_process_from_id( current->process->parent_id ))) + { + if (fd != -1) close( fd ); + set_error( STATUS_ACCESS_DENIED ); + return; + } +- grab_object( process ); + attach = 1; + break; + case 0xffffffff: +diff --git a/server/process.c b/server/process.c +index e00b429..1fdaaca 100644 +--- a/server/process.c ++++ b/server/process.c +@@ -502,7 +502,7 @@ struct thread *create_process( int fd, struct thread *parent_thread, int inherit + close( fd ); + goto error; + } +- process->parent = NULL; ++ process->parent_id = 0; + process->debugger = NULL; + process->handles = NULL; + process->msg_fd = NULL; +@@ -554,7 +554,7 @@ struct thread *create_process( int fd, struct thread *parent_thread, int inherit + else + { + struct process *parent = parent_thread->process; +- process->parent = (struct process *)grab_object( parent ); ++ process->parent_id = parent->id; + process->handles = inherit_all ? copy_handle_table( process, parent ) + : alloc_handle_table( process, 0 ); + /* Note: for security reasons, starting a new process does not attempt +@@ -621,7 +621,6 @@ static void process_destroy( struct object *obj ) + release_object( process->job ); + } + if (process->console) release_object( process->console ); +- if (process->parent) release_object( process->parent ); + if (process->msg_fd) release_object( process->msg_fd ); + list_remove( &process->entry ); + if (process->idle_event) release_object( process->idle_event ); +@@ -1350,7 +1349,7 @@ DECL_HANDLER(get_process_info) + if ((process = get_process_from_handle( req->handle, PROCESS_QUERY_LIMITED_INFORMATION ))) + { + reply->pid = get_process_id( process ); +- reply->ppid = process->parent ? get_process_id( process->parent ) : 0; ++ reply->ppid = process->parent_id; + reply->exit_code = process->exit_code; + reply->priority = process->priority; + reply->affinity = process->affinity; +diff --git a/server/process.h b/server/process.h +index fa7f60d..34b6ea6 100644 +--- a/server/process.h ++++ b/server/process.h +@@ -56,7 +56,7 @@ struct process + { + struct object obj; /* object header */ + struct list entry; /* entry in system-wide process list */ +- struct process *parent; /* parent process */ ++ process_id_t parent_id; /* parent process id (at the time of creation) */ + struct list thread_list; /* thread list */ + struct thread *debugger; /* thread debugging this process */ + struct handle_table *handles; /* handle entries */ +diff --git a/server/snapshot.c b/server/snapshot.c +index dd00bd1..bec281d 100644 +--- a/server/snapshot.c ++++ b/server/snapshot.c +@@ -113,7 +113,7 @@ static int snapshot_next_process( struct snapshot *snapshot, struct next_process + ptr = &snapshot->processes[snapshot->process_pos++]; + reply->count = ptr->count; + reply->pid = get_process_id( ptr->process ); +- reply->ppid = ptr->process->parent ? get_process_id( ptr->process->parent ) : 0; ++ reply->ppid = ptr->process->parent_id; + reply->threads = ptr->threads; + reply->priority = ptr->priority; + reply->handles = ptr->handles; +diff --git a/server/thread.c b/server/thread.c +index bad2231..176cf44 100644 +--- a/server/thread.c ++++ b/server/thread.c +@@ -1300,7 +1300,7 @@ DECL_HANDLER(init_thread) + process->peb = req->entry; + process->cpu = req->cpu; + reply->info_size = init_process( current ); +- if (!process->parent) ++ if (!process->parent_id) + process->affinity = current->affinity = get_thread_affinity( current ); + else + set_thread_affinity( current, current->affinity ); +-- +2.6.2 + diff --git a/patches/server-Parent_Process/0002-server-Increase-size-of-PID-table-to-512-to-reduce-r.patch b/patches/server-Parent_Process/0002-server-Increase-size-of-PID-table-to-512-to-reduce-r.patch new file mode 100644 index 00000000..85b901ef --- /dev/null +++ b/patches/server-Parent_Process/0002-server-Increase-size-of-PID-table-to-512-to-reduce-r.patch @@ -0,0 +1,26 @@ +From d99e740d73f63cb26e638c6daa550eb2bb28d061 Mon Sep 17 00:00:00 2001 +From: Sebastian Lackner +Date: Thu, 10 Dec 2015 05:57:34 +0100 +Subject: server: Increase size of PID table to 512 to reduce risk of + collisions. + +--- + server/process.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/server/process.c b/server/process.c +index 1fdaaca..d7ddc21 100644 +--- a/server/process.c ++++ b/server/process.c +@@ -361,7 +361,7 @@ unsigned int alloc_ptid( void *ptr ) + else /* need to grow the array */ + { + unsigned int count = alloc_ptid_entries + (alloc_ptid_entries / 2); +- if (!count) count = 64; ++ if (!count) count = 512; + if (!(entry = realloc( ptid_entries, count * sizeof(*entry) ))) + { + set_error( STATUS_NO_MEMORY ); +-- +2.6.2 + diff --git a/patches/server-Parent_Process/definition b/patches/server-Parent_Process/definition new file mode 100644 index 00000000..61699472 --- /dev/null +++ b/patches/server-Parent_Process/definition @@ -0,0 +1 @@ +Fixes: [37087] Do not hold reference on parent process in wineserver diff --git a/staging/changelog b/staging/changelog index cfae9fac..bc904eb0 100644 --- a/staging/changelog +++ b/staging/changelog @@ -1,6 +1,9 @@ wine-staging (1.8~rc4) UNRELEASED; urgency=low * Removed patch to set LastError to 0 in GetSidIdentifierAuthority (accepted upstream). + * Removed patch to return an error when trying to open a terminated process + (replaced with alternative approach). + * Added patch to avoid holding reference on parent process in wineserver. -- Sebastian Lackner Tue, 08 Dec 2015 18:32:59 +0100 wine-staging (1.8~rc3) unstable; urgency=low