Read/WriteProcessMemory fix

Andrew andrew at transgaming.com
Thu Dec 6 17:09:58 CST 2001


This fixes a bug in ReadProcessMemory and WriteProcessMemory, and
rearranges some of the code. (In particular, all the code that knows
that ptrace works in ints has been contained to server/ptrace.c.)

I hope that the servercall bits are correct.

Andrew Lewycky
andrew at transgaming.com

-------------- next part --------------
Index: scheduler/process.c
===================================================================
RCS file: /home/wine/wine/scheduler/process.c,v
retrieving revision 1.167
diff -u -p -r1.167 process.c
--- scheduler/process.c	2001/12/04 19:50:18	1.167
+++ scheduler/process.c	2001/12/06 21:28:33
@@ -1365,8 +1365,6 @@ BOOL WINAPI ReadProcessMemory( HANDLE pr
 BOOL WINAPI WriteProcessMemory( HANDLE process, LPVOID addr, LPCVOID buffer, DWORD size,
                                 LPDWORD bytes_written )
 {
-    static const int zero;
-    unsigned int first_offset, last_offset, first_mask, last_mask;
     DWORD res;
 
     if (!size)
@@ -1375,26 +1373,11 @@ BOOL WINAPI WriteProcessMemory( HANDLE p
         return FALSE;
     }
 
-    /* compute the mask for the first int */
-    first_mask = ~0;
-    first_offset = (unsigned int)addr % sizeof(int);
-    memset( &first_mask, 0, first_offset );
-
-    /* compute the mask for the last int */
-    last_offset = (size + first_offset) % sizeof(int);
-    last_mask = 0;
-    memset( &last_mask, 0xff, last_offset ? last_offset : sizeof(int) );
-
     SERVER_START_REQ( write_process_memory )
     {
         req->handle     = process;
-        req->addr       = (char *)addr - first_offset;
-        req->first_mask = first_mask;
-        req->last_mask  = last_mask;
-        if (first_offset) wine_server_add_data( req, &zero, first_offset );
+        req->addr       = (char *)addr;
         wine_server_add_data( req, buffer, size );
-        if (last_offset) wine_server_add_data( req, &zero, sizeof(int) - last_offset );
-
         if ((res = wine_server_call_err( req ))) size = 0;
     }
     SERVER_END_REQ;
Index: server/process.c
===================================================================
RCS file: /home/wine/wine/server/process.c,v
retrieving revision 1.75
diff -u -p -r1.75 process.c
--- server/process.c	2001/12/04 20:17:44	1.75
+++ server/process.c	2001/12/06 21:28:34
@@ -595,57 +595,73 @@ static void set_process_info( struct pro
     }
 }
 
-/* read data from a process memory space */
-/* len is the total size (in ints) */
-static int read_process_memory( struct process *process, const int *addr, size_t len, int *dest )
+/* read data from a process memory space
+ * addr is the virtual address in that process, len is the number of bytes.
+ *
+ * check_len (if nonzero) is the length of the range that needs to be
+ * checked for read permissions.
+ */
+static int read_process_memory( struct process *process, uintptr_t addr,
+				size_t len, void *dest )
 {
     struct thread *thread = process->thread_list;
+    size_t bytes_read;
 
-    assert( !((unsigned int)addr % sizeof(int)) );  /* address must be aligned */
-
     if (!thread)  /* process is dead */
     {
         set_error( STATUS_ACCESS_DENIED );
         return 0;
     }
+
     if (suspend_for_ptrace( thread ))
     {
-        while (len > 0)
-        {
-            if (read_thread_int( thread, addr++, dest++ ) == -1) break;
-            len--;
-        }
+	read_thread_data( thread, addr, len, dest, &bytes_read );
+
         resume_thread( thread );
     }
-    return !len;
+
+    return bytes_read == len;
 }
 
 /* make sure we can write to the whole address range */
-/* len is the total size (in ints) */
-static int check_process_write_access( struct thread *thread, int *addr, size_t len )
+/* len is the total size */
+static int check_process_write_access( struct thread *thread, uintptr_t addr,
+				       size_t len )
 {
     int page = get_page_size() / sizeof(int);
+    size_t dummy;
+    char buf;
 
     for (;;)
     {
-        if (write_thread_int( thread, addr, 0, 0 ) == -1) return 0;
+	if (read_thread_data( thread, addr, sizeof(buf), &buf, &dummy ) == -1)
+	    return 0;
+	if (write_thread_data( thread, addr, sizeof(buf), &buf, &dummy ) == -1)
+	    return 0;
+
         if (len <= page) break;
         addr += page;
         len -= page;
     }
-    return (write_thread_int( thread, addr + len - 1, 0, 0 ) != -1);
+
+    addr += len - 1;
+
+    if (read_thread_data( thread, addr, sizeof(buf), &buf, &dummy) == -1)
+	return 0;
+    if (write_thread_data( thread, addr, sizeof(buf), &buf, &dummy ) == -1)
+	return 0;
+
+    return 1;
 }
 
 /* write data to a process memory space */
 /* len is the total size (in ints), max is the size we can actually read from the input buffer */
 /* we check the total size for write permissions */
-static void write_process_memory( struct process *process, int *addr, size_t len,
-                                  unsigned int first_mask, unsigned int last_mask, const int *src )
+static void write_process_memory( struct process *process, uintptr_t addr,
+				  size_t len, const void *src )
 {
     struct thread *thread = process->thread_list;
 
-    assert( !((unsigned int)addr % sizeof(int) ));  /* address must be aligned */
-
     if (!thread)  /* process is dead */
     {
         set_error( STATUS_ACCESS_DENIED );
@@ -653,29 +669,16 @@ static void write_process_memory( struct
     }
     if (suspend_for_ptrace( thread ))
     {
+	size_t bytes_written;
+
         if (!check_process_write_access( thread, addr, len ))
         {
             set_error( STATUS_ACCESS_DENIED );
             return;
         }
-        /* first word is special */
-        if (len > 1)
-        {
-            if (write_thread_int( thread, addr++, *src++, first_mask ) == -1) goto done;
-            len--;
-        }
-        else last_mask &= first_mask;
 
-        while (len > 1)
-        {
-            if (write_thread_int( thread, addr++, *src++, ~0 ) == -1) goto done;
-            len--;
-        }
+	write_thread_data( thread, addr, len, src, &bytes_written );
 
-        /* last word is special too */
-        if (write_thread_int( thread, addr, *src, last_mask ) == -1) goto done;
-
-    done:
         resume_thread( thread );
     }
 }
@@ -886,45 +889,33 @@ DECL_HANDLER(set_process_info)
 DECL_HANDLER(read_process_memory)
 {
     struct process *process;
-    size_t len = get_reply_max_size();
 
-    if (!(process = get_process_from_handle( req->handle, PROCESS_VM_READ ))) return;
+    process = get_process_from_handle( req->handle, PROCESS_VM_READ );
 
-    if (len)
+    if (process)
     {
-        unsigned int start_offset = (unsigned int)req->addr % sizeof(int);
-        unsigned int nb_ints = (len + start_offset + sizeof(int) - 1) / sizeof(int);
-        const int *start = (int *)((char *)req->addr - start_offset);
-        int *buffer = mem_alloc( nb_ints * sizeof(int) );
-        if (buffer)
-        {
-            if (read_process_memory( process, start, nb_ints, buffer ))
-            {
-                /* move start of requested data to start of buffer */
-                if (start_offset) memmove( buffer, (char *)buffer + start_offset, len );
-                set_reply_data_ptr( buffer, len );
-            }
-            else len = 0;
-        }
+	size_t len = get_reply_max_size();
+
+	read_process_memory( process, (uintptr_t)req->addr, len,
+			     set_reply_data_size( len ) );
+
+	release_object( process );
     }
-    release_object( process );
 }
 
 /* write data to a process address space */
 DECL_HANDLER(write_process_memory)
 {
     struct process *process;
+
+    process = get_process_from_handle( req->handle, PROCESS_VM_WRITE );
 
-    if ((process = get_process_from_handle( req->handle, PROCESS_VM_WRITE )))
+    if (process)
     {
-        size_t len = get_req_data_size();
-        if ((len % sizeof(int)) || ((unsigned int)req->addr % sizeof(int)))
-            set_error( STATUS_INVALID_PARAMETER );
-        else
-        {
-            if (len) write_process_memory( process, req->addr, len / sizeof(int),
-                                           req->first_mask, req->last_mask, get_req_data() );
-        }
+	size_t len = get_req_data_size();
+
+	write_process_memory( process, (uintptr_t)req->addr, len,
+			      set_reply_data_size( len ) );
         release_object( process );
     }
 }
Index: server/ptrace.c
===================================================================
RCS file: /home/wine/wine/server/ptrace.c,v
retrieving revision 1.12
diff -u -p -r1.12 ptrace.c
--- server/ptrace.c	2001/07/14 00:50:30	1.12
+++ server/ptrace.c	2001/12/06 21:28:34
@@ -201,8 +201,10 @@ int suspend_for_ptrace( struct thread *t
 }
 
 /* read an int from a thread address space */
-int read_thread_int( struct thread *thread, const int *addr, int *data )
+static int read_thread_int( struct thread *thread, const uint32_t addr,
+			    uint32_t *data )
 {
+    errno = 0;
     *data = ptrace( PTRACE_PEEKDATA, thread->unix_pid, (caddr_t)addr, 0 );
     if ( *data == -1 && errno)
     {
@@ -212,16 +214,145 @@ int read_thread_int( struct thread *thre
     return 0;
 }
 
-/* write an int to a thread address space */
-int write_thread_int( struct thread *thread, int *addr, int data, unsigned int mask )
+int read_thread_data( struct thread *thread, uintptr_t addr, size_t len,
+		      void *data, size_t *p_num_read)
 {
-    int res;
-    if (mask != ~0)
+    size_t num_read = 0;
+    char* p_data = (char *)data;
+    size_t num_words;
+
+    if ( addr % sizeof(uint32_t) != 0 )
     {
-        if (read_thread_int( thread, addr, &res ) == -1) return -1;
-        data = (data & mask) | (res & ~mask);
+	/* partial word read */
+
+	uintptr_t aligned_addr = addr - (addr % sizeof(uint32_t));
+	unsigned int bytes_wanted = sizeof(uint32_t) - (addr % sizeof(uint32_t));
+
+	uint32_t buf;
+	if (read_thread_int( thread, aligned_addr, &buf ) == -1) goto out_err;
+
+	/* endian-specific: transfer the last bytes in buf */
+	memcpy(p_data, (char *)&buf + sizeof(buf) - bytes_wanted,
+	       bytes_wanted);
+
+	p_data += bytes_wanted;
+	num_read += bytes_wanted;
+	addr += bytes_wanted;
+	len -= bytes_wanted;
     }
+
+    num_words = len / sizeof(uint32_t);
+
+    while (num_words-- > 0)
+    {
+	uint32_t buf;
+	if (read_thread_int( thread, addr, &buf ) == -1) goto out_err;
+
+	memcpy(p_data, &buf, sizeof(buf));
+
+	p_data += sizeof(buf);
+	num_read += sizeof(buf);
+	addr += sizeof(buf);
+    }
+
+    len %= sizeof(uint32_t);
+
+    if ( len != 0 )
+    {
+	/* partial word read */
+
+	uint32_t buf;
+	if (read_thread_int( thread, addr, &buf ) == -1) goto out_err;
+
+	/* endian-specific: transfer the first bytes in buf */
+	memcpy(p_data, (char *)&buf, len);
+
+	num_read += len;
+    }
+
+    *p_num_read = num_read;
+    return 0;
+
+out_err:
+    *p_num_read = num_read;
+    return -1;
+}
+
+/* write an int to a thread address space */
+static int write_thread_int( struct thread *thread, uintptr_t addr,
+			     uint32_t data )
+{
+    int res;
     if ((res = ptrace( PTRACE_POKEDATA, thread->unix_pid, (caddr_t)addr, data )) == -1)
         file_set_error();
     return res;
+}
+
+int write_thread_data( struct thread *thread, uintptr_t addr, size_t len,
+		       const void *data, size_t *p_num_written)
+{
+    size_t num_written = 0;
+    char* p_data = (char *)data;
+    size_t num_words;
+
+    if ( addr % sizeof(uint32_t) != 0 )
+    {
+	/* partial word write */
+
+	uintptr_t aligned_addr = addr - (addr % sizeof(uint32_t));
+	unsigned int bytes_wanted = sizeof(uint32_t) - (addr % sizeof(uint32_t));
+
+	uint32_t buf;
+	if (read_thread_int( thread, aligned_addr, &buf ) == -1) goto out_err;
+
+	/* endian-specific: transfer the last bytes in buf */
+	memcpy((char *)&buf + sizeof(buf) - bytes_wanted, p_data,
+	       bytes_wanted);
+
+	if (write_thread_int( thread, aligned_addr, buf ) == -1) goto out_err;
+
+	p_data += bytes_wanted;
+	num_written += bytes_wanted;
+	addr += bytes_wanted;
+	len -= bytes_wanted;
+    }
+
+    num_words = len / sizeof(uint32_t);
+
+    while (num_words-- > 0)
+    {
+	uint32_t buf;
+	memcpy(p_data, &buf, sizeof(buf));
+
+	if (write_thread_int( thread, addr, buf ) == -1) goto out_err;
+
+	p_data += sizeof(buf);
+	num_written += sizeof(buf);
+	addr += sizeof(buf);
+    }
+
+    len %= sizeof(uint32_t);
+
+    if ( len != 0 )
+    {
+	/* partial word write */
+
+	uint32_t buf;
+
+	if (read_thread_int( thread, addr, &buf ) == -1) goto out_err;
+
+	/* endian-specific: transfer the first bytes into buf */
+	memcpy((char *)&buf, p_data, len);
+
+	if (write_thread_int( thread, addr, buf ) == -1) goto out_err;
+
+	num_written += len;
+    }
+
+    *p_num_written = num_written;
+    return 0;
+
+out_err:
+    *p_num_written = num_written;
+    return -1;
 }
Index: server/thread.c
===================================================================
RCS file: /home/wine/wine/server/thread.c,v
retrieving revision 1.71
diff -u -p -r1.71 thread.c
--- server/thread.c	2001/12/04 20:17:44	1.71
+++ server/thread.c	2001/12/06 21:28:34
@@ -671,11 +671,18 @@ static void get_selector_entry( struct t
     if (suspend_for_ptrace( thread ))
     {
         unsigned char flags_buf[4];
-        int *addr = (int *)thread->process->ldt_copy + entry;
-        if (read_thread_int( thread, addr, base ) == -1) goto done;
-        if (read_thread_int( thread, addr + 8192, limit ) == -1) goto done;
-        addr = (int *)thread->process->ldt_copy + 2*8192 + (entry >> 2);
-        if (read_thread_int( thread, addr, (int *)flags_buf ) == -1) goto done;
+	size_t num_read;
+        uintptr_t addr = (uintptr_t)((uint32_t *)thread->process->ldt_copy + entry);
+
+        if (read_thread_data( thread, addr, sizeof(*base), base,
+			      &num_read ) == -1) goto done;
+        if (read_thread_data( thread, addr + 8192, sizeof(*limit), limit,
+			      &num_read ) == -1) goto done;
+
+        addr = (uintptr_t)((int *)thread->process->ldt_copy + 2*8192 + (entry >> 2));
+        if (read_thread_data( thread, addr, sizeof(flags_buf), flags_buf,
+			      &num_read ) == -1) goto done;
+
         *flags = flags_buf[entry & 3];
     done:
         resume_thread( thread );
Index: server/thread.h
===================================================================
RCS file: /home/wine/wine/server/thread.h,v
retrieving revision 1.37
diff -u -p -r1.37 thread.h
--- server/thread.h	2001/11/30 18:46:51	1.37
+++ server/thread.h	2001/12/06 21:28:34
@@ -7,6 +7,7 @@
 #ifndef __WINE_SERVER_THREAD_H
 #define __WINE_SERVER_THREAD_H
 
+#include <stdint.h>
 #include "object.h"
 
 /* thread structure */
@@ -114,8 +115,8 @@ extern void stop_thread( struct thread *
 extern void continue_thread( struct thread *thread );
 extern void detach_thread( struct thread *thread, int sig );
 extern int suspend_for_ptrace( struct thread *thread );
-extern int read_thread_int( struct thread *thread, const int *addr, int *data );
-extern int write_thread_int( struct thread *thread, int *addr, int data, unsigned int mask );
+extern int read_thread_data( struct thread *thread, uintptr_t addr, size_t len, void *data, size_t *num_read );
+extern int write_thread_data( struct thread *thread, uintptr_t addr, size_t len, const void *data, size_t *num_written );
 extern void *get_thread_ip( struct thread *thread );
 extern int get_thread_single_step( struct thread *thread );
 


More information about the wine-patches mailing list