CreateRemoteThread and related stuff (patch)
Robert Shearman
rob at codeweavers.com
Mon Aug 23 08:58:28 CDT 2004
Alexander Yaworsky wrote:
>Hello
>
>This is a basic framework. Not finished, not really tested -- just stable point. It does no harm for applications I'm using. I would
>be glad to get some comments and suggestions.
>
>
Looks good so far, apart from a few comments below.
>@@ -168,16 +137,68 @@
> * Success: Handle to the new thread.
> * Failure: NULL. Use GetLastError() to find the error cause.
> *
>- * BUGS
>- * Unimplemented
> */
> HANDLE WINAPI CreateRemoteThread( HANDLE hProcess, SECURITY_ATTRIBUTES *sa, SIZE_T stack,
> LPTHREAD_START_ROUTINE start, LPVOID param,
> DWORD flags, LPDWORD id )
> {
>- FIXME("(): stub, Write Me.\n");
>+ extern BOOL __wine_is_current_process( HANDLE hProcess );
>+
>+ HANDLE handle;
>+ CLIENT_ID client_id;
>+ NTSTATUS status;
>+ SIZE_T stack_reserve = 0, stack_commit = 0;
>+ struct new_thread_info *info;
>+ PRTL_THREAD_START_ROUTINE start_addr;
>+
>+ if (!(info = VirtualAllocEx( hProcess, NULL, sizeof(*info),
>+ MEM_COMMIT, PAGE_READWRITE )))
>+ return 0;
>+
>+ if( __wine_is_current_process( hProcess ) )
>
>
There is no need to export __wine_is_current_process from ntdll and use
it here when standard win32 apis will suffice.
>+ {
>+ info->func = start;
>+ info->arg = param;
>+ start_addr = (void*) THREAD_Start;
>+ }
>+ else
>+ {
>+ struct new_thread_info local_info;
>+ DWORD written;
>+
>+ local_info.func = start;
>+ local_info.arg = param;
>+ if( ! WriteProcessMemory( hProcess, &info, &local_info,
>
This looks wrong. &info? Don't you mean just info?
> /***********************************************************************
>+ * remote_op
>+ *
>+ */
>+NTSTATUS remote_op( HANDLE process, int type,
>+ void* params, int params_size, void* result, int* result_size )
>+{
>+ HANDLE event;
>+ NTSTATUS status, remote_status;
>+
>+ /* send params */
>+ SERVER_START_REQ( remote_operation )
>+ {
>+ req->handle = process;
>+ req->type = type;
>+ if (params) wine_server_add_data( req, params, params_size );
>+ if (!(status = wine_server_call( req )))
>+ event = reply->event;
>+ }
>+ SERVER_END_REQ;
>+ if (status)
>+ return status;
>+
>+ /* wait for completion */
>+ status = NtWaitForMultipleObjects( 1, &event, FALSE, FALSE, NULL );
>+ NtClose( event );
>+ if (HIWORD(status))
>+ return status;
>+
>+ /* get result */
>+ remote_status = 0; /* make compiler happy */
>+ SERVER_START_REQ( remote_operation_result )
>+ {
>+ wine_server_set_reply( req, result, *result_size );
>+ if (!(status = wine_server_call( req )))
>+ {
>+ remote_status = reply->status;
>+ if (result)
>+ *result_size = wine_server_reply_size( reply );
>+ }
>+ }
>+ SERVER_END_REQ;
>+
>+ return status? status : remote_status;
>+}
>
>
I don't really like the idea of a multiplexer like this, but I guess
otherwise we would end up with duplicated code.
>+
>+/* return address of internal thread start function in kernel32 */
>+DECL_HANDLER(get_kernel_thread_start)
>+{
>+ struct process *process;
>+
>+ /* because this is used for CreateRemoteThread,
>+ required access rights must be the same
>+ */
>+ if ((process = get_process_from_handle( req->handle,
>+ PROCESS_CREATE_THREAD
>+ | PROCESS_QUERY_INFORMATION
>+ | PROCESS_VM_OPERATION
>+ | PROCESS_VM_READ | PROCESS_VM_WRITE )))
>
>
I would argue that it is perfectly alright for the access rights to be
just PROCESS_QUERY_INFORMATION here.
>+ {
>+ reply->address = process->kernel_thread_start;
>+ release_object( process );
>+ }
>+}
>+
>+/* Accept parameters for remote operation and start it */
>+DECL_HANDLER(remote_operation)
>+{
>+ int access;
>+ struct process *process;
>+ struct event *event;
>+
>+ /* define required access rights */
>+ switch( req->type )
>+ {
>+ case REMOTE_OP_NEW_THREAD:
>+ access = PROCESS_CREATE_THREAD | PROCESS_QUERY_INFORMATION
>+ | PROCESS_VM_OPERATION | PROCESS_VM_READ | PROCESS_VM_WRITE;
>+ break;
>+ case REMOTE_OP_VM_ALLOC: access = PROCESS_VM_OPERATION; break;
>+ case REMOTE_OP_VM_FREE: access = PROCESS_VM_OPERATION; break;
>+ case REMOTE_OP_VM_PROTECT: access = PROCESS_VM_OPERATION; break;
>+ case REMOTE_OP_VM_QUERY: access = PROCESS_QUERY_INFORMATION; break;
>+ case REMOTE_OP_VM_MAP: access = PROCESS_VM_OPERATION; break;
>+ case REMOTE_OP_VM_UNMAP: access = PROCESS_VM_OPERATION; break;
>+ case REMOTE_OP_VM_FLUSH: access = PROCESS_VM_OPERATION; break; /* FIXME: is access right? */
>+ default:
>+ set_error( STATUS_INVALID_PARAMETER );
>+ return;
>+ }
>+
>+ /* get process object */
>+ if (!(process = get_process_from_handle( req->handle, access ))) return;
>+
>+ /* dispose result data buffer if allocated */
>+ if (current->result_data)
>+ {
>+ free( current->result_data );
>+ current->result_data = NULL;
>+ }
>+
>+ /* create event object */
>+ reply->event = NULL;
>+ if (current->result_event)
>+ {
>+ release_object( current->result_event );
>+ current->result_event = NULL;
>+ }
>+ if (!(event = create_event( NULL, 0, 1, 0 )))
>+ goto error;
>+
>+ if (!(reply->event = alloc_handle( current->process, event, EVENT_ALL_ACCESS, FALSE )))
>+ goto error;
>+
>+ /* FIXME: pass somehow operation type, params and originator thread id
>+ for some thread in the process and force execution of operation */
>+ set_error( STATUS_NOT_IMPLEMENTED );
>+
>+ /* save event object in thread structure for future set operation;
>+ we do not release it here */
>+ current->result_event = event;
>+ release_object( process );
>+ return;
>+
>+error:
>+ if (reply->event) close_handle( current->process, reply->event, NULL );
>+ if (event) release_object( event );
>+ release_object( process );
>+}
>
>
Is it really necessary to perform the action required for remote
invocation of some operation asynchronously? It seems to add a lot of
unneeded complexity.
>+/* save result of remote operation and wakeup originator */
>+DECL_HANDLER(remote_operation_complete)
>+{
>+ struct thread *thread = get_thread_from_id( req->originator );
>+
>+ if (!thread) return;
>+
>+ /* save status */
>+ thread->remote_status = req->status;
>+
>+ /* allocate buffer for result data, if required */
>+ if (thread->result_data)
>+ {
>+ free( thread->result_data );
>+ thread->result_data = NULL;
>+ }
>+ if ((thread->result_size = get_req_data_size()))
>+ {
>+ if ((thread->result_data = mem_alloc( thread->result_size )))
>+ memcpy( thread->result_data, get_req_data(), thread->result_size );
>+ else
>+ thread->remote_status = get_error();
>+ }
>+
>+ /* set event */
>+ if (thread->result_event)
>+ {
>+ set_event( thread->result_event );
>+ release_object( thread->result_event );
>+ thread->result_event = NULL;
>+ }
>+ release_object( thread );
>+}
>+
>+/* return status and result data from remote opertaion */
>+DECL_HANDLER(remote_operation_result)
>+{
>+ reply->status = current->remote_status;
>+
>+ if (current->result_data)
>+ {
>+ void *result = current->result_data;
>+ current->result_data = NULL;
>+ set_reply_data_ptr( result, current->result_size );
> }
> }
>
>
Rob
More information about the wine-devel
mailing list