qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Stuart Anderson <anderson@netsweng.com>
To: qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH] linux-user EFAULT implementation
Date: Fri, 13 Jul 2007 13:55:52 -0400 (EDT)	[thread overview]
Message-ID: <Pine.LNX.4.64.0707131352380.32663@trantor.stuart.netsweng.com> (raw)
In-Reply-To: <Pine.LNX.4.64.0707101702540.28322@trantor.stuart.netsweng.com>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 911 bytes --]

On Tue, 10 Jul 2007, Stuart Anderson wrote:

> OK, I'll continue in this direction, after taking a fresh look at what I
> did yesterday.

Attached is a snapshot of the work after a couple more evenings on it.
I'm still doing a lot of testing, and I haven't finished converting
everything, but there should be enough in this patch to be able to tell
if this is heading in a good direction or not. There are even a couple of
systems calls actually yield better test suite results as a result of the
additional scrutiny of this process.

Further comments would be appreciated.



                                 Stuart

Stuart R. Anderson                               anderson@netsweng.com
Network & Software Engineering                   http://www.netsweng.com/
1024D/37A79149:                                  0791 D3B8 9A4C 2CDC A31F
                                                  BD03 0A62 E534 37A7 9149

[-- Attachment #2: EFAULT patch --]
[-- Type: TEXT/x-diff, Size: 57832 bytes --]

Index: qemu/exec.c
===================================================================
--- qemu.orig/exec.c	2007-07-13 07:10:58.000000000 -0400
+++ qemu/exec.c	2007-07-13 07:11:12.000000000 -0400
@@ -1862,6 +1862,29 @@
     spin_unlock(&tb_lock);
 }
 
+int page_check_range(target_ulong start, target_ulong len, int flags)
+{
+    PageDesc *p;
+    target_ulong end;
+    target_ulong addr;
+
+    end = TARGET_PAGE_ALIGN(start+len); /* must do before we loose bits in the next step */
+    start = start & TARGET_PAGE_MASK;
+
+    if( end < start ) return -1;  /* we've wrapped around */
+    for(addr = start; addr < end; addr += TARGET_PAGE_SIZE) {
+        p = page_find(addr >> TARGET_PAGE_BITS);
+	if( !p ) return -1;
+	if( !(p->flags & PAGE_VALID) ) return -1;
+
+        if (!(p->flags & PAGE_READ) &&
+            (flags & PAGE_READ) ) return -1;
+        if (!(p->flags & PAGE_WRITE) &&
+            (flags & PAGE_WRITE) ) return -1;
+    }
+    return 0;
+}
+
 /* called from signal handler: invalidate the code and unprotect the
    page. Return TRUE if the fault was succesfully handled. */
 int page_unprotect(target_ulong address, unsigned long pc, void *puc)
Index: qemu/cpu-all.h
===================================================================
--- qemu.orig/cpu-all.h	2007-07-13 07:10:58.000000000 -0400
+++ qemu/cpu-all.h	2007-07-13 07:11:12.000000000 -0400
@@ -691,6 +691,7 @@
 int page_get_flags(target_ulong address);
 void page_set_flags(target_ulong start, target_ulong end, int flags);
 void page_unprotect_range(target_ulong data, target_ulong data_size);
+int page_check_range(target_ulong start, target_ulong len, int flags);
 
 CPUState *cpu_copy(CPUState *env);
 
Index: qemu/linux-user/syscall.c
===================================================================
--- qemu.orig/linux-user/syscall.c	2007-07-13 07:10:58.000000000 -0400
+++ qemu/linux-user/syscall.c	2007-07-13 07:12:56.000000000 -0400
@@ -404,12 +404,10 @@
 #endif
 }
 
-static inline void host_to_target_rusage(target_ulong target_addr,
+static inline long copy_to_user_rusage(struct target_rusage *target_rusage,
                                          const struct rusage *rusage)
 {
-    struct target_rusage *target_rusage;
-
-    lock_user_struct(target_rusage, target_addr, 0);
+    if( copy_to_user(target_rusage,rusage,sizeof(*target_rusage)) ) return -1;
     target_rusage->ru_utime.tv_sec = tswapl(rusage->ru_utime.tv_sec);
     target_rusage->ru_utime.tv_usec = tswapl(rusage->ru_utime.tv_usec);
     target_rusage->ru_stime.tv_sec = tswapl(rusage->ru_stime.tv_sec);
@@ -428,29 +426,28 @@
     target_rusage->ru_nsignals = tswapl(rusage->ru_nsignals);
     target_rusage->ru_nvcsw = tswapl(rusage->ru_nvcsw);
     target_rusage->ru_nivcsw = tswapl(rusage->ru_nivcsw);
-    unlock_user_struct(target_rusage, target_addr, 1);
+
+    return 0;
 }
 
-static inline void target_to_host_timeval(struct timeval *tv,
-                                          target_ulong target_addr)
+static inline long copy_from_user_timeval(struct timeval *tv,
+                                          struct target_timeval *target_tv)
 {
-    struct target_timeval *target_tv;
-
-    lock_user_struct(target_tv, target_addr, 1);
+    if( copy_from_user(tv,target_tv,sizeof(*target_tv)) ) return -1;
     tv->tv_sec = tswapl(target_tv->tv_sec);
     tv->tv_usec = tswapl(target_tv->tv_usec);
-    unlock_user_struct(target_tv, target_addr, 0);
+
+    return 0;
 }
 
-static inline void host_to_target_timeval(target_ulong target_addr,
+static inline long copy_to_user_timeval(struct target_timeval *target_tv,
                                           const struct timeval *tv)
 {
-    struct target_timeval *target_tv;
-
-    lock_user_struct(target_tv, target_addr, 0);
+    if( copy_to_user(target_tv,tv,sizeof(*target_tv)) ) return -1;
     target_tv->tv_sec = tswapl(tv->tv_sec);
     target_tv->tv_usec = tswapl(tv->tv_usec);
-    unlock_user_struct(target_tv, target_addr, 1);
+
+    return 0;
 }
 
 
@@ -466,21 +463,21 @@
     int ok;
 
     if (rfd_p) {
-        target_rfds = lock_user(rfd_p, sizeof(target_long) * n, 1);
+	if( lock_and_check_user_struct(&target_rfds,rfd_p,sizeof(target_long)*n,1,PAGE_READ) ) return -EFAULT;
         rfds_ptr = target_to_host_fds(&rfds, target_rfds, n);
     } else {
         target_rfds = NULL;
         rfds_ptr = NULL;
     }
     if (wfd_p) {
-        target_wfds = lock_user(wfd_p, sizeof(target_long) * n, 1);
+	if( lock_and_check_user_struct(&target_wfds,wfd_p,sizeof(target_long)*n,1,PAGE_READ) ) return -EFAULT;
         wfds_ptr = target_to_host_fds(&wfds, target_wfds, n);
     } else {
         target_wfds = NULL;
         wfds_ptr = NULL;
     }
     if (efd_p) {
-        target_efds = lock_user(efd_p, sizeof(target_long) * n, 1);
+	if( lock_and_check_user_struct(&target_efds,efd_p,sizeof(target_long)*n,1,PAGE_READ) ) return -EFAULT;
         efds_ptr = target_to_host_fds(&efds, target_efds, n);
     } else {
         target_efds = NULL;
@@ -488,7 +485,7 @@
     }
             
     if (target_tv) {
-        target_to_host_timeval(&tv, target_tv);
+        if( copy_from_user_timeval(&tv, target_tv) ) return -EFAULT;
         tv_ptr = &tv;
     } else {
         tv_ptr = NULL;
@@ -502,7 +499,7 @@
         host_to_target_fds(target_efds, efds_ptr, n);
 
         if (target_tv) {
-            host_to_target_timeval(target_tv, &tv);
+            if( copy_to_user_timeval(target_tv, &tv) ) return -EFAULT;
         }
     }
     if (target_rfds)
@@ -515,28 +512,24 @@
     return ret;
 }
 
-static inline void target_to_host_sockaddr(struct sockaddr *addr,
-                                           target_ulong target_addr,
+static inline long copy_from_user_sockaddr(struct sockaddr *addr,
+                                           struct target_sockaddr *target_saddr,
                                            socklen_t len)
 {
-    struct target_sockaddr *target_saddr;
-
-    target_saddr = lock_user(target_addr, len, 1);
-    memcpy(addr, target_saddr, len);
+    if( copy_from_user(addr,target_saddr,len)) return -1;
     addr->sa_family = tswap16(target_saddr->sa_family);
-    unlock_user(target_saddr, target_addr, 0);
+
+    return 0;
 }
 
-static inline void host_to_target_sockaddr(target_ulong target_addr,
+static inline long copy_to_user_sockaddr(struct target_sockaddr *target_saddr,
                                            struct sockaddr *addr,
                                            socklen_t len)
 {
-    struct target_sockaddr *target_saddr;
-
-    target_saddr = lock_user(target_addr, len, 0);
-    memcpy(target_saddr, addr, len);
+    if( copy_to_user(target_saddr,addr,len) ) return -1;
     target_saddr->sa_family = tswap16(addr->sa_family);
-    unlock_user(target_saddr, target_addr, len);
+
+    return 0;
 }
 
 /* ??? Should this also swap msgh->name?  */
@@ -628,7 +621,7 @@
     msgh->msg_controllen = tswapl(space);
 }
 
-static long do_setsockopt(int sockfd, int level, int optname, 
+static long do_setsockopt(int sockfd, int level, int optname,
                           target_ulong optval, socklen_t optlen)
 {
     int val, ret;
@@ -639,6 +632,8 @@
         if (optlen < sizeof(uint32_t))
             return -EINVAL;
         
+	if( !access_ok(VERIFY_READ, optval, optlen) )
+            return -EFAULT;
         val = tget32(optval);
         ret = get_errno(setsockopt(sockfd, level, optname, &val, sizeof(val)));
         break;
@@ -660,6 +655,8 @@
         case IP_MULTICAST_TTL:
         case IP_MULTICAST_LOOP:
             val = 0;
+	    if( !access_ok(VERIFY_READ, optval, optlen) )
+                return -EFAULT;
             if (optlen >= sizeof(uint32_t)) {
                 val = tget32(optval);
             } else if (optlen >= 1) {
@@ -668,7 +665,7 @@
             ret = get_errno(setsockopt(sockfd, level, optname, &val, sizeof(val)));
             break;
         default:
-            goto unimplemented;
+	   goto unimplemented;
         }
         break;
     case TARGET_SOL_SOCKET:
@@ -732,24 +729,26 @@
 		break;
             break;
         default:
-            goto unimplemented;
+	   goto unimplemented;
         }
 	if (optlen < sizeof(uint32_t))
-	return -EINVAL;
+	    return -EINVAL;
+	if( !access_ok(VERIFY_READ, optval, optlen) )
+            return -EFAULT;
 
 	val = tget32(optval);
 	ret = get_errno(setsockopt(sockfd, SOL_SOCKET, optname, &val, sizeof(val)));
         break;
     default:
     unimplemented:
-        gemu_log("Unsupported setsockopt level=%d optname=%d \n", level, optname);
-        ret = -ENOSYS;
+        gemu_log("Unsupported setsockopt level=%d optname=%d\n", level, optname);
+        ret = -ENOPROTOOPT;
     }
     return ret;
 }
 
-static long do_getsockopt(int sockfd, int level, int optname, 
-                          target_ulong optval, target_ulong optlen)
+static long do_getsockopt(int sockfd, int level, int optname,
+                          target_ulong optval, socklen_t *optlen)
 {
     int len, lv, val, ret;
 
@@ -771,7 +770,10 @@
     case SOL_TCP:
         /* TCP options all take an 'int' value.  */
     int_case:
-        len = tget32(optlen);
+	if( get_user(len,optlen) )
+            return -EFAULT;
+	if( !access_ok(VERIFY_WRITE, optval,len) )
+            return -EFAULT;
         if (len < 0)
             return -EINVAL;
         lv = sizeof(int);
@@ -804,7 +806,10 @@
 #endif
         case IP_MULTICAST_TTL:
         case IP_MULTICAST_LOOP:
-            len = tget32(optlen);
+	    if( get_user(len,optlen) )
+                return -EINVAL;
+	if( !access_ok(VERIFY_WRITE, optval,len) )
+            return -EFAULT;
             if (len < 0)
                 return -EINVAL;
             lv = sizeof(int);
@@ -823,14 +828,15 @@
             }
             break;
         default:
-            goto unimplemented;
+            ret = -ENOPROTOOPT;
+            break;
         }
         break;
     default:
     unimplemented:
         gemu_log("getsockopt level=%d optname=%d not yet supported\n",
                  level, optname);
-        ret = -ENOSYS;
+        ret = -EOPNOTSUPP;
         break;
     }
     return ret;
@@ -899,7 +905,7 @@
 {
     void *addr = alloca(addrlen);
     
-    target_to_host_sockaddr(addr, target_addr, addrlen);
+    if( copy_from_user_sockaddr(addr, target_addr, addrlen) ) return -EFAULT;
     return get_errno(bind(sockfd, addr, addrlen));
 }
 
@@ -908,7 +914,7 @@
 {
     void *addr = alloca(addrlen);
     
-    target_to_host_sockaddr(addr, target_addr, addrlen);
+    if( copy_from_user_sockaddr(addr, target_addr, addrlen) ) return -EFAULT;
     return get_errno(connect(sockfd, addr, addrlen));
 }
 
@@ -922,12 +928,25 @@
     struct iovec *vec;
     target_ulong target_vec;
 
-    lock_user_struct(msgp, target_msg, 1);
+    if( send ) {
+       if( lock_and_check_user_struct(&msgp,target_msg,sizeof(*msgp),1,
+		PAGE_READ)  ) return -EFAULT;
+    } else {
+       if( lock_and_check_user_struct(&msgp,target_msg,sizeof(*msgp),1,
+		PAGE_WRITE) ) return -EFAULT;
+    }
     if (msgp->msg_name) {
         msg.msg_namelen = tswap32(msgp->msg_namelen);
         msg.msg_name = alloca(msg.msg_namelen);
-        target_to_host_sockaddr(msg.msg_name, tswapl(msgp->msg_name),
-                                msg.msg_namelen);
+        if( send ) {
+          if( copy_from_user_sockaddr(msg.msg_name, tswapl(msgp->msg_name),
+                                msg.msg_namelen) ) return -EFAULT;
+        } else {
+	  /* FIXME
+          if( (ret=copy_from_user_sockaddr(msg.msg_name, tswapl(msgp->msg_name),
+                                msg.msg_namelen,PAGE_WRITE)) != 0 ) return ret;
+				*/
+	}
     } else {
         msg.msg_name = NULL;
         msg.msg_namelen = 0;
@@ -956,46 +975,55 @@
 }
 
 static long do_accept(int fd, target_ulong target_addr,
-                      target_ulong target_addrlen)
+                      socklen_t *target_addrlen)
 {
-    socklen_t addrlen = tget32(target_addrlen);
-    void *addr = alloca(addrlen);
+    socklen_t addrlen;
+    void *addr;
     long ret;
 
+    if( get_user(addrlen,target_addrlen) ) return -EINVAL;
+    addr = alloca(addrlen);
+
     ret = get_errno(accept(fd, addr, &addrlen));
     if (!is_error(ret)) {
-        host_to_target_sockaddr(target_addr, addr, addrlen);
+        if( copy_to_user_sockaddr(target_addr, addr, addrlen) ) return -EFAULT;
         tput32(target_addrlen, addrlen);
     }
     return ret;
 }
 
 static long do_getpeername(int fd, target_ulong target_addr,
-                           target_ulong target_addrlen)
+                           socklen_t *target_addrlen)
 {
-    socklen_t addrlen = tget32(target_addrlen);
-    void *addr = alloca(addrlen);
+    socklen_t addrlen;
+    void *addr;
     long ret;
 
+    if( get_user(addrlen,target_addrlen) ) return -EFAULT;
+    addr = alloca(addrlen);
+
     ret = get_errno(getpeername(fd, addr, &addrlen));
     if (!is_error(ret)) {
-        host_to_target_sockaddr(target_addr, addr, addrlen);
-        tput32(target_addrlen, addrlen);
+        if( copy_to_user_sockaddr(target_addr, addr, addrlen) ) return -EFAULT;
+        tput32(target_addrlen,addrlen);
     }
     return ret;
 }
 
 static long do_getsockname(int fd, target_ulong target_addr,
-                           target_ulong target_addrlen)
+                           socklen_t *target_addrlen)
 {
-    socklen_t addrlen = tget32(target_addrlen);
-    void *addr = alloca(addrlen);
+    socklen_t addrlen;
+    void *addr;
     long ret;
 
+    if( get_user(addrlen,target_addrlen) ) return -EFAULT;
+    addr = alloca(addrlen);
+
     ret = get_errno(getsockname(fd, addr, &addrlen));
     if (!is_error(ret)) {
-        host_to_target_sockaddr(target_addr, addr, addrlen);
-        tput32(target_addrlen, addrlen);
+        if( copy_to_user_sockaddr(target_addr, addr, addrlen) ) return -EFAULT;
+        tput32(target_addrlen,addrlen);
     }
     return ret;
 }
@@ -1006,6 +1034,8 @@
     int tab[2];
     long ret;
 
+    if( !access_ok(VERIFY_WRITE,target_tab,2*sizeof(int)) )
+	return -EFAULT;
     ret = get_errno(socketpair(domain, type, protocol, tab));
     if (!is_error(ret)) {
         tput32(target_tab, tab[0]);
@@ -1018,18 +1048,17 @@
                       target_ulong target_addr, socklen_t addrlen)
 {
     void *addr;
-    void *host_msg;
+    void *host_msg = alloca(len);
     long ret;
 
-    host_msg = lock_user(msg, len, 1);
+    if (copy_from_user(host_msg,msg,len) ) return -EFAULT;
     if (target_addr) {
         addr = alloca(addrlen);
-        target_to_host_sockaddr(addr, target_addr, addrlen);
+        if (copy_from_user_sockaddr(addr, target_addr, addrlen) ) return -EFAULT;
         ret = get_errno(sendto(fd, host_msg, len, flags, addr, addrlen));
     } else {
         ret = get_errno(send(fd, host_msg, len, flags));
     }
-    unlock_user(host_msg, msg, 0);
     return ret;
 }
 
@@ -1038,10 +1067,9 @@
 {
     socklen_t addrlen;
     void *addr;
-    void *host_msg;
+    void *host_msg = alloca(len);
     long ret;
 
-    host_msg = lock_user(msg, len, 0);
     if (target_addr) {
         addrlen = tget32(target_addrlen);
         addr = alloca(addrlen);
@@ -1052,12 +1080,10 @@
     }
     if (!is_error(ret)) {
         if (target_addr) {
-            host_to_target_sockaddr(target_addr, addr, addrlen);
+            if (copy_to_user_sockaddr(target_addr, addr, addrlen) ) return -EFAULT;
             tput32(target_addrlen, addrlen);
         }
-        unlock_user(host_msg, msg, len);
-    } else {
-        unlock_user(host_msg, msg, 0);
+        if( copy_to_user(msg, host_msg, len) ) return -EFAULT;
     }
     return ret;
 }
@@ -1219,7 +1245,7 @@
         break;
     default:
         gemu_log("Unsupported socketcall: %d\n", num);
-        ret = -ENOSYS;
+        ret = -EINVAL;
         break;
     }
     return ret;
@@ -1259,64 +1285,44 @@
   target_ulong __unused4;
 };
 
-static inline void target_to_host_ipc_perm(struct ipc_perm *host_ip,
-                                           target_ulong target_addr)
+static inline void copy_from_user_ipc_perm(struct ipc_perm *host_ip,
+                                           struct target_ipc_perm *target_ip)
 {
-    struct target_ipc_perm *target_ip;
-    struct target_semid_ds *target_sd;
-
-    lock_user_struct(target_sd, target_addr, 1);
-    target_ip=&(target_sd->sem_perm);
     host_ip->__key = tswapl(target_ip->__key);
     host_ip->uid = tswapl(target_ip->uid);
     host_ip->gid = tswapl(target_ip->gid);
     host_ip->cuid = tswapl(target_ip->cuid);
     host_ip->cgid = tswapl(target_ip->cgid);
     host_ip->mode = tswapl(target_ip->mode);
-    unlock_user_struct(target_sd, target_addr, 0);
 }
 
-static inline void host_to_target_ipc_perm(target_ulong target_addr,
+static inline void copy_to_user_ipc_perm(struct target_ipc_perm *target_ip,
                                            struct ipc_perm *host_ip)
 {
-    struct target_ipc_perm *target_ip;
-    struct target_semid_ds *target_sd;
-
-    lock_user_struct(target_sd, target_addr, 0);
-    target_ip = &(target_sd->sem_perm);
     target_ip->__key = tswapl(host_ip->__key);
     target_ip->uid = tswapl(host_ip->uid);
     target_ip->gid = tswapl(host_ip->gid);
     target_ip->cuid = tswapl(host_ip->cuid);
     target_ip->cgid = tswapl(host_ip->cgid);
     target_ip->mode = tswapl(host_ip->mode);
-    unlock_user_struct(target_sd, target_addr, 1);
 }
 
-static inline void target_to_host_semid_ds(struct semid_ds *host_sd,
-                                          target_ulong target_addr)
+static inline void copy_from_user_semid_ds(struct semid_ds *host_sd,
+                                          struct target_semid_ds *target_sd)
 {
-    struct target_semid_ds *target_sd;
-
-    lock_user_struct(target_sd, target_addr, 1);
-    target_to_host_ipc_perm(&(host_sd->sem_perm),target_addr);
+    copy_from_user_ipc_perm(&(host_sd->sem_perm),&(target_sd->sem_perm));
     host_sd->sem_nsems = tswapl(target_sd->sem_nsems);
     host_sd->sem_otime = tswapl(target_sd->sem_otime);
     host_sd->sem_ctime = tswapl(target_sd->sem_ctime);
-    unlock_user_struct(target_sd, target_addr, 0);
 }
 
-static inline void host_to_target_semid_ds(target_ulong target_addr,
+static inline void copy_to_user_semid_ds(struct target_semid_ds *target_sd,
                                            struct semid_ds *host_sd)
 {
-    struct target_semid_ds *target_sd;
-
-    lock_user_struct(target_sd, target_addr, 0);
-    host_to_target_ipc_perm(target_addr,&(host_sd->sem_perm));
+    copy_to_user_ipc_perm(&(target_sd->sem_perm),&(host_sd->sem_perm));
     target_sd->sem_nsems = tswapl(host_sd->sem_nsems);
     target_sd->sem_otime = tswapl(host_sd->sem_otime);
     target_sd->sem_ctime = tswapl(host_sd->sem_ctime);
-    unlock_user_struct(target_sd, target_addr, 1);
 }
 
 union semun {
@@ -1328,109 +1334,122 @@
 union target_semun {
 	int val;
 	target_long buf;
-	unsigned short int *array;
+	target_long array;
 };
 
-static inline void target_to_host_semun(unsigned long cmd,
+/* static inline*/ long copy_from_user_semun(unsigned long cmd,
                                         union semun *host_su,
-                                        target_ulong target_addr,
+                                        union target_semun *target_su,
                                         struct semid_ds *ds)
 {
-    union target_semun *target_su;
-
     switch( cmd ) {
 	case IPC_STAT:
 	case IPC_SET:
-           lock_user_struct(target_su, target_addr, 1);
-	   target_to_host_semid_ds(ds,target_su->buf);
+	   if( copy_from_user(ds,target_su->buf,sizeof(struct semid_ds)) ) return -1;
+	   copy_from_user_semid_ds(ds,target_su->buf);
 	   host_su->buf = ds;
-           unlock_user_struct(target_su, target_addr, 0);
 	   break;
 	case GETVAL:
 	case SETVAL:
-           lock_user_struct(target_su, target_addr, 1);
-	   host_su->val = tswapl(target_su->val);
-           unlock_user_struct(target_su, target_addr, 0);
+	   if (get_user(host_su->val,&(target_su->val))) return -1;
 	   break;
 	case GETALL:
+	   host_su->array=(unsigned short *)ds;
+	   break;
 	case SETALL:
-           lock_user_struct(target_su, target_addr, 1);
+	   host_su->array=(unsigned short *)ds;
+	   if (copy_from_user(host_su->array,target_su->array,sizeof(unsigned short))) return -1;
+/*
+	   if (get_user(host_su->array[0],target_su->array)) return -1;
+	   if (lock_and_check_user_struct(&target_su,,sizeof(*target_su),1,pg_access) ) return -1;
 	   *host_su->array = tswap16(*target_su->array);
            unlock_user_struct(target_su, target_addr, 0);
+*/
 	   break;
 	default:
            gemu_log("semun operation not fully supported: %d\n", (int)cmd);
     }
+
+    return 0;
 }
 
-static inline void host_to_target_semun(unsigned long cmd,
-                                        target_ulong target_addr,
+/* static inline */ long copy_to_user_semun(unsigned long cmd,
+                                        union target_semun *target_su,
                                         union semun *host_su,
                                         struct semid_ds *ds)
 {
-    union target_semun *target_su;
-
     switch( cmd ) {
 	case IPC_STAT:
 	case IPC_SET:
-           lock_user_struct(target_su, target_addr, 0);
-	   host_to_target_semid_ds(target_su->buf,ds);
-           unlock_user_struct(target_su, target_addr, 1);
+	   if( copy_to_user(target_su->buf,ds,sizeof(struct semid_ds)) ) return -1;
+	   copy_to_user_semid_ds(target_su->buf,ds);
 	   break;
 	case GETVAL:
 	case SETVAL:
-           lock_user_struct(target_su, target_addr, 0);
-	   target_su->val = tswapl(host_su->val);
-           unlock_user_struct(target_su, target_addr, 1);
+	   if (put_user(host_su->val,&(target_su->val))) return -1;
 	   break;
 	case GETALL:
+	   if( copy_to_user(target_su->array,host_su->array,sizeof(unsigned short)) ) return -1;
+/*
+	   if (put_user(target_su->array[0],host_su->array)) return -1;
+	   *target_su->array = tswap16(*host_su->array);
+*/
+	   break;
 	case SETALL:
-           lock_user_struct(target_su, target_addr, 0);
+/*
+	   if (put_user(*host_su->array,target_su->array)) return -1;
+	   if (lock_and_check_user_struct(&target_su,target_addr,sizeof(*target_su),0,pg_access) ) return -1;
 	   *target_su->array = tswap16(*host_su->array);
            unlock_user_struct(target_su, target_addr, 1);
+*/
 	   break;
         default:
            gemu_log("semun operation not fully supported: %d\n", (int)cmd);
     }
+
+    return 0;
 }
 
-static inline long do_semctl(long first, long second, long third, long ptr)
+/* static inline */ long do_semctl(long first, long second, long third, long ptr)
 {
     union semun arg;
     struct semid_ds dsarg;
     int cmd = third&0xff;
     long ret = 0;
 
+    if( copy_from_user(&arg,ptr,sizeof(union semun)) ) return -EFAULT;
     switch( cmd ) {
 	case GETVAL:
-            target_to_host_semun(cmd,&arg,ptr,&dsarg);
+            if( copy_from_user_semun(cmd,&arg,ptr,&dsarg) ) return -EFAULT;
             ret = get_errno(semctl(first, second, cmd, arg));
-            host_to_target_semun(cmd,ptr,&arg,&dsarg);
+            if( copy_to_user_semun(cmd,ptr,&arg,&dsarg) ) return -EFAULT;
             break;
 	case SETVAL:
-            target_to_host_semun(cmd,&arg,ptr,&dsarg);
+            if( copy_from_user_semun(cmd,&arg,ptr,&dsarg) ) return -EFAULT;
             ret = get_errno(semctl(first, second, cmd, arg));
-            host_to_target_semun(cmd,ptr,&arg,&dsarg);
+            if( copy_to_user_semun(cmd,ptr,&arg,&dsarg) ) return -EFAULT;
             break;
 	case GETALL:
-            target_to_host_semun(cmd,&arg,ptr,&dsarg);
+            if( copy_from_user_semun(cmd,&arg,ptr,&dsarg) ) return -EFAULT;
             ret = get_errno(semctl(first, second, cmd, arg));
-            host_to_target_semun(cmd,ptr,&arg,&dsarg);
+    	    if (!is_error(ret)) {
+                if( copy_to_user_semun(cmd,ptr,&arg,&dsarg) ) return -EFAULT;
+	    }
             break;
 	case SETALL:
-            target_to_host_semun(cmd,&arg,ptr,&dsarg);
+            if( copy_from_user_semun(cmd,&arg,ptr,&dsarg) ) return -EFAULT;
             ret = get_errno(semctl(first, second, cmd, arg));
-            host_to_target_semun(cmd,ptr,&arg,&dsarg);
+            if( copy_to_user_semun(cmd,ptr,&arg,&dsarg) ) return -EFAULT;
             break;
 	case IPC_STAT:
-            target_to_host_semun(cmd,&arg,ptr,&dsarg);
+            if( copy_from_user_semun(cmd,&arg,ptr,&dsarg) ) return -EFAULT;
             ret = get_errno(semctl(first, second, cmd, arg));
-            host_to_target_semun(cmd,ptr,&arg,&dsarg);
+            if( copy_to_user_semun(cmd,ptr,&arg,&dsarg) ) return -EFAULT;
             break;
 	case IPC_SET:
-            target_to_host_semun(cmd,&arg,ptr,&dsarg);
+            if( copy_from_user_semun(cmd,&arg,ptr,&dsarg) ) return -EFAULT;
             ret = get_errno(semctl(first, second, cmd, arg));
-            host_to_target_semun(cmd,ptr,&arg,&dsarg);
+            if( copy_to_user_semun(cmd,ptr,&arg,&dsarg) ) return -EFAULT;
             break;
     default:
             ret = get_errno(semctl(first, second, cmd, arg));
@@ -1457,13 +1476,10 @@
   target_ulong __unused5;
 };
 
-static inline void target_to_host_msqid_ds(struct msqid_ds *host_md,
-                                          target_ulong target_addr)
+static inline long copy_from_user_msqid_ds(struct msqid_ds *host_md,
+                                          struct target_msqid_ds *target_md)
 {
-    struct target_msqid_ds *target_md;
-
-    lock_user_struct(target_md, target_addr, 1);
-    target_to_host_ipc_perm(&(host_md->msg_perm),target_addr);
+    copy_from_user_ipc_perm(&(host_md->msg_perm),&(target_md->msg_perm));
     host_md->msg_stime = tswapl(target_md->msg_stime);
     host_md->msg_rtime = tswapl(target_md->msg_rtime);
     host_md->msg_ctime = tswapl(target_md->msg_ctime);
@@ -1472,16 +1488,14 @@
     host_md->msg_qbytes = tswapl(target_md->msg_qbytes);
     host_md->msg_lspid = tswapl(target_md->msg_lspid);
     host_md->msg_lrpid = tswapl(target_md->msg_lrpid);
-    unlock_user_struct(target_md, target_addr, 0);
+
+    return 0;
 }
 
-static inline void host_to_target_msqid_ds(target_ulong target_addr,
-                                           struct msqid_ds *host_md)
+static inline long copy_to_usermsqid_ds(struct target_msqid_ds *target_md,
+                                          struct msqid_ds *host_md)
 {
-    struct target_msqid_ds *target_md;
-
-    lock_user_struct(target_md, target_addr, 0);
-    host_to_target_ipc_perm(target_addr,&(host_md->msg_perm));
+    copy_to_user_ipc_perm(&(target_md->msg_perm),&(host_md->msg_perm));
     target_md->msg_stime = tswapl(host_md->msg_stime);
     target_md->msg_rtime = tswapl(host_md->msg_rtime);
     target_md->msg_ctime = tswapl(host_md->msg_ctime);
@@ -1490,7 +1504,8 @@
     target_md->msg_qbytes = tswapl(host_md->msg_qbytes);
     target_md->msg_lspid = tswapl(host_md->msg_lspid);
     target_md->msg_lrpid = tswapl(host_md->msg_lrpid);
-    unlock_user_struct(target_md, target_addr, 1);
+
+    return 0;
 }
 
 static inline long do_msgctl(long first, long second, long ptr)
@@ -1500,10 +1515,17 @@
     long ret = 0;
     switch( cmd ) {
     case IPC_STAT:
+	if( copy_from_user(&dsarg,ptr,sizeof(struct msqid_ds)) ) return -EFAULT;
+        copy_from_user_msqid_ds(&dsarg,ptr);
+        ret = get_errno(msgctl(first, cmd, &dsarg));
+        if( copy_to_usermsqid_ds(ptr,&dsarg) ) return -EFAULT;
+	break;
     case IPC_SET:
-        target_to_host_msqid_ds(&dsarg,ptr);
+	if( copy_from_user(&dsarg,ptr,sizeof(struct msqid_ds)) ) return -EFAULT;
+        copy_from_user_msqid_ds(&dsarg,ptr);
         ret = get_errno(msgctl(first, cmd, &dsarg));
-        host_to_target_msqid_ds(ptr,&dsarg);
+        if( copy_to_usermsqid_ds(ptr,&dsarg) ) return -EFAULT;
+	break;
     default:
         ret = get_errno(msgctl(first, cmd, &dsarg));
     }
@@ -1515,19 +1537,20 @@
 	char	mtext[1];
 };
 
-static inline long do_msgsnd(long msqid, long msgp, long msgsz, long msgflg)
+/* static inline */ long do_msgsnd(long msqid, long msgp, long msgsz, long msgflg)
 {
-    struct target_msgbuf *target_mb;
-    struct msgbuf *host_mb;
+    struct target_msgbuf *target_mb = (struct msgbuf *)msgp;
+    struct msgbuf *host_mb = alloca(sizeof(long)+msgsz);
     long ret = 0;
 
-    lock_user_struct(target_mb,msgp,0);
+    if( copy_from_user(host_mb,target_mb,sizeof(long)+msgsz) ) return -EFAULT;
+/*
     host_mb = malloc(msgsz+sizeof(long));
-    host_mb->mtype = tswapl(target_mb->mtype);
     memcpy(host_mb->mtext,target_mb->mtext,msgsz);
-    ret = get_errno(msgsnd(msqid, host_mb, msgsz, msgflg));
     free(host_mb);
-    unlock_user_struct(target_mb, msgp, 0);
+*/
+    host_mb->mtype = tswapl(target_mb->mtype);
+    ret = get_errno(msgsnd(msqid, host_mb, msgsz, msgflg));
 
     return ret;
 }
@@ -1538,7 +1561,7 @@
     struct msgbuf *host_mb;
     long ret = 0;
 
-    lock_user_struct(target_mb, msgp, 0);
+    if( lock_and_check_user_struct(&target_mb,msgp,sizeof(long)+msgsz,0,PAGE_WRITE) ) return -EFAULT;
     host_mb = malloc(msgsz+sizeof(long));
     ret = get_errno(msgrcv(msqid, host_mb, msgsz, 1, msgflg));
     if (ret > 0)
@@ -2018,6 +2041,7 @@
 static int write_ldt(CPUX86State *env, 
                      target_ulong ptr, unsigned long bytecount, int oldmode)
 {
+    long ret = 0;
     struct target_modify_ldt_ldt_s ldt_info;
     struct target_modify_ldt_ldt_s *target_ldt_info;
     int seg_32bit, contents, read_exec_only, limit_in_pages;
@@ -2026,7 +2050,7 @@
 
     if (bytecount != sizeof(ldt_info))
         return -EINVAL;
-    lock_user_struct(target_ldt_info, ptr, 1);
+    if( lock_and_check_user_struct(&target_ldt_info,ptr,sizeof(struct target_modify_ldt_ldt_s),1,PAGE_READ) ) return -EFAULT;
     ldt_info.entry_number = tswap32(target_ldt_info->entry_number);
     ldt_info.base_addr = tswapl(target_ldt_info->base_addr);
     ldt_info.limit = tswap32(target_ldt_info->limit);
@@ -2225,7 +2249,7 @@
 
     switch(cmd) {
     case TARGET_F_GETLK:
-        lock_user_struct(target_fl, arg, 1);
+	if( lock_and_check_user_struct(&target_fl,arg,sizeof(struct target_flock),1,PAGE_READ) ) return -EFAULT;
         fl.l_type = tswap16(target_fl->l_type);
         fl.l_whence = tswap16(target_fl->l_whence);
         fl.l_start = tswapl(target_fl->l_start);
@@ -2234,6 +2258,7 @@
         unlock_user_struct(target_fl, arg, 0);
         ret = fcntl(fd, cmd, &fl);
         if (ret == 0) {
+	    if( lock_and_check_user_struct(&target_fl,arg,sizeof(struct target_flock),0,PAGE_WRITE) ) return -EFAULT;
             lock_user_struct(target_fl, arg, 0);
             target_fl->l_type = tswap16(fl.l_type);
             target_fl->l_whence = tswap16(fl.l_whence);
@@ -2246,7 +2271,7 @@
         
     case TARGET_F_SETLK:
     case TARGET_F_SETLKW:
-        lock_user_struct(target_fl, arg, 1);
+	if( lock_and_check_user_struct(&target_fl,arg,sizeof(struct target_flock),1,PAGE_READ) ) return -EFAULT;
         fl.l_type = tswap16(target_fl->l_type);
         fl.l_whence = tswap16(target_fl->l_whence);
         fl.l_start = tswapl(target_fl->l_start);
@@ -2257,7 +2282,7 @@
         break;
         
     case TARGET_F_GETLK64:
-        lock_user_struct(target_fl64, arg, 1);
+	if( lock_and_check_user_struct(&target_fl64,arg,sizeof(struct target_flock64),1,PAGE_READ) ) return -EFAULT;
         fl64.l_type = tswap16(target_fl64->l_type) >> 1;
         fl64.l_whence = tswap16(target_fl64->l_whence);
         fl64.l_start = tswapl(target_fl64->l_start);
@@ -2266,7 +2291,7 @@
         unlock_user_struct(target_fl64, arg, 0);
         ret = fcntl(fd, cmd >> 1, &fl64);
         if (ret == 0) {
-            lock_user_struct(target_fl64, arg, 0);
+	    if( lock_and_check_user_struct(&target_fl64,arg,sizeof(struct target_flock64),0,PAGE_WRITE) ) return -EFAULT;
             target_fl64->l_type = tswap16(fl64.l_type) >> 1;
             target_fl64->l_whence = tswap16(fl64.l_whence);
             target_fl64->l_start = tswapl(fl64.l_start);
@@ -2277,7 +2302,7 @@
 		break;
     case TARGET_F_SETLK64:
     case TARGET_F_SETLKW64:
-        lock_user_struct(target_fl64, arg, 1);
+	if( lock_and_check_user_struct(&target_fl64,arg,sizeof(struct target_flock64),1,PAGE_READ) ) return -EFAULT;
         fl64.l_type = tswap16(target_fl64->l_type) >> 1;
         fl64.l_whence = tswap16(target_fl64->l_whence);
         fl64.l_start = tswapl(target_fl64->l_start);
@@ -2419,32 +2444,30 @@
 }
 #endif
 
-static inline void target_to_host_timespec(struct timespec *host_ts,
-                                           target_ulong target_addr)
+static inline long copy_from_user_timespec(struct timespec *host_ts,
+                                           struct target_timespec *target_ts)
 {
-    struct target_timespec *target_ts;
-
-    lock_user_struct(target_ts, target_addr, 1);
+    if( copy_from_user(host_ts,target_ts,sizeof(*target_ts)) ) return -1;
     host_ts->tv_sec = tswapl(target_ts->tv_sec);
     host_ts->tv_nsec = tswapl(target_ts->tv_nsec);
-    unlock_user_struct(target_ts, target_addr, 0);
+
+    return 0;
 }
 
-static inline void host_to_target_timespec(target_ulong target_addr,
+static inline long copy_to_user_timespec(struct target_timespec *target_ts,
                                            struct timespec *host_ts)
 {
-    struct target_timespec *target_ts;
-
-    lock_user_struct(target_ts, target_addr, 0);
+    if( copy_to_user(target_ts, host_ts, sizeof(*target_ts)) ) return -1;
     target_ts->tv_sec = tswapl(host_ts->tv_sec);
     target_ts->tv_nsec = tswapl(host_ts->tv_nsec);
-    unlock_user_struct(target_ts, target_addr, 1);
+
+    return 0;
 }
 
 long do_syscall(void *cpu_env, int num, long arg1, long arg2, long arg3, 
                 long arg4, long arg5, long arg6)
 {
-    long ret;
+    long ret = 0;
     struct stat st;
     struct statfs stfs;
     void *p;
@@ -2671,7 +2694,7 @@
             struct utimbuf tbuf, *host_tbuf;
             struct target_utimbuf *target_tbuf;
             if (arg2) {
-                lock_user_struct(target_tbuf, arg2, 1);
+		if( lock_and_check_user_struct(&target_tbuf,arg2,sizeof(struct target_utimbuf),1,PAGE_READ) ) return -EFAULT;
                 tbuf.actime = tswapl(target_tbuf->actime);
                 tbuf.modtime = tswapl(target_tbuf->modtime);
                 unlock_user_struct(target_tbuf, arg2, 0);
@@ -2689,9 +2712,11 @@
         {
             struct timeval *tvp, tv[2];
             if (arg2) {
-                target_to_host_timeval(&tv[0], arg2);
-                target_to_host_timeval(&tv[1],
-                    arg2 + sizeof (struct target_timeval));
+                if( copy_from_user_timeval(&tv[0], arg2) )
+			return -EFAULT;
+                if( copy_from_user_timeval(&tv[1],
+		    arg2 + sizeof (struct target_timeval)) )
+			return -EFAULT;
                 tvp = tv;
             } else {
                 tvp = NULL;
@@ -2860,7 +2885,7 @@
             struct target_old_sigaction *old_act;
             struct target_sigaction act, oact, *pact;
             if (arg2) {
-                lock_user_struct(old_act, arg2, 1);
+		if( lock_and_check_user_struct(&old_act,arg2,sizeof(*old_act),1,PAGE_READ) ) return -EFAULT;
                 act._sa_handler = old_act->_sa_handler;
                 target_siginitset(&act.sa_mask, old_act->sa_mask);
                 act.sa_flags = old_act->sa_flags;
@@ -2872,7 +2897,7 @@
             }
             ret = get_errno(do_sigaction(arg1, pact, &oact));
             if (!is_error(ret) && arg3) {
-                lock_user_struct(old_act, arg3, 0);
+		if( lock_and_check_user_struct(&old_act,arg3,sizeof(*old_act),0,PAGE_WRITE) ) return -EFAULT;
                 old_act->_sa_handler = oact._sa_handler;
                 old_act->sa_mask = oact.sa_mask.sig[0];
                 old_act->sa_flags = oact.sa_flags;
@@ -2883,7 +2908,7 @@
 	    struct target_sigaction act, oact, *pact, *old_act;
 
 	    if (arg2) {
-		lock_user_struct(old_act, arg2, 1);
+		if( lock_and_check_user_struct(&old_act,arg2,sizeof(*old_act),1,PAGE_READ) ) return -EFAULT;
 		act._sa_handler = old_act->_sa_handler;
 		target_siginitset(&act.sa_mask, old_act->sa_mask.sig[0]);
 		act.sa_flags = old_act->sa_flags;
@@ -2896,7 +2921,7 @@
 	    ret = get_errno(do_sigaction(arg1, pact, &oact));
 
 	    if (!is_error(ret) && arg3) {
-		lock_user_struct(old_act, arg3, 0);
+		if( lock_and_check_user_struct(&old_act,arg3,sizeof(*old_act),0,PAGE_WRITE) ) return -EFAULT;
 		old_act->_sa_handler = oact._sa_handler;
 		old_act->sa_flags = oact.sa_flags;
 		old_act->sa_mask.sig[0] = oact.sa_mask.sig[0];
@@ -2914,12 +2939,14 @@
             struct target_sigaction *act;
             struct target_sigaction *oact;
 
-            if (arg2)
-                lock_user_struct(act, arg2, 1);
+            if (arg2) {
+		if( lock_and_check_user_struct(&act,arg2,sizeof(*act),1,PAGE_READ) ) return -EFAULT;
+                }
             else
                 act = NULL;
-            if (arg3)
-                lock_user_struct(oact, arg3, 0);
+            if (arg3) {
+		if( lock_and_check_user_struct(&oact,arg2,sizeof(*oact),0,PAGE_WRITE) ) return -EFAULT;
+                }
             else
                 oact = NULL;
             ret = get_errno(do_sigaction(arg1, act, oact));
@@ -3046,7 +3073,7 @@
             sigset_t set;
             ret = get_errno(sigpending(&set));
             if (!is_error(ret)) {
-                p = lock_user(arg1, sizeof(target_sigset_t), 0);
+		if( lock_and_check_user_struct(&p,arg1,sizeof(target_sigset_t),0,PAGE_WRITE) ) return -EFAULT;
                 host_to_target_sigset(p, &set);
                 unlock_user(p, arg1, sizeof(target_sigset_t));
             }
@@ -3056,7 +3083,7 @@
     case TARGET_NR_sigsuspend:
         {
             sigset_t set;
-            p = lock_user(arg1, sizeof(target_sigset_t), 1);
+	    if( lock_and_check_user_struct(&p,arg1,sizeof(target_sigset_t),1,PAGE_READ) ) return -EFAULT;
             target_to_host_old_sigset(&set, p);
             unlock_user(p, arg1, 0);
             ret = get_errno(sigsuspend(&set));
@@ -3066,7 +3093,7 @@
     case TARGET_NR_rt_sigsuspend:
         {
             sigset_t set;
-            p = lock_user(arg1, sizeof(target_sigset_t), 1);
+	    if( lock_and_check_user_struct(&p,arg1,sizeof(target_sigset_t),1,PAGE_READ) ) return -EFAULT;
             target_to_host_sigset(&set, p);
             unlock_user(p, arg1, 0);
             ret = get_errno(sigsuspend(&set));
@@ -3078,18 +3105,18 @@
             struct timespec uts, *puts;
             siginfo_t uinfo;
             
-            p = lock_user(arg1, sizeof(target_sigset_t), 1);
+	    if( lock_and_check_user_struct(&p,arg1,sizeof(target_sigset_t),1,PAGE_READ) ) return -EFAULT;
             target_to_host_sigset(&set, p);
             unlock_user(p, arg1, 0);
             if (arg3) {
                 puts = &uts;
-                target_to_host_timespec(puts, arg3);
+                if( copy_from_user_timespec(puts, arg3) ) return -EFAULT;
             } else {
                 puts = NULL;
             }
             ret = get_errno(sigtimedwait(&set, &uinfo, puts));
             if (!is_error(ret) && arg2) {
-                p = lock_user(arg2, sizeof(target_sigset_t), 0);
+		if( lock_and_check_user_struct(&p,arg2,sizeof(target_sigset_t),0,PAGE_WRITE) ) return -EFAULT;
                 host_to_target_siginfo(p, &uinfo);
                 unlock_user(p, arg2, sizeof(target_sigset_t));
             }
@@ -3098,7 +3125,7 @@
     case TARGET_NR_rt_sigqueueinfo:
         {
             siginfo_t uinfo;
-            p = lock_user(arg3, sizeof(target_sigset_t), 1);
+	    if( lock_and_check_user_struct(&p,arg3,sizeof(target_sigset_t),1,PAGE_READ) ) return -EFAULT;
             target_to_host_siginfo(&uinfo, p);
             unlock_user(p, arg1, 0);
             ret = get_errno(sys_rt_sigqueueinfo(arg1, arg2, &uinfo));
@@ -3153,7 +3180,7 @@
             struct rusage rusage;
             ret = get_errno(getrusage(arg1, &rusage));
             if (!is_error(ret)) {
-                host_to_target_rusage(arg2, &rusage);
+                if( copy_to_user_rusage(arg2, &rusage) ) return -EFAULT;
             }
         }
         break;
@@ -3162,14 +3189,14 @@
             struct timeval tv;
             ret = get_errno(gettimeofday(&tv, NULL));
             if (!is_error(ret)) {
-                host_to_target_timeval(arg1, &tv);
+                if( copy_to_user_timeval(arg1, &tv) ) return -EFAULT;
             }
         }
         break;
     case TARGET_NR_settimeofday:
         {
             struct timeval tv;
-            target_to_host_timeval(&tv, arg1);
+            if( copy_from_user_timeval(&tv, arg1) ) return -EFAULT;
             ret = get_errno(settimeofday(&tv, NULL));
         }
         break;
@@ -3335,20 +3362,19 @@
         unlock_user(p, arg1, 0);
     convert_statfs:
         if (!is_error(ret)) {
-            struct target_statfs *target_stfs;
+            struct target_statfs *target_stfs = (struct target_statfs *)arg2;
             
-            lock_user_struct(target_stfs, arg2, 0);
-            /* ??? put_user is probably wrong.  */
-            put_user(stfs.f_type, &target_stfs->f_type);
-            put_user(stfs.f_bsize, &target_stfs->f_bsize);
-            put_user(stfs.f_blocks, &target_stfs->f_blocks);
-            put_user(stfs.f_bfree, &target_stfs->f_bfree);
-            put_user(stfs.f_bavail, &target_stfs->f_bavail);
-            put_user(stfs.f_files, &target_stfs->f_files);
-            put_user(stfs.f_ffree, &target_stfs->f_ffree);
-            put_user(stfs.f_fsid.__val[0], &target_stfs->f_fsid);
-            put_user(stfs.f_namelen, &target_stfs->f_namelen);
-            unlock_user_struct(target_stfs, arg2, 1);
+	    if( !access_ok(VERIFY_WRITE,target_stfs,sizeof(struct target_statfs)) ) return -EFAULT;
+            /* use __put_user since we just validated the entire bufffer.  */
+            __put_user(stfs.f_type, &target_stfs->f_type);
+            __put_user(stfs.f_bsize, &target_stfs->f_bsize);
+            __put_user(stfs.f_blocks, &target_stfs->f_blocks);
+            __put_user(stfs.f_bfree, &target_stfs->f_bfree);
+            __put_user(stfs.f_bavail, &target_stfs->f_bavail);
+            __put_user(stfs.f_files, &target_stfs->f_files);
+            __put_user(stfs.f_ffree, &target_stfs->f_ffree);
+            __put_user(stfs.f_fsid.__val[0], &target_stfs->f_fsid);
+            __put_user(stfs.f_namelen, &target_stfs->f_namelen);
         }
         break;
     case TARGET_NR_fstatfs:
@@ -3361,20 +3387,19 @@
         unlock_user(p, arg1, 0);
     convert_statfs64:
         if (!is_error(ret)) {
-            struct target_statfs64 *target_stfs;
+            struct target_statfs64 *target_stfs = (struct target_statfs64 *)arg3;
             
-            lock_user_struct(target_stfs, arg3, 0);
-            /* ??? put_user is probably wrong.  */
-            put_user(stfs.f_type, &target_stfs->f_type);
-            put_user(stfs.f_bsize, &target_stfs->f_bsize);
-            put_user(stfs.f_blocks, &target_stfs->f_blocks);
-            put_user(stfs.f_bfree, &target_stfs->f_bfree);
-            put_user(stfs.f_bavail, &target_stfs->f_bavail);
-            put_user(stfs.f_files, &target_stfs->f_files);
-            put_user(stfs.f_ffree, &target_stfs->f_ffree);
-            put_user(stfs.f_fsid.__val[0], &target_stfs->f_fsid);
-            put_user(stfs.f_namelen, &target_stfs->f_namelen);
-            unlock_user_struct(target_stfs, arg3, 0);
+	    if( !access_ok(VERIFY_WRITE,target_stfs,sizeof(struct target_statfs64)) ) return -EFAULT;
+            /* use __put_user since we just validated the entire bufffer.  */
+            __put_user(stfs.f_type, &target_stfs->f_type);
+            __put_user(stfs.f_bsize, &target_stfs->f_bsize);
+            __put_user(stfs.f_blocks, &target_stfs->f_blocks);
+            __put_user(stfs.f_bfree, &target_stfs->f_bfree);
+            __put_user(stfs.f_bavail, &target_stfs->f_bavail);
+            __put_user(stfs.f_files, &target_stfs->f_files);
+            __put_user(stfs.f_ffree, &target_stfs->f_ffree);
+            __put_user(stfs.f_fsid.__val[0], &target_stfs->f_fsid);
+            __put_user(stfs.f_namelen, &target_stfs->f_namelen);
         }
         break;
     case TARGET_NR_fstatfs64:
@@ -3488,19 +3513,20 @@
 
             if (arg2) {
                 pvalue = &value;
-                target_to_host_timeval(&pvalue->it_interval, 
-                                       arg2);
-                target_to_host_timeval(&pvalue->it_value, 
-                                       arg2 + sizeof(struct target_timeval));
+                if( copy_from_user_timeval(&pvalue->it_interval, arg2) )
+			return -EFAULT;
+                if( copy_from_user_timeval(&pvalue->it_value,
+                                       arg2 + sizeof(struct target_timeval)) )
+			return -EFAULT;
             } else {
                 pvalue = NULL;
             }
             ret = get_errno(setitimer(arg1, pvalue, &ovalue));
             if (!is_error(ret) && arg3) {
-                host_to_target_timeval(arg3,
-                                       &ovalue.it_interval);
-                host_to_target_timeval(arg3 + sizeof(struct target_timeval),
-                                       &ovalue.it_value);
+                if( copy_to_user_timeval(arg3,
+                                       &ovalue.it_interval) ) return -EFAULT;
+                if( copy_to_user_timeval(arg3 + sizeof(struct target_timeval),
+                                       &ovalue.it_value) ) return -EFAULT;
             }
         }
         break;
@@ -3510,10 +3536,10 @@
             
             ret = get_errno(getitimer(arg1, &value));
             if (!is_error(ret) && arg2) {
-                host_to_target_timeval(arg2,
-                                       &value.it_interval);
-                host_to_target_timeval(arg2 + sizeof(struct target_timeval),
-                                       &value.it_value);
+                if( copy_to_user_timeval(arg2,
+                                       &value.it_interval) ) return -EFAULT;
+                if( copy_to_user_timeval(arg2 + sizeof(struct target_timeval),
+                                       &value.it_value) ) return -EFAULT;
             }
         }
         break;
@@ -3534,7 +3560,7 @@
             if (!is_error(ret)) {
                 struct target_stat *target_st;
 
-                lock_user_struct(target_st, arg2, 0);
+		if( lock_and_check_user_struct(&target_st,arg2,sizeof(*target_st),9,PAGE_WRITE) ) return -EFAULT;
 #if defined(TARGET_MIPS)
                 target_st->st_dev = tswap32(st.st_dev);
 #else
@@ -3603,7 +3629,8 @@
                 if (status_ptr)
                     tputl(status_ptr, status);
                 if (target_rusage) {
-                    host_to_target_rusage(target_rusage, &rusage);
+                    if( !copy_to_user_rusage(target_rusage, &rusage) )
+			return -EFAULT;
                 }
             }
         }
@@ -3965,17 +3992,17 @@
             struct timespec ts;
             ret = get_errno(sched_rr_get_interval(arg1, &ts));
             if (!is_error(ret)) {
-                host_to_target_timespec(arg2, &ts);
+                if( copy_to_user_timespec(arg2, &ts) ) return -EFAULT;
             }
         }
         break;
     case TARGET_NR_nanosleep:
         {
             struct timespec req, rem;
-            target_to_host_timespec(&req, arg1);
+            if( copy_from_user_timespec(&req, arg1) ) return -EFAULT;
             ret = get_errno(nanosleep(&req, &rem));
             if (is_error(ret) && arg2) {
-                host_to_target_timespec(arg2, &rem);
+                if( copy_to_user_timespec(arg2, &rem) ) return -EFAULT;
             }
         }
         break;
@@ -4072,6 +4099,7 @@
 #ifdef TARGET_NR_stat64
     case TARGET_NR_stat64:
         p = lock_user_string(arg1);
+        if( ret=page_check_range(p,1,PAGE_READ) ) return ret;
         ret = get_errno(stat(path(p), &st));
         unlock_user(p, arg1, 0);
         goto do_stat64;
@@ -4079,6 +4107,7 @@
 #ifdef TARGET_NR_lstat64
     case TARGET_NR_lstat64:
         p = lock_user_string(arg1);
+        if( ret=page_check_range(p,1,PAGE_READ) ) return ret;
         ret = get_errno(lstat(path(p), &st));
         unlock_user(p, arg1, 0);
         goto do_stat64;
@@ -4091,53 +4120,51 @@
             if (!is_error(ret)) {
 #ifdef TARGET_ARM
                 if (((CPUARMState *)cpu_env)->eabi) {
-                    struct target_eabi_stat64 *target_st;
-                    lock_user_struct(target_st, arg2, 1);
+                    struct target_eabi_stat64 *target_st = (struct target_eabi_stat64 *)arg2;
+		    if( !access_ok(VERIFY_WRITE,target_st,sizeof(struct target_eabi_stat64)) ) return -EFAULT;
                     memset(target_st, 0, sizeof(struct target_eabi_stat64));
-                    /* put_user is probably wrong.  */
-                    put_user(st.st_dev, &target_st->st_dev);
-                    put_user(st.st_ino, &target_st->st_ino);
+                    /* use __put_user() since we just checked that the buffer is valid  */
+                    __put_user(st.st_dev, &target_st->st_dev);
+                    __put_user(st.st_ino, &target_st->st_ino);
 #ifdef TARGET_STAT64_HAS_BROKEN_ST_INO
-                    put_user(st.st_ino, &target_st->__st_ino);
+                    __put_user(st.st_ino, &target_st->__st_ino);
 #endif
-                    put_user(st.st_mode, &target_st->st_mode);
-                    put_user(st.st_nlink, &target_st->st_nlink);
-                    put_user(st.st_uid, &target_st->st_uid);
-                    put_user(st.st_gid, &target_st->st_gid);
-                    put_user(st.st_rdev, &target_st->st_rdev);
+                    __put_user(st.st_mode, &target_st->st_mode);
+                    __put_user(st.st_nlink, &target_st->st_nlink);
+                    __put_user(st.st_uid, &target_st->st_uid);
+                    __put_user(st.st_gid, &target_st->st_gid);
+                    __put_user(st.st_rdev, &target_st->st_rdev);
                     /* XXX: better use of kernel struct */
-                    put_user(st.st_size, &target_st->st_size);
-                    put_user(st.st_blksize, &target_st->st_blksize);
-                    put_user(st.st_blocks, &target_st->st_blocks);
-                    put_user(st.st_atime, &target_st->target_st_atime);
-                    put_user(st.st_mtime, &target_st->target_st_mtime);
-                    put_user(st.st_ctime, &target_st->target_st_ctime);
-                    unlock_user_struct(target_st, arg2, 0);
+                    __put_user(st.st_size, &target_st->st_size);
+                    __put_user(st.st_blksize, &target_st->st_blksize);
+                    __put_user(st.st_blocks, &target_st->st_blocks);
+                    __put_user(st.st_atime, &target_st->target_st_atime);
+                    __put_user(st.st_mtime, &target_st->target_st_mtime);
+                    __put_user(st.st_ctime, &target_st->target_st_ctime);
                 } else
 #endif
                 {
-                    struct target_stat64 *target_st;
-                    lock_user_struct(target_st, arg2, 1);
+                    struct target_stat64 *target_st = (struct target_stat64 *)arg2;
+		    if( !access_ok(VERIFY_WRITE,target_st,sizeof(struct target_stat64)) ) return -EFAULT;
                     memset(target_st, 0, sizeof(struct target_stat64));
-                    /* ??? put_user is probably wrong.  */
-                    put_user(st.st_dev, &target_st->st_dev);
-                    put_user(st.st_ino, &target_st->st_ino);
+                    /* use __put_user() since we just checked that the buffer is valid  */
+                    __put_user(st.st_dev, &target_st->st_dev);
+                    __put_user(st.st_ino, &target_st->st_ino);
 #ifdef TARGET_STAT64_HAS_BROKEN_ST_INO
-                    put_user(st.st_ino, &target_st->__st_ino);
+                    __put_user(st.st_ino, &target_st->__st_ino);
 #endif
-                    put_user(st.st_mode, &target_st->st_mode);
-                    put_user(st.st_nlink, &target_st->st_nlink);
-                    put_user(st.st_uid, &target_st->st_uid);
-                    put_user(st.st_gid, &target_st->st_gid);
-                    put_user(st.st_rdev, &target_st->st_rdev);
+                    __put_user(st.st_mode, &target_st->st_mode);
+                    __put_user(st.st_nlink, &target_st->st_nlink);
+                    __put_user(st.st_uid, &target_st->st_uid);
+                    __put_user(st.st_gid, &target_st->st_gid);
+                    __put_user(st.st_rdev, &target_st->st_rdev);
                     /* XXX: better use of kernel struct */
-                    put_user(st.st_size, &target_st->st_size);
-                    put_user(st.st_blksize, &target_st->st_blksize);
-                    put_user(st.st_blocks, &target_st->st_blocks);
-                    put_user(st.st_atime, &target_st->target_st_atime);
-                    put_user(st.st_mtime, &target_st->target_st_mtime);
-                    put_user(st.st_ctime, &target_st->target_st_ctime);
-                    unlock_user_struct(target_st, arg2, 0);
+                    __put_user(st.st_size, &target_st->st_size);
+                    __put_user(st.st_blksize, &target_st->st_blksize);
+                    __put_user(st.st_blocks, &target_st->st_blocks);
+                    __put_user(st.st_atime, &target_st->target_st_atime);
+                    __put_user(st.st_mtime, &target_st->target_st_mtime);
+                    __put_user(st.st_ctime, &target_st->target_st_ctime);
                 }
             }
         }
@@ -4583,7 +4610,7 @@
         struct timespec ts;
         ret = get_errno(clock_gettime(arg1, &ts));
         if (!is_error(ret)) {
-            host_to_target_timespec(arg2, &ts);
+            if( copy_to_user_timespec(arg2, &ts)) return -EFAULT;
         }
         break;
     }
@@ -4594,7 +4621,7 @@
         struct timespec ts;
         ret = get_errno(clock_getres(arg1, &ts));
         if (!is_error(ret)) {
-            host_to_target_timespec(arg2, &ts);
+            if( copy_to_user_timespec(arg2, &ts) ) return -EFAULT;
         }
         break;
     }
Index: qemu/linux-user/qemu.h
===================================================================
--- qemu.orig/linux-user/qemu.h	2007-07-13 07:10:58.000000000 -0400
+++ qemu/linux-user/qemu.h	2007-07-13 07:11:12.000000000 -0400
@@ -171,7 +171,8 @@
 #define VERIFY_READ 0
 #define VERIFY_WRITE 1
 
-#define access_ok(type,addr,size) (1)
+#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 addresses.  */
 #define __put_user(x,ptr)\
@@ -238,6 +239,26 @@
     __ret;\
 })
 
+#define copy_from_user(hptr, gptr, len) \
+({ \
+    int ret=0;\
+    if( access_ok(VERIFY_READ, gptr, (len) ) )\
+        memcpy(hptr,gptr,(len)); \
+    else \
+        ret=1;\
+    ret; \
+})
+
+#define copy_to_user(gptr, hptr, len) \
+({ \
+    int ret=0;\
+    if( access_ok(VERIFY_WRITE, gptr, (len)) )\
+        memcpy(gptr,hptr,(len)); \
+    else \
+        ret=1;\
+    ret; \
+})
+
 /* 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
@@ -295,6 +316,16 @@
 #define unlock_user_struct(host_ptr, guest_addr, copy) \
     unlock_user(host_ptr, guest_addr, (copy) ? sizeof(*host_ptr) : 0)
 
+/* This is being phased ot in favor of the copy*user() interfaces */
+static inline long lock_and_check_user_struct(void *host_addr, target_ulong guest_addr, long len, int copy, int pg_access)
+{
+    long *haddr, ret = 0;
+    haddr = (long *)host_addr;
+    if( (ret=page_check_range(guest_addr,len,pg_access)) != 0 ) return ret;
+    *haddr = (long)lock_user(guest_addr, len, copy);
+    return 0;
+}
+
 #define tget8(addr) ldub(addr)
 #define tput8(addr, val) stb(addr, val)
 #define tget16(addr) lduw(addr)

  reply	other threads:[~2007-07-13 17:57 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-07-06 15:15 [Qemu-devel] [PATCH] linux-user EFAULT implementation Stuart Anderson
2007-07-06 16:08 ` Fabrice Bellard
2007-07-06 16:55   ` Stuart Anderson
2007-07-06 18:45     ` Stuart Anderson
2007-07-09 11:41       ` Stuart Anderson
2007-07-09 12:02         ` Stuart Anderson
2007-07-09 21:15         ` Fabrice Bellard
2007-07-10  2:22           ` Stuart Anderson
2007-07-10 12:38             ` Paul Brook
2007-07-10 20:45               ` Fabrice Bellard
2007-07-10 21:09                 ` Stuart Anderson
2007-07-13 17:55                   ` Stuart Anderson [this message]
2007-07-20 20:16                     ` Stuart Anderson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Pine.LNX.4.64.0707131352380.32663@trantor.stuart.netsweng.com \
    --to=anderson@netsweng.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).