[PATCH v3 1/4] kernelbase: Support PROC_THREAD_ATTRIBUTE_PARENT_PROCESS in CreateProcessInternalW().
Signed-off-by: Paul Gofman gofmanp@gmail.com --- v2: - don't use INVALID_HANDLE_VALUE to return an error on zero parent; v3: - no changes.
dlls/kernelbase/process.c | 88 ++++++++++++++++++++++++++------------- 1 file changed, 59 insertions(+), 29 deletions(-)
diff --git a/dlls/kernelbase/process.c b/dlls/kernelbase/process.c index 90ea299416..d6106eb5f6 100644 --- a/dlls/kernelbase/process.c +++ b/dlls/kernelbase/process.c @@ -244,7 +244,7 @@ static RTL_USER_PROCESS_PARAMETERS *create_process_params( const WCHAR *filename */ static NTSTATUS create_nt_process( SECURITY_ATTRIBUTES *psa, SECURITY_ATTRIBUTES *tsa, BOOL inherit, DWORD flags, RTL_USER_PROCESS_PARAMETERS *params, - RTL_USER_PROCESS_INFORMATION *info ) + RTL_USER_PROCESS_INFORMATION *info, HANDLE parent ) { NTSTATUS status; UNICODE_STRING nameW; @@ -257,7 +257,7 @@ static NTSTATUS create_nt_process( SECURITY_ATTRIBUTES *psa, SECURITY_ATTRIBUTES status = RtlCreateUserProcess( &nameW, OBJ_CASE_INSENSITIVE, params, psa ? psa->lpSecurityDescriptor : NULL, tsa ? tsa->lpSecurityDescriptor : NULL, - 0, inherit, 0, 0, info ); + parent, inherit, 0, 0, info ); RtlFreeUnicodeString( &nameW ); } return status; @@ -288,7 +288,7 @@ static NTSTATUS create_vdm_process( SECURITY_ATTRIBUTES *psa, SECURITY_ATTRIBUTE winevdm, params->ImagePathName.Buffer, params->CommandLine.Buffer ); RtlInitUnicodeString( ¶ms->ImagePathName, winevdm ); RtlInitUnicodeString( ¶ms->CommandLine, newcmdline ); - status = create_nt_process( psa, tsa, inherit, flags, params, info ); + status = create_nt_process( psa, tsa, inherit, flags, params, info, NULL ); HeapFree( GetProcessHeap(), 0, newcmdline ); return status; } @@ -316,7 +316,7 @@ static NTSTATUS create_cmd_process( SECURITY_ATTRIBUTES *psa, SECURITY_ATTRIBUTE swprintf( newcmdline, len, L"%s /s/c "%s"", comspec, params->CommandLine.Buffer ); RtlInitUnicodeString( ¶ms->ImagePathName, comspec ); RtlInitUnicodeString( ¶ms->CommandLine, newcmdline ); - status = create_nt_process( psa, tsa, inherit, flags, params, info ); + status = create_nt_process( psa, tsa, inherit, flags, params, info, NULL ); RtlFreeHeap( GetProcessHeap(), 0, newcmdline ); return status; } @@ -368,7 +368,6 @@ BOOL WINAPI DECLSPEC_HOTPATCH CreateProcessAsUserW( HANDLE token, const WCHAR *a inherit, flags, env, cur_dir, startup_info, info, NULL ); }
- /********************************************************************** * CreateProcessInternalA (kernelbase.@) */ @@ -382,7 +381,7 @@ BOOL WINAPI DECLSPEC_HOTPATCH CreateProcessInternalA( HANDLE token, const char * BOOL ret = FALSE; WCHAR *app_nameW = NULL, *cmd_lineW = NULL, *cur_dirW = NULL; UNICODE_STRING desktopW, titleW; - STARTUPINFOW infoW; + STARTUPINFOEXW infoW;
desktopW.Buffer = NULL; titleW.Buffer = NULL; @@ -393,12 +392,15 @@ BOOL WINAPI DECLSPEC_HOTPATCH CreateProcessInternalA( HANDLE token, const char * if (startup_info->lpDesktop) RtlCreateUnicodeStringFromAsciiz( &desktopW, startup_info->lpDesktop ); if (startup_info->lpTitle) RtlCreateUnicodeStringFromAsciiz( &titleW, startup_info->lpTitle );
- memcpy( &infoW, startup_info, sizeof(infoW) ); - infoW.lpDesktop = desktopW.Buffer; - infoW.lpTitle = titleW.Buffer; + memcpy( &infoW.StartupInfo, startup_info, sizeof(infoW.StartupInfo) ); + infoW.StartupInfo.lpDesktop = desktopW.Buffer; + infoW.StartupInfo.lpTitle = titleW.Buffer; + + if (flags & EXTENDED_STARTUPINFO_PRESENT) + infoW.lpAttributeList = ((STARTUPINFOEXW *)startup_info)->lpAttributeList;
ret = CreateProcessInternalW( token, app_nameW, cmd_lineW, process_attr, thread_attr, - inherit, flags, env, cur_dirW, &infoW, info, new_token ); + inherit, flags, env, cur_dirW, (STARTUPINFOW *)&infoW, info, new_token ); done: RtlFreeHeap( GetProcessHeap(), 0, app_nameW ); RtlFreeHeap( GetProcessHeap(), 0, cmd_lineW ); @@ -408,6 +410,22 @@ done: return ret; }
+struct proc_thread_attr +{ + DWORD_PTR attr; + SIZE_T size; + void *value; +}; + +struct _PROC_THREAD_ATTRIBUTE_LIST +{ + DWORD mask; /* bitmask of items in list */ + DWORD size; /* max number of items in list */ + DWORD count; /* number of items in list */ + DWORD pad; + DWORD_PTR unk; + struct proc_thread_attr attrs[1]; +};
/********************************************************************** * CreateProcessInternalW (kernelbase.@) @@ -423,6 +441,7 @@ BOOL WINAPI DECLSPEC_HOTPATCH CreateProcessInternalW( HANDLE token, const WCHAR WCHAR *p, *tidy_cmdline = cmd_line; RTL_USER_PROCESS_PARAMETERS *params = NULL; RTL_USER_PROCESS_INFORMATION rtl_info; + HANDLE parent = NULL; NTSTATUS status;
/* Process the AppName and/or CmdLine to get module name and path */ @@ -473,7 +492,36 @@ BOOL WINAPI DECLSPEC_HOTPATCH CreateProcessInternalW( HANDLE token, const WCHAR goto done; }
- status = create_nt_process( process_attr, thread_attr, inherit, flags, params, &rtl_info ); + if (flags & EXTENDED_STARTUPINFO_PRESENT) + { + struct _PROC_THREAD_ATTRIBUTE_LIST *attrs = + (struct _PROC_THREAD_ATTRIBUTE_LIST *)((STARTUPINFOEXW *)startup_info)->lpAttributeList; + unsigned int i; + + if (attrs) + { + for (i = 0; i < attrs->count; ++i) + { + switch(attrs->attrs[i].attr) + { + case PROC_THREAD_ATTRIBUTE_PARENT_PROCESS: + parent = *(HANDLE *)attrs->attrs[i].value; + TRACE("PROC_THREAD_ATTRIBUTE_PARENT_PROCESS parent %p.\n", parent); + if (!parent) + { + status = STATUS_INVALID_HANDLE; + goto done; + } + break; + default: + FIXME("Unsupported attribute %#lx.\n", attrs->attrs[i].attr); + break; + } + } + } + } + + status = create_nt_process( process_attr, thread_attr, inherit, flags, params, &rtl_info, parent ); switch (status) { case STATUS_SUCCESS: @@ -1301,24 +1349,6 @@ BOOL WINAPI DECLSPEC_HOTPATCH SetEnvironmentVariableW( LPCWSTR name, LPCWSTR val * Process/thread attribute lists ***********************************************************************/
- -struct proc_thread_attr -{ - DWORD_PTR attr; - SIZE_T size; - void *value; -}; - -struct _PROC_THREAD_ATTRIBUTE_LIST -{ - DWORD mask; /* bitmask of items in list */ - DWORD size; /* max number of items in list */ - DWORD count; /* number of items in list */ - DWORD pad; - DWORD_PTR unk; - struct proc_thread_attr attrs[1]; -}; - /*********************************************************************** * InitializeProcThreadAttributeList (kernelbase.@) */
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=47817 Signed-off-by: Paul Gofman gofmanp@gmail.com --- v2: - don't use arbitrary thread as parent; v3: - don't check for current process pseudo handle value in RtlCreateUserProcess().
dlls/kernel32/tests/process.c | 4 ++-- dlls/ntdll/process.c | 5 +++-- include/wine/server_protocol.h | 4 +++- server/console.c | 32 ++++++++++++++++---------------- server/process.c | 30 ++++++++++++++++++++++++------ server/process.h | 3 ++- server/protocol.def | 1 + server/request.h | 17 +++++++++-------- server/trace.c | 3 ++- server/user.h | 3 ++- server/winstation.c | 24 ++++++++++++++++-------- 11 files changed, 80 insertions(+), 46 deletions(-)
diff --git a/dlls/kernel32/tests/process.c b/dlls/kernel32/tests/process.c index 3efbfa2402..6d7a9a74c3 100644 --- a/dlls/kernel32/tests/process.c +++ b/dlls/kernel32/tests/process.c @@ -3874,14 +3874,14 @@ void test_parent_process_attribute(unsigned int level, HANDLE read_pipe)
memset(&parent_data, 0, sizeof(parent_data)); ret = ReadFile(read_pipe, &parent_data, sizeof(parent_data), &size, NULL); - todo_wine_if(level == 2) ok((level == 2 && ret) || (level == 1 && !ret && GetLastError() == ERROR_INVALID_HANDLE), + ok((level == 2 && ret) || (level == 1 && !ret && GetLastError() == ERROR_INVALID_HANDLE), "Got unexpected ret %#x, level %u, GetLastError() %u.\n", ret, level, GetLastError()); }
if (level == 2) { - todo_wine ok(parent_id == parent_data.parent_id, "Got parent id %u, parent_data.parent_id %u.\n", + ok(parent_id == parent_data.parent_id, "Got parent id %u, parent_data.parent_id %u.\n", parent_id, parent_data.parent_id); return; } diff --git a/dlls/ntdll/process.c b/dlls/ntdll/process.c index 52d7ea429e..a4a2f17b4f 100644 --- a/dlls/ntdll/process.c +++ b/dlls/ntdll/process.c @@ -1667,8 +1667,8 @@ NTSTATUS WINAPI RtlCreateUserProcess( UNICODE_STRING *path, ULONG attributes,
RtlNormalizeProcessParams( params );
- TRACE( "%s image %s cmdline %s\n", debugstr_us( path ), - debugstr_us( ¶ms->ImagePathName ), debugstr_us( ¶ms->CommandLine )); + TRACE("path %s, image %s, cmdline %s, parent %p.\n", debugstr_us(path), + debugstr_us(¶ms->ImagePathName), debugstr_us(¶ms->CommandLine), parent);
if ((status = get_pe_file_info( path, attributes, &file_handle, &pe_info ))) { @@ -1709,6 +1709,7 @@ NTSTATUS WINAPI RtlCreateUserProcess( UNICODE_STRING *path, ULONG attributes,
SERVER_START_REQ( new_process ) { + req->parent_process = wine_server_obj_handle(parent); req->inherit_all = inherit; req->create_flags = params->DebugFlags; /* hack: creation flags stored in DebugFlags for now */ req->socket_fd = socketfd[1]; diff --git a/include/wine/server_protocol.h b/include/wine/server_protocol.h index aaa5fd2e33..98ecd98b08 100644 --- a/include/wine/server_protocol.h +++ b/include/wine/server_protocol.h @@ -769,6 +769,7 @@ struct rawinput_device struct new_process_request { struct request_header __header; + obj_handle_t parent_process; int inherit_all; unsigned int create_flags; int socket_fd; @@ -779,6 +780,7 @@ struct new_process_request /* VARARG(objattr,object_attributes); */ /* VARARG(info,startup_info,info_size); */ /* VARARG(env,unicode_str); */ + char __pad_44[4]; }; struct new_process_reply { @@ -6702,6 +6704,6 @@ union generic_reply struct resume_process_reply resume_process_reply; };
-#define SERVER_PROTOCOL_VERSION 593 +#define SERVER_PROTOCOL_VERSION 594
#endif /* __WINE_WINE_SERVER_PROTOCOL_H */ diff --git a/server/console.c b/server/console.c index 59f8843a75..691a0bf05a 100644 --- a/server/console.c +++ b/server/console.c @@ -504,37 +504,37 @@ int free_console( struct process *process ) * 2/ parent is a renderer which launches process, and process should attach to the console * rendered by parent */ -void inherit_console(struct thread *parent_thread, struct process *process, obj_handle_t hconin) +void inherit_console(struct thread *parent_thread, struct process *parent, struct process *process, + obj_handle_t hconin) { int done = 0; - struct process* parent = parent_thread->process;
/* if parent is a renderer, then attach current process to its console * a bit hacky.... */ - if (hconin) + if (hconin && parent_thread) { - struct console_input* console; + struct console_input *console;
/* FIXME: should we check some access rights ? */ - if ((console = (struct console_input*)get_handle_obj( parent, hconin, - 0, &console_input_ops ))) - { + if ((console = (struct console_input *)get_handle_obj( parent, hconin, + 0, &console_input_ops ))) + { if (console->renderer == parent_thread) - { - process->console = (struct console_input*)grab_object( console ); - process->console->num_proc++; - done = 1; - } - release_object( console ); - } + { + process->console = (struct console_input*)grab_object( console ); + process->console->num_proc++; + done = 1; + } + release_object( console ); + } else clear_error(); /* ignore error */ } /* otherwise, if parent has a console, attach child to this console */ if (!done && parent->console) { - process->console = (struct console_input*)grab_object( parent->console ); - process->console->num_proc++; + process->console = (struct console_input*)grab_object( parent->console ); + process->console->num_proc++; } }
diff --git a/server/process.c b/server/process.c index 16bb5d57e7..00ea45b068 100644 --- a/server/process.c +++ b/server/process.c @@ -1117,6 +1117,7 @@ DECL_HANDLER(new_process) const struct object_attributes *objattr = get_req_object_attributes( &sd, &name, NULL ); struct process *process = NULL; struct process *parent = current->process; + struct thread *parent_thread = current; int socket_fd = thread_get_inflight_fd( current, req->socket_fd );
if (socket_fd == -1) @@ -1148,11 +1149,26 @@ DECL_HANDLER(new_process) return; }
+ if (req->parent_process) + { + if (!(parent = get_process_from_handle( req->parent_process, PROCESS_CREATE_PROCESS))) + { + set_error(STATUS_INVALID_HANDLE); + close(socket_fd); + return; + } + parent_thread = NULL; + } + if (parent->job && (req->create_flags & CREATE_BREAKAWAY_FROM_JOB) && !(parent->job->limit_flags & (JOB_OBJECT_LIMIT_BREAKAWAY_OK | JOB_OBJECT_LIMIT_SILENT_BREAKAWAY_OK))) { set_error( STATUS_ACCESS_DENIED ); close( socket_fd ); + + if (req->parent_process) + release_object(parent); + return; }
@@ -1222,7 +1238,7 @@ DECL_HANDLER(new_process) }
/* connect to the window station */ - connect_process_winstation( process, current ); + connect_process_winstation( process, parent_thread, parent );
/* set the process console */ if (!(req->create_flags & (DETACHED_PROCESS | CREATE_NEW_CONSOLE))) @@ -1231,7 +1247,7 @@ DECL_HANDLER(new_process) * like if hConOut and hConIn are console handles, then they should be on the same * physical console */ - inherit_console( current, process, req->inherit_all ? info->data->hstdin : 0 ); + inherit_console( parent_thread, parent, process, req->inherit_all ? info->data->hstdin : 0 ); }
if (!req->inherit_all && !(req->create_flags & CREATE_NEW_CONSOLE)) @@ -1246,16 +1262,15 @@ DECL_HANDLER(new_process) if (get_error() == STATUS_INVALID_HANDLE || get_error() == STATUS_OBJECT_TYPE_MISMATCH) clear_error(); } - /* attach to the debugger if requested */ if (req->create_flags & (DEBUG_PROCESS | DEBUG_ONLY_THIS_PROCESS)) { set_process_debugger( process, current ); process->debug_children = !(req->create_flags & DEBUG_ONLY_THIS_PROCESS); } - else if (parent->debugger && parent->debug_children) + else if (current->process->debugger && current->process->debug_children) { - set_process_debugger( process, parent->debugger ); + set_process_debugger( process, current->process->debugger ); /* debug_children is set to 1 by default */ }
@@ -1265,9 +1280,12 @@ DECL_HANDLER(new_process) info->process = (struct process *)grab_object( process ); reply->info = alloc_handle( current->process, info, SYNCHRONIZE, 0 ); reply->pid = get_process_id( process ); - reply->handle = alloc_handle_no_access_check( parent, process, req->access, objattr->attributes ); + reply->handle = alloc_handle_no_access_check( current->process, process, req->access, objattr->attributes );
done: + if (req->parent_process) + release_object(parent); + if (process) release_object( process ); release_object( info ); } diff --git a/server/process.h b/server/process.h index 20ff6beda6..d8453eeaf2 100644 --- a/server/process.h +++ b/server/process.h @@ -141,7 +141,8 @@ extern struct process_snapshot *process_snap( int *count ); extern void enum_processes( int (*cb)(struct process*, void*), void *user);
/* console functions */ -extern void inherit_console(struct thread *parent_thread, struct process *process, obj_handle_t hconin); +extern void inherit_console(struct thread *parent_thread, struct process *parent, + struct process *process, obj_handle_t hconin); extern int free_console( struct process *process ); extern struct thread *console_get_renderer( struct console_input *console );
diff --git a/server/protocol.def b/server/protocol.def index 1cb1fea602..7f9ec3a149 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -783,6 +783,7 @@ struct rawinput_device
/* Create a new process from the context of the parent */ @REQ(new_process) + obj_handle_t parent_process; /* parent process */ int inherit_all; /* inherit all handles from parent */ unsigned int create_flags; /* creation flags */ int socket_fd; /* file descriptor for process socket */ diff --git a/server/request.h b/server/request.h index 90a3180a6c..9f36bcb711 100644 --- a/server/request.h +++ b/server/request.h @@ -745,14 +745,15 @@ C_ASSERT( sizeof(unsigned char) == 1 ); C_ASSERT( sizeof(unsigned int) == 4 ); C_ASSERT( sizeof(unsigned short) == 2 ); C_ASSERT( sizeof(user_handle_t) == 4 ); -C_ASSERT( FIELD_OFFSET(struct new_process_request, inherit_all) == 12 ); -C_ASSERT( FIELD_OFFSET(struct new_process_request, create_flags) == 16 ); -C_ASSERT( FIELD_OFFSET(struct new_process_request, socket_fd) == 20 ); -C_ASSERT( FIELD_OFFSET(struct new_process_request, exe_file) == 24 ); -C_ASSERT( FIELD_OFFSET(struct new_process_request, access) == 28 ); -C_ASSERT( FIELD_OFFSET(struct new_process_request, cpu) == 32 ); -C_ASSERT( FIELD_OFFSET(struct new_process_request, info_size) == 36 ); -C_ASSERT( sizeof(struct new_process_request) == 40 ); +C_ASSERT( FIELD_OFFSET(struct new_process_request, parent_process) == 12 ); +C_ASSERT( FIELD_OFFSET(struct new_process_request, inherit_all) == 16 ); +C_ASSERT( FIELD_OFFSET(struct new_process_request, create_flags) == 20 ); +C_ASSERT( FIELD_OFFSET(struct new_process_request, socket_fd) == 24 ); +C_ASSERT( FIELD_OFFSET(struct new_process_request, exe_file) == 28 ); +C_ASSERT( FIELD_OFFSET(struct new_process_request, access) == 32 ); +C_ASSERT( FIELD_OFFSET(struct new_process_request, cpu) == 36 ); +C_ASSERT( FIELD_OFFSET(struct new_process_request, info_size) == 40 ); +C_ASSERT( sizeof(struct new_process_request) == 48 ); C_ASSERT( FIELD_OFFSET(struct new_process_reply, info) == 8 ); C_ASSERT( FIELD_OFFSET(struct new_process_reply, pid) == 12 ); C_ASSERT( FIELD_OFFSET(struct new_process_reply, handle) == 16 ); diff --git a/server/trace.c b/server/trace.c index 5b1d3ddea9..47f66b582c 100644 --- a/server/trace.c +++ b/server/trace.c @@ -1243,7 +1243,8 @@ typedef void (*dump_func)( const void *req );
static void dump_new_process_request( const struct new_process_request *req ) { - fprintf( stderr, " inherit_all=%d", req->inherit_all ); + fprintf( stderr, " parent_process=%04x", req->parent_process ); + fprintf( stderr, ", inherit_all=%d", req->inherit_all ); fprintf( stderr, ", create_flags=%08x", req->create_flags ); fprintf( stderr, ", socket_fd=%d", req->socket_fd ); fprintf( stderr, ", exe_file=%04x", req->exe_file ); diff --git a/server/user.h b/server/user.h index eb1b7ce1e4..35184d8780 100644 --- a/server/user.h +++ b/server/user.h @@ -183,7 +183,8 @@ extern client_ptr_t get_class_client_ptr( struct window_class *class ); extern struct desktop *get_desktop_obj( struct process *process, obj_handle_t handle, unsigned int access ); extern struct winstation *get_process_winstation( struct process *process, unsigned int access ); extern struct desktop *get_thread_desktop( struct thread *thread, unsigned int access ); -extern void connect_process_winstation( struct process *process, struct thread *parent ); +extern void connect_process_winstation( struct process *process, struct thread *parent_thread, + struct process *parent_process ); extern void set_process_default_desktop( struct process *process, struct desktop *desktop, obj_handle_t handle ); extern void close_process_desktop( struct process *process ); diff --git a/server/winstation.c b/server/winstation.c index a09ca03e3b..f7932a9dc6 100644 --- a/server/winstation.c +++ b/server/winstation.c @@ -372,7 +372,8 @@ void set_process_default_desktop( struct process *process, struct desktop *deskt }
/* connect a process to its window station */ -void connect_process_winstation( struct process *process, struct thread *parent ) +void connect_process_winstation( struct process *process, struct thread *parent_thread, + struct process *parent_process) { struct winstation *winstation = NULL; struct desktop *desktop = NULL; @@ -383,9 +384,9 @@ void connect_process_winstation( struct process *process, struct thread *parent { winstation = (struct winstation *)get_handle_obj( process, handle, 0, &winstation_ops ); } - else if (parent && parent->process->winstation) + else if (parent_process->winstation) { - handle = duplicate_handle( parent->process, parent->process->winstation, + handle = duplicate_handle( parent_process, parent_process->winstation, process, 0, 0, DUP_HANDLE_SAME_ACCESS ); winstation = (struct winstation *)get_handle_obj( process, handle, 0, &winstation_ops ); } @@ -397,14 +398,21 @@ void connect_process_winstation( struct process *process, struct thread *parent desktop = get_desktop_obj( process, handle, 0 ); if (!desktop || desktop->winstation != winstation) goto done; } - else if (parent && parent->desktop) + else { - desktop = get_desktop_obj( parent->process, parent->desktop, 0 ); + if (parent_thread && parent_thread->desktop) + handle = parent_thread->desktop; + else if (parent_process->desktop) + handle = parent_process->desktop; + else + goto done; + + desktop = get_desktop_obj( parent_process, handle, 0 ); + if (!desktop || desktop->winstation != winstation) goto done; - handle = duplicate_handle( parent->process, parent->desktop, - process, 0, 0, DUP_HANDLE_SAME_ACCESS ); - }
+ handle = duplicate_handle( parent_process, handle, process, 0, 0, DUP_HANDLE_SAME_ACCESS ); + } if (handle) set_process_default_desktop( process, desktop, handle );
done:
Signed-off-by: Paul Gofman gofmanp@gmail.com --- v2: - no changes; v3: - mark failed child process creation for GetCurrentProcess() parent handle as broken instead of successful creation; - replace test with INVALID_HANDLE_VALUE parent (which is the same as GetCurrentProcess()) with OpenProcess(..., GetCurrentProcessId()) handle.
dlls/kernel32/tests/process.c | 85 ++++++++++++++++++++++++++++++++++- 1 file changed, 84 insertions(+), 1 deletion(-)
diff --git a/dlls/kernel32/tests/process.c b/dlls/kernel32/tests/process.c index 6d7a9a74c3..1194c54202 100644 --- a/dlls/kernel32/tests/process.c +++ b/dlls/kernel32/tests/process.c @@ -3825,7 +3825,8 @@ static void test_ProcThreadAttributeList(void) /* level 0: Main test process * level 1: Process created by level 0 process without handle inheritance * level 2: Process created by level 1 process with handle inheritance and level 0 - * process parent substitute. */ + * process parent substitute. + * level 255: Process created by level 1 process during invalid parent handles testing. */ void test_parent_process_attribute(unsigned int level, HANDLE read_pipe) { PROCESS_BASIC_INFORMATION pbi; @@ -3848,6 +3849,9 @@ void test_parent_process_attribute(unsigned int level, HANDLE read_pipe) } parent_data;
+ if (level == 255) + return; + if (!pInitializeProcThreadAttributeList) { win_skip("No support for ProcThreadAttributeList.\n"); @@ -3891,11 +3895,90 @@ void test_parent_process_attribute(unsigned int level, HANDLE read_pipe)
if (level) { + HANDLE handle; SIZE_T size;
ret = pInitializeProcThreadAttributeList(NULL, 1, 0, &size); ok(!ret && GetLastError() == ERROR_INSUFFICIENT_BUFFER, "Got unexpected ret %#x, GetLastError() %u.\n", ret, GetLastError()); + + sprintf(buffer, ""%s" tests/process.c parent %u %p", selfname, 255, read_pipe); + +#if 0 + /* Crashes on some Windows installations, otherwise successfully creates process. */ + ret = CreateProcessA(NULL, buffer, NULL, NULL, FALSE, EXTENDED_STARTUPINFO_PRESENT, + NULL, NULL, (STARTUPINFOA *)&si, &info); + ok(ret, "Got unexpected ret %#x, GetLastError() %u.\n", ret, GetLastError()); + ok(WaitForSingleObject(info.hProcess, 30000) == WAIT_OBJECT_0, "Child process termination\n"); + CloseHandle(info.hThread); + CloseHandle(info.hProcess); +#endif + si.lpAttributeList = heap_alloc(size); + ret = pInitializeProcThreadAttributeList(si.lpAttributeList, 1, 0, &size); + ok(ret, "Got unexpected ret %#x, GetLastError() %u.\n", ret, GetLastError()); + handle = OpenProcess(PROCESS_CREATE_PROCESS, TRUE, GetCurrentProcessId()); + ret = pUpdateProcThreadAttribute(si.lpAttributeList, 0, PROC_THREAD_ATTRIBUTE_PARENT_PROCESS, + &handle, sizeof(handle), NULL, NULL); + ok(ret, "Got unexpected ret %#x, GetLastError() %u.\n", ret, GetLastError()); + ret = CreateProcessA(NULL, buffer, NULL, NULL, TRUE, EXTENDED_STARTUPINFO_PRESENT, + NULL, NULL, (STARTUPINFOA *)&si, &info); + ok(ret, "Got unexpected ret %#x, GetLastError() %u.\n", ret, GetLastError()); + ok(WaitForSingleObject(info.hProcess, 30000) == WAIT_OBJECT_0, "Child process termination\n"); + CloseHandle(info.hThread); + CloseHandle(info.hProcess); + CloseHandle(handle); + pDeleteProcThreadAttributeList(si.lpAttributeList); + heap_free(si.lpAttributeList); + + si.lpAttributeList = heap_alloc(size); + ret = pInitializeProcThreadAttributeList(si.lpAttributeList, 1, 0, &size); + ok(ret, "Got unexpected ret %#x, GetLastError() %u.\n", ret, GetLastError()); + handle = (HANDLE)0xdeadbeef; + ret = pUpdateProcThreadAttribute(si.lpAttributeList, 0, PROC_THREAD_ATTRIBUTE_PARENT_PROCESS, + &handle, sizeof(handle), NULL, NULL); + ok(ret, "Got unexpected ret %#x, GetLastError() %u.\n", ret, GetLastError()); + ret = CreateProcessA(NULL, buffer, NULL, NULL, TRUE, EXTENDED_STARTUPINFO_PRESENT, + NULL, NULL, (STARTUPINFOA *)&si, &info); + ok(!ret && GetLastError() == ERROR_INVALID_HANDLE, "Got unexpected ret %#x, GetLastError() %u.\n", + ret, GetLastError()); + pDeleteProcThreadAttributeList(si.lpAttributeList); + heap_free(si.lpAttributeList); + + si.lpAttributeList = heap_alloc(size); + ret = pInitializeProcThreadAttributeList(si.lpAttributeList, 1, 0, &size); + ok(ret, "Got unexpected ret %#x, GetLastError() %u.\n", ret, GetLastError()); + handle = NULL; + ret = pUpdateProcThreadAttribute(si.lpAttributeList, 0, PROC_THREAD_ATTRIBUTE_PARENT_PROCESS, + &handle, sizeof(handle), NULL, NULL); + ok(ret, "Got unexpected ret %#x, GetLastError() %u.\n", ret, GetLastError()); + ret = CreateProcessA(NULL, buffer, NULL, NULL, TRUE, EXTENDED_STARTUPINFO_PRESENT, + NULL, NULL, (STARTUPINFOA *)&si, &info); + ok(!ret && GetLastError() == ERROR_INVALID_HANDLE, "Got unexpected ret %#x, GetLastError() %u.\n", + ret, GetLastError()); + pDeleteProcThreadAttributeList(si.lpAttributeList); + heap_free(si.lpAttributeList); + + si.lpAttributeList = heap_alloc(size); + ret = pInitializeProcThreadAttributeList(si.lpAttributeList, 1, 0, &size); + ok(ret, "Got unexpected ret %#x, GetLastError() %u.\n", ret, GetLastError()); + handle = GetCurrentProcess(); + ret = pUpdateProcThreadAttribute(si.lpAttributeList, 0, PROC_THREAD_ATTRIBUTE_PARENT_PROCESS, + &handle, sizeof(handle), NULL, NULL); + ok(ret, "Got unexpected ret %#x, GetLastError() %u.\n", ret, GetLastError()); + ret = CreateProcessA(NULL, buffer, NULL, NULL, TRUE, EXTENDED_STARTUPINFO_PRESENT, + NULL, NULL, (STARTUPINFOA *)&si, &info); + /* Broken on Vista / w7 / w10. */ + ok(ret || broken(!ret && GetLastError() == ERROR_INVALID_HANDLE), + "Got unexpected ret %#x, GetLastError() %u.\n", ret, GetLastError()); + if (ret) + { + ok(WaitForSingleObject(info.hProcess, 30000) == WAIT_OBJECT_0, "Child process termination\n"); + CloseHandle(info.hThread); + CloseHandle(info.hProcess); + } + pDeleteProcThreadAttributeList(si.lpAttributeList); + heap_free(si.lpAttributeList); + si.lpAttributeList = heap_alloc(size); ret = pInitializeProcThreadAttributeList(si.lpAttributeList, 1, 0, &size); ok(ret, "Got unexpected ret %#x, GetLastError() %u.\n", ret, GetLastError());
Hi,
While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=62019
Your paranoid android.
=== wvistau64 (64 bit report) ===
kernel32: process.c:1226: Test failed: expected 1, got 0 process.c:1229: Test failed: Child process termination process.c:1233: Test failed: Toolhelp:cntUsage expected 0, but got 1 process.c:1234: Test failed: Toolhelp:th32DefaultHeapID expected 0, but got 1 process.c:1235: Test failed: Toolhelp:th32ModuleID expected 0, but got 1 process.c:1236: Test failed: Toolhelp:th32ParentProcessID expected 1428, but got 0 process.c:1238: Test failed: Toolhelp:dwFlags expected 0, but got 1
Signed-off-by: Paul Gofman gofmanp@gmail.com --- v2: - added test; v3: - no changes.
dlls/kernel32/tests/process.c | 55 ++++++++++++++++++++++++++++------- 1 file changed, 45 insertions(+), 10 deletions(-)
diff --git a/dlls/kernel32/tests/process.c b/dlls/kernel32/tests/process.c index 1194c54202..92838c09ff 100644 --- a/dlls/kernel32/tests/process.c +++ b/dlls/kernel32/tests/process.c @@ -92,6 +92,7 @@ static SIZE_T (WINAPI *pGetLargePageMinimum)(void); static BOOL (WINAPI *pInitializeProcThreadAttributeList)(struct _PROC_THREAD_ATTRIBUTE_LIST*, DWORD, DWORD, SIZE_T*); static BOOL (WINAPI *pUpdateProcThreadAttribute)(struct _PROC_THREAD_ATTRIBUTE_LIST*, DWORD, DWORD_PTR, void *,SIZE_T,void*,SIZE_T*); static void (WINAPI *pDeleteProcThreadAttributeList)(struct _PROC_THREAD_ATTRIBUTE_LIST*); +static DWORD (WINAPI *pGetFinalPathNameByHandleA)(HANDLE, LPSTR, DWORD, DWORD);
/* ############################### */ static char base[MAX_PATH]; @@ -259,6 +260,7 @@ static BOOL init(void) pInitializeProcThreadAttributeList = (void *)GetProcAddress(hkernel32, "InitializeProcThreadAttributeList"); pUpdateProcThreadAttribute = (void *)GetProcAddress(hkernel32, "UpdateProcThreadAttribute"); pDeleteProcThreadAttributeList = (void *)GetProcAddress(hkernel32, "DeleteProcThreadAttributeList"); + pGetFinalPathNameByHandleA = (void *)GetProcAddress(hkernel32, "GetFinalPathNameByHandleA");
return TRUE; } @@ -3827,7 +3829,7 @@ static void test_ProcThreadAttributeList(void) * level 2: Process created by level 1 process with handle inheritance and level 0 * process parent substitute. * level 255: Process created by level 1 process during invalid parent handles testing. */ -void test_parent_process_attribute(unsigned int level, HANDLE read_pipe) +void test_parent_process_attribute(unsigned int level, HANDLE read_pipe, HANDLE creator_stdhandle) { PROCESS_BASIC_INFORMATION pbi; char buffer[MAX_PATH + 64]; @@ -3837,7 +3839,9 @@ void test_parent_process_attribute(unsigned int level, HANDLE read_pipe) STARTUPINFOEXA si; DWORD parent_id; NTSTATUS status; + DWORD exit_code; ULONG pbi_size; + HANDLE hstderr; HANDLE parent; DWORD size; BOOL ret; @@ -3885,8 +3889,24 @@ void test_parent_process_attribute(unsigned int level, HANDLE read_pipe)
if (level == 2) { + char file_path[MAX_PATH]; + ok(parent_id == parent_data.parent_id, "Got parent id %u, parent_data.parent_id %u.\n", parent_id, parent_data.parent_id); + + GetStartupInfoA(&si.StartupInfo); + hstderr = si.StartupInfo.hStdError; + + /* On Windows, std handle values are copied from creator process but seems not to be inherited from it. + * Some operation on such handle may sometimes succeed, but various way of quering information + * from such handle suggest that the handle refers to some other object. + * Windows seem to just keep the handle value, even for invalid handle. It does not always work exacly + * like that in Wine now due to special handling of console handles in kernelbase/process.c:create_process_params() + * and initialization in dlls/msvcrt: msvcrt_init_io(). */ + ok(hstderr == creator_stdhandle, "Unexpected hstderr %p, creator_hstdhandle %p.\n", + hstderr, creator_stdhandle); + size = pGetFinalPathNameByHandleA(hstderr, file_path, sizeof(file_path), FILE_NAME_NORMALIZED); + ok(!size, "Got unexpected size %u.\n", size); return; }
@@ -3902,7 +3922,7 @@ void test_parent_process_attribute(unsigned int level, HANDLE read_pipe) ok(!ret && GetLastError() == ERROR_INSUFFICIENT_BUFFER, "Got unexpected ret %#x, GetLastError() %u.\n", ret, GetLastError());
- sprintf(buffer, ""%s" tests/process.c parent %u %p", selfname, 255, read_pipe); + sprintf(buffer, ""%s" tests/process.c parent %u %p %p", selfname, 255, read_pipe, NULL);
#if 0 /* Crashes on some Windows installations, otherwise successfully creates process. */ @@ -3988,9 +4008,17 @@ void test_parent_process_attribute(unsigned int level, HANDLE read_pipe) ret = pUpdateProcThreadAttribute(si.lpAttributeList, 0, PROC_THREAD_ATTRIBUTE_PARENT_PROCESS, &parent, sizeof(parent), NULL, NULL); ok(ret, "Got unexpected ret %#x, GetLastError() %u.\n", ret, GetLastError()); + + hstderr = CreateFileA("stderr_1.tmp", GENERIC_WRITE, FILE_SHARE_READ | FILE_SHARE_WRITE, &sa, + CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL | FILE_FLAG_DELETE_ON_CLOSE, NULL); + ok(hstderr != INVALID_HANDLE_VALUE, "Could not create file, GetLastError() %u.\n", GetLastError()); + si.StartupInfo.dwFlags = STARTF_USESTDHANDLES; + si.StartupInfo.hStdError = hstderr; + si.StartupInfo.hStdOutput = creator_stdhandle; }
- sprintf(buffer, ""%s" tests/process.c parent %u %p", selfname, level + 1, read_pipe); + sprintf(buffer, ""%s" tests/process.c parent %u %p %p", selfname, level + 1, read_pipe, + level ? hstderr : GetStdHandle(STD_OUTPUT_HANDLE)); ret = CreateProcessA(NULL, buffer, NULL, NULL, level == 1, level == 1 ? EXTENDED_STARTUPINFO_PRESENT : 0, NULL, NULL, (STARTUPINFOA *)&si, &info); ok(ret, "Got unexpected ret %#x, GetLastError() %u.\n", ret, GetLastError()); @@ -4008,10 +4036,16 @@ void test_parent_process_attribute(unsigned int level, HANDLE read_pipe)
/* wait for child to terminate */ ok(WaitForSingleObject(info.hProcess, 30000) == WAIT_OBJECT_0, "Child process termination\n"); + GetExitCodeProcess(info.hProcess, &exit_code); + ok(!exit_code, "Child test failed, exit_code %#x.\n", exit_code); + CloseHandle(info.hThread); CloseHandle(info.hProcess); - - if (!level) + if (level) + { + CloseHandle(hstderr); + } + else { CloseHandle(read_pipe); CloseHandle(write_pipe); @@ -4062,12 +4096,13 @@ START_TEST(process) CloseHandle(info.hThread); return; } - else if (!strcmp(myARGV[2], "parent") && myARGC >= 5) + else if (!strcmp(myARGV[2], "parent") && myARGC >= 6) { - HANDLE h; + HANDLE h1, h2;
- sscanf(myARGV[4], "%p", &h); - test_parent_process_attribute(atoi(myARGV[3]), h); + sscanf(myARGV[4], "%p", &h1); + sscanf(myARGV[5], "%p", &h2); + test_parent_process_attribute(atoi(myARGV[3]), h1, h2); return; }
@@ -4136,5 +4171,5 @@ START_TEST(process) test_jobInheritance(job); test_BreakawayOk(job); CloseHandle(job); - test_parent_process_attribute(0, NULL); + test_parent_process_attribute(0, NULL, NULL); }
Hi,
While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=62020
Your paranoid android.
=== w1064v1507 (32 bit report) ===
kernel32: 0e38:process:proce 1 sts.c:2925: Test failed: WaitFoerSinglesObject returnted 258
Report errors: kernel32:process is missing some failure messages
=== wvistau64 (64 bit report) ===
kernel32: process.c:1228: Test failed: expected 1, got 0 process.c:1231: Test failed: Child process termination process.c:1235: Test failed: Toolhelp:cntUsage expected 0, but got 1 process.c:1236: Test failed: Toolhelp:th32DefaultHeapID expected 0, but got 1 process.c:1237: Test failed: Toolhelp:th32ModuleID expected 0, but got 1 process.c:1238: Test failed: Toolhelp:th32ParentProcessID expected 1428, but got 0 process.c:1240: Test failed: Toolhelp:dwFlags expected 0, but got 1
Paul Gofman gofmanp@gmail.com writes:
Signed-off-by: Paul Gofman gofmanp@gmail.com
v2: - added test; v3: - no changes.dlls/kernel32/tests/process.c | 55 ++++++++++++++++++++++++++++------- 1 file changed, 45 insertions(+), 10 deletions(-)
This fails here:
../../../tools/runtest -q -P wine -T ../../.. -M kernel32.dll -p kernel32_test.exe process && touch process.ok process.c:3893: Test failed: Unexpected hstderr 0000005F, creator_hstdhandle 0000005C. process.c:4027: Test failed: Child test failed, exit_code 0x1. process.c:4027: Test failed: Child test failed, exit_code 0x1. make: *** [Makefile:720: process.ok] Error 1
On 12/13/19 14:21, Alexandre Julliard wrote:
Paul Gofman gofmanp@gmail.com writes:
Signed-off-by: Paul Gofman gofmanp@gmail.com
v2: - added test; v3: - no changes.dlls/kernel32/tests/process.c | 55 ++++++++++++++++++++++++++++------- 1 file changed, 45 insertions(+), 10 deletions(-)
This fails here:
../../../tools/runtest -q -P wine -T ../../.. -M kernel32.dll -p kernel32_test.exe process && touch process.ok process.c:3893: Test failed: Unexpected hstderr 0000005F, creator_hstdhandle 0000005C. process.c:4027: Test failed: Child test failed, exit_code 0x1. process.c:4027: Test failed: Child test failed, exit_code 0x1. make: *** [Makefile:720: process.ok] Error 1
I guess the handle gets different value in ntdll/env.c:init_user_process_params() in this case:
...
if (!isatty(2)) wine_server_fd_to_handle( 2, GENERIC_WRITE|SYNCHRONIZE, OBJ_INHERIT, ¶ms->hStdError ); ...
On Windows I just get always the same value for handle there (or if I get the handle by GetStdHandle()), and if it is a handle of creator process (not the specified parent) it is either invalid or accidentally equals to some other object's handle inherited from the parent process. It looks like std handles management differes in Wine in a few places regardless of the parent substitution. I thought the test is stable in this form but it is apparently isn't.
Maybe we can just drop this last patch in the series for now?
Paul Gofman gofmanp@gmail.com writes:
On Windows I just get always the same value for handle there (or if I get the handle by GetStdHandle()), and if it is a handle of creator process (not the specified parent) it is either invalid or accidentally equals to some other object's handle inherited from the parent process. It looks like std handles management differes in Wine in a few places regardless of the parent substitution. I thought the test is stable in this form but it is apparently isn't.
Maybe we can just drop this last patch in the series for now?
Sure.
-
Alexandre Julliard -
Marvin -
Paul Gofman