Linux user memory access API change (initial patch by Thayne Harbaugh)

git-svn-id: svn://svn.savannah.nongnu.org/qemu/trunk@3583 c046a42c-6fe2-441c-8c8c-71466251a162

Makefile.target

@@ -260,7 +260,7 @@ endif
 
 ifdef CONFIG_LINUX_USER
 OBJS= main.o syscall.o strace.o mmap.o signal.o path.o osdep.o thunk.o \
-      elfload.o linuxload.o
+      elfload.o linuxload.o uaccess.o
 LIBS+= $(AIOLIBS)
 ifdef TARGET_HAS_BFLT
 OBJS+= flatload.o

arm-semi.c

@@ -184,9 +184,11 @@ uint32_t do_arm_semihosting(CPUState *env)
     args = env->regs[1];
     switch (nr) {
     case SYS_OPEN:
-        s = lock_user_string(ARG(0));
+        if (!(s = lock_user_string(ARG(0))))
+            /* FIXME - should this error code be -TARGET_EFAULT ? */
+            return (uint32_t)-1;
         if (ARG(1) >= 12)
-          return (uint32_t)-1;
+            return (uint32_t)-1;
         if (strcmp(s, ":tt") == 0) {
             if (ARG(1) < 4)
                 return STDIN_FILENO;
@@ -221,7 +223,9 @@ uint32_t do_arm_semihosting(CPUState *env)
           }
         }
     case SYS_WRITE0:
-        s = lock_user_string(args);
+        if (!(s = lock_user_string(args)))
+            /* FIXME - should this error code be -TARGET_EFAULT ? */
+            return (uint32_t)-1;
         len = strlen(s);
         if (use_gdb_syscalls()) {
             gdb_do_syscall(arm_semi_cb, "write,2,%x,%x\n", args, len);
@@ -238,7 +242,9 @@ uint32_t do_arm_semihosting(CPUState *env)
             gdb_do_syscall(arm_semi_cb, "write,%x,%x,%x", ARG(0), ARG(1), len);
             return env->regs[0];
         } else {
-            s = lock_user(ARG(1), len, 1);
+            if (!(s = lock_user(VERIFY_READ, ARG(1), len, 1)))
+                /* FIXME - should this error code be -TARGET_EFAULT ? */
+                return (uint32_t)-1;
             ret = set_swi_errno(ts, write(ARG(0), s, len));
             unlock_user(s, ARG(1), 0);
             if (ret == (uint32_t)-1)
@@ -252,7 +258,9 @@ uint32_t do_arm_semihosting(CPUState *env)
             gdb_do_syscall(arm_semi_cb, "read,%x,%x,%x", ARG(0), ARG(1), len);
             return env->regs[0];
         } else {
-            s = lock_user(ARG(1), len, 0);
+            if (!(s = lock_user(VERIFY_WRITE, ARG(1), len, 0)))
+                /* FIXME - should this error code be -TARGET_EFAULT ? */
+                return (uint32_t)-1;
             do
               ret = set_swi_errno(ts, read(ARG(0), s, len));
             while (ret == -1 && errno == EINTR);
@@ -301,7 +309,9 @@ uint32_t do_arm_semihosting(CPUState *env)
             gdb_do_syscall(arm_semi_cb, "unlink,%s", ARG(0), (int)ARG(1)+1);
             ret = env->regs[0];
         } else {
-            s = lock_user_string(ARG(0));
+            if (!(s = lock_user_string(ARG(0))))
+                /* FIXME - should this error code be -TARGET_EFAULT ? */
+                return (uint32_t)-1;
             ret =  set_swi_errno(ts, remove(s));
             unlock_user(s, ARG(0), 0);
         }
@@ -315,9 +325,15 @@ uint32_t do_arm_semihosting(CPUState *env)
             char *s2;
             s = lock_user_string(ARG(0));
             s2 = lock_user_string(ARG(2));
-            ret = set_swi_errno(ts, rename(s, s2));
-            unlock_user(s2, ARG(2), 0);
-            unlock_user(s, ARG(0), 0);
+            if (!s || !s2)
+                /* FIXME - should this error code be -TARGET_EFAULT ? */
+                ret = (uint32_t)-1;
+            else
+                ret = set_swi_errno(ts, rename(s, s2));
+            if (s2)
+                unlock_user(s2, ARG(2), 0);
+            if (s)
+                unlock_user(s, ARG(0), 0);
             return ret;
         }
     case SYS_CLOCK:
@@ -329,7 +345,9 @@ uint32_t do_arm_semihosting(CPUState *env)
             gdb_do_syscall(arm_semi_cb, "system,%s", ARG(0), (int)ARG(1)+1);
             return env->regs[0];
         } else {
-            s = lock_user_string(ARG(0));
+            if (!(s = lock_user_string(ARG(0))))
+                /* FIXME - should this error code be -TARGET_EFAULT ? */
+                return (uint32_t)-1;
             ret = set_swi_errno(ts, system(s));
             unlock_user(s, ARG(0), 0);
         }
@@ -346,7 +364,11 @@ uint32_t do_arm_semihosting(CPUState *env)
             char **arg = ts->info->host_argv;
             int len = ARG(1);
             /* lock the buffer on the ARM side */
-            char *cmdline_buffer = (char*)lock_user(ARG(0), len, 0);
+            char *cmdline_buffer = (char*)lock_user(VERIFY_WRITE, ARG(0), len, 0);
+
+            if (!cmdline_buffer)
+                /* FIXME - should this error code be -TARGET_EFAULT ? */
+                return (uint32_t)-1;
 
             s = cmdline_buffer;
             while (*arg && len > 2) {
@@ -402,15 +424,19 @@ uint32_t do_arm_semihosting(CPUState *env)
                 ts->heap_limit = limit;
             }
 
-            ptr = lock_user(ARG(0), 16, 0);
+            if (!(ptr = lock_user(VERIFY_WRITE, ARG(0), 16, 0)))
+                /* FIXME - should this error code be -TARGET_EFAULT ? */
+                return (uint32_t)-1;
             ptr[0] = tswap32(ts->heap_base);
             ptr[1] = tswap32(ts->heap_limit);
             ptr[2] = tswap32(ts->stack_base);
             ptr[3] = tswap32(0); /* Stack limit.  */
             unlock_user(ptr, ARG(0), 16);
 #else
             limit = ram_size;
-            ptr = lock_user(ARG(0), 16, 0);
+            if (!(ptr = lock_user(VERIFY_WRITE, ARG(0), 16, 0)))
+                /* FIXME - should this error code be -TARGET_EFAULT ? */
+                return (uint32_t)-1;
             /* TODO: Make this use the limit of the loaded application.  */
             ptr[0] = tswap32(limit / 2);
             ptr[1] = tswap32(limit);

exec.c

@@ -2510,13 +2510,19 @@ void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
         if (is_write) {
             if (!(flags & PAGE_WRITE))
                 return;
-            p = lock_user(addr, len, 0);
+            /* XXX: this code should not depend on lock_user */
+            if (!(p = lock_user(VERIFY_WRITE, addr, len, 0)))
+                /* FIXME - should this return an error rather than just fail? */
+                return;
             memcpy(p, buf, len);
             unlock_user(p, addr, len);
         } else {
             if (!(flags & PAGE_READ))
                 return;
-            p = lock_user(addr, len, 1);
+            /* XXX: this code should not depend on lock_user */
+            if (!(p = lock_user(VERIFY_READ, addr, len, 1)))
+                /* FIXME - should this return an error rather than just fail? */
+                return;
             memcpy(buf, p, len);
             unlock_user(p, addr, 0);
         }

linux-user/elfload.c

@@ -677,7 +677,7 @@ static abi_ulong setup_arg_pages(abi_ulong p, struct linux_binprm *bprm,
     for (i = 0 ; i < MAX_ARG_PAGES ; i++) {
 	if (bprm->page[i]) {
 	    info->rss++;
-
+            /* FIXME - check return value of memcpy_to_target() for failure */
 	    memcpy_to_target(stack_base, bprm->page[i], TARGET_PAGE_SIZE);
 	    free(bprm->page[i]);
 	}
@@ -760,6 +760,7 @@ static abi_ulong create_elf_tables(abi_ulong p, int argc, int envc,
             size_t len = strlen(k_platform) + 1;
             sp -= (len + n - 1) & ~(n - 1);
             u_platform = sp;
+            /* FIXME - check return value of memcpy_to_target() for failure */
             memcpy_to_target(sp, k_platform, len);
         }
 	/*

linux-user/flatload.c

@@ -108,7 +108,7 @@ int target_pread(int fd, abi_ulong ptr, abi_ulong len,
     void *buf;
     int ret;
 
-    buf = lock_user(ptr, len, 0);
+    buf = lock_user(VERIFY_WRITE, ptr, len, 0);
     ret = pread(fd, buf, len, offset);
     unlock_user(buf, ptr, len);
     return ret;

linux-user/linuxload.c

@@ -13,14 +13,17 @@
 #define NGROUPS 32
 
 /* ??? This should really be somewhere else.  */
-void memcpy_to_target(abi_ulong dest, const void *src,
-                      unsigned long len)
+abi_long memcpy_to_target(abi_ulong dest, const void *src,
+                          unsigned long len)
 {
     void *host_ptr;
 
-    host_ptr = lock_user(dest, len, 0);
+    host_ptr = lock_user(VERIFY_WRITE, dest, len, 0);
+    if (!host_ptr)
+        return -TARGET_EFAULT;
     memcpy(host_ptr, src, len);
     unlock_user(host_ptr, dest, 1);
+    return 0;
 }
 
 static int in_group_p(gid_t g)

linux-user/qemu.h

@@ -146,8 +146,8 @@ int load_elf_binary_multi(struct linux_binprm *bprm,
                           struct image_info *info);
 #endif
 
-void memcpy_to_target(abi_ulong dest, const void *src,
-                      unsigned long len);
+abi_long memcpy_to_target(abi_ulong dest, const void *src,
+                          unsigned long len);
 void target_set_brk(abi_ulong new_brk);
 abi_long do_brk(abi_ulong new_brk);
 void syscall_init(void);
@@ -179,9 +179,7 @@ void host_to_target_siginfo(target_siginfo_t *tinfo, const siginfo_t *info);
 void target_to_host_siginfo(siginfo_t *info, const target_siginfo_t *tinfo);
 long do_sigreturn(CPUState *env);
 long do_rt_sigreturn(CPUState *env);
-int do_sigaltstack(const struct target_sigaltstack *uss,
-                   struct target_sigaltstack *uoss,
-                   abi_ulong sp);
+abi_long do_sigaltstack(abi_ulong uss_addr, abi_ulong uoss_addr, abi_ulong sp);
 
 #ifdef TARGET_I386
 /* vm86.c */
@@ -207,12 +205,15 @@ int target_msync(abi_ulong start, abi_ulong len, int flags);
 /* user access */
 
 #define VERIFY_READ 0
-#define VERIFY_WRITE 1
+#define VERIFY_WRITE 1 /* implies read access */
 
 #define access_ok(type,addr,size) \
     (page_check_range((target_ulong)addr,size,(type==VERIFY_READ)?PAGE_READ:PAGE_WRITE)==0)
 
 /* NOTE __get_user and __put_user use host pointers and don't check access. */
+/* These are usually used to access struct data members once the
+ * struct has been locked - usually with lock_user_struct().
+ */
 #define __put_user(x, hptr)\
 ({\
     int size = sizeof(*hptr);\
@@ -257,26 +258,44 @@ int target_msync(abi_ulong start, abi_ulong len, int flags);
     0;\
 })
 
-#define put_user(x,ptr)\
-({\
-    int __ret;\
-    if (access_ok(VERIFY_WRITE, ptr, sizeof(*ptr)))\
-        __ret = __put_user(x, ptr);\
-    else\
-        __ret = -EFAULT;\
-    __ret;\
+/* put_user()/get_user() take a guest address and check access */
+/* These are usually used to access an atomic data type, such as an int,
+ * that has been passed by address.  These internally perform locking
+ * and unlocking on the data type.
+ */
+#define put_user(x, gaddr, target_type)					\
+({									\
+    abi_ulong __gaddr = (gaddr);					\
+    target_type *__hptr;						\
+    abi_long __ret;							\
+    if ((__hptr = lock_user(VERIFY_WRITE, __gaddr, sizeof(target_type), 0))) { \
+        __ret = __put_user((x), __hptr);				\
+        unlock_user(__hptr, __gaddr, sizeof(target_type));		\
+    } else								\
+        __ret = -TARGET_EFAULT;						\
+    __ret;								\
 })
 
-#define get_user(x,ptr)\
-({\
-    int __ret;\
-    if (access_ok(VERIFY_READ, ptr, sizeof(*ptr)))\
-        __ret = __get_user(x, ptr);\
-    else\
-        __ret = -EFAULT;\
-    __ret;\
+#define get_user(x, gaddr, target_type)					\
+({									\
+    abi_ulong __gaddr = (gaddr);					\
+    target_type *__hptr;						\
+    abi_long __ret;							\
+    if ((__hptr = lock_user(VERIFY_READ, __gaddr, sizeof(target_type), 1))) { \
+        __ret = __get_user((x), __hptr);				\
+        unlock_user(__hptr, __gaddr, 0);				\
+    } else								\
+        __ret = -TARGET_EFAULT;						\
+    __ret;								\
 })
 
+/* copy_from_user() and copy_to_user() are usually used to copy data
+ * buffers between the target and host.  These internally perform
+ * locking/unlocking of the memory.
+ */
+abi_long copy_from_user(void *hptr, abi_ulong gaddr, size_t len);
+abi_long copy_to_user(abi_ulong gaddr, void *hptr, size_t len);
+
 /* Functions for accessing guest memory.  The tget and tput functions
    read/write single values, byteswapping as neccessary.  The lock_user
    gets a pointer to a contiguous area of guest memory, but does not perform
@@ -285,53 +304,61 @@ int target_msync(abi_ulong start, abi_ulong len, int flags);
 
 /* Lock an area of guest memory into the host.  If copy is true then the
    host area will have the same contents as the guest.  */
-static inline void *lock_user(abi_ulong guest_addr, long len, int copy)
+static inline void *lock_user(int type, abi_ulong guest_addr, long len, int copy)
 {
+    if (!access_ok(type, guest_addr, len))
+        return NULL;
 #ifdef DEBUG_REMAP
-    void *addr;
-    addr = malloc(len);
-    if (copy)
-        memcpy(addr, g2h(guest_addr), len);
-    else
-        memset(addr, 0, len);
-    return addr;
+    {
+        void *addr;
+        addr = malloc(len);
+        if (copy)
+            memcpy(addr, g2h(guest_addr), len);
+        else
+            memset(addr, 0, len);
+        return addr;
+    }
 #else
     return g2h(guest_addr);
 #endif
 }
 
-/* Unlock an area of guest memory.  The first LEN bytes must be flushed back
-   to guest memory.  */
-static inline void unlock_user(void *host_addr, abi_ulong guest_addr,
+/* Unlock an area of guest memory.  The first LEN bytes must be
+   flushed back to guest memory. host_ptr = NULL is explicitely
+   allowed and does nothing. */
+static inline void unlock_user(void *host_ptr, abi_ulong guest_addr,
                                long len)
 {
+
 #ifdef DEBUG_REMAP
-    if (host_addr == g2h(guest_addr))
+    if (!host_ptr)
+        return;
+    if (host_ptr == g2h(guest_addr))
         return;
     if (len > 0)
-        memcpy(g2h(guest_addr), host_addr, len);
-    free(host_addr);
+        memcpy(g2h(guest_ptr), host_ptr, len);
+    free(host_ptr);
 #endif
 }
 
-/* Return the length of a string in target memory.  */
-static inline int target_strlen(abi_ulong ptr)
-{
-  return strlen(g2h(ptr));
-}
+/* Return the length of a string in target memory or -TARGET_EFAULT if
+   access error. */
+abi_long target_strlen(abi_ulong gaddr);
 
 /* Like lock_user but for null terminated strings.  */
 static inline void *lock_user_string(abi_ulong guest_addr)
 {
-    long len;
-    len = target_strlen(guest_addr) + 1;
-    return lock_user(guest_addr, len, 1);
+    abi_long len;
+    len = target_strlen(guest_addr);
+    if (len < 0)
+        return NULL;
+    return lock_user(VERIFY_READ, guest_addr, (long)(len + 1), 1);
 }
 
 /* Helper macros for locking/ulocking a target struct.  */
-#define lock_user_struct(host_ptr, guest_addr, copy) \
-    host_ptr = lock_user(guest_addr, sizeof(*host_ptr), copy)
-#define unlock_user_struct(host_ptr, guest_addr, copy) \
+#define lock_user_struct(type, host_ptr, guest_addr, copy)	\
+    (host_ptr = lock_user(type, guest_addr, sizeof(*host_ptr), copy))
+#define unlock_user_struct(host_ptr, guest_addr, copy)		\
     unlock_user(host_ptr, guest_addr, (copy) ? sizeof(*host_ptr) : 0)
 
 #define tget8(addr) ldub(addr)