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
