qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] additional EFAULT patches
@ 2007-11-20 19:00 Thayne Harbaugh
  2007-11-20 19:08 ` [Qemu-devel] Re: [PATCH] 06_efault.3.patch - copy_from_user_fdset() Thayne Harbaugh
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Thayne Harbaugh @ 2007-11-20 19:00 UTC (permalink / raw)
  To: qemu-devel

These are some additional EFAULT patches.  They improve the code
consistency, check return values of copy_{to,from}_user() operations and
provide minor fixes.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [Qemu-devel] Re: [PATCH] 06_efault.3.patch - copy_from_user_fdset()
  2007-11-20 19:00 [Qemu-devel] [PATCH] additional EFAULT patches Thayne Harbaugh
@ 2007-11-20 19:08 ` Thayne Harbaugh
  2007-11-20 19:30   ` Thayne Harbaugh
  2007-11-21  4:09 ` [Qemu-devel] Re: [PATCH] 06_efault.4.patch - timeval Thayne Harbaugh
  2007-11-21  4:11 ` [Qemu-devel] Re: [PATCH] 06_efault.5.timespec.patch Thayne Harbaugh
  2 siblings, 1 reply; 5+ messages in thread
From: Thayne Harbaugh @ 2007-11-20 19:08 UTC (permalink / raw)
  To: qemu-devel

[-- Attachment #1: Type: text/plain, Size: 325 bytes --]

This updates target_to_host_fds() to match the copy_from_user() code.
It drops some unused variables, checks and handles return values for
copy_from_user_fdset() and corrects an error where the "n" value was
incorrectly multiplied with abi_long instead of used as one greater than
the number of descriptors (which are bits).

[-- Attachment #2: 06_efault.3.patch --]
[-- Type: text/x-patch, Size: 6467 bytes --]

Index: qemu/linux-user/syscall.c
===================================================================
--- qemu.orig/linux-user/syscall.c	2007-11-17 09:00:14.000000000 -0700
+++ qemu/linux-user/syscall.c	2007-11-17 09:09:08.000000000 -0700
@@ -443,50 +443,66 @@
     }
 }
 
-static inline fd_set *target_to_host_fds(fd_set *fds,
-                                         abi_long *target_fds, int n)
+static inline abi_long copy_from_user_fdset(fd_set *fds,
+                                            abi_ulong target_fds_addr,
+                                            int n)
 {
-#if !defined(BSWAP_NEEDED) && !defined(WORDS_BIGENDIAN)
-    return (fd_set *)target_fds;
-#else
-    int i, b;
-    if (target_fds) {
-        FD_ZERO(fds);
-        for(i = 0;i < n; i++) {
-            b = (tswapl(target_fds[i / TARGET_ABI_BITS]) >>
-                 (i & (TARGET_ABI_BITS - 1))) & 1;
-            if (b)
-                FD_SET(i, fds);
+    int i, nw, j, k;
+    abi_ulong b, *target_fds;
+
+    nw = (n + TARGET_ABI_BITS - 1) / TARGET_ABI_BITS;
+    if (!(target_fds = lock_user(VERIFY_READ,
+                                 target_fds_addr,
+                                 sizeof(abi_ulong) * nw,
+                                 1)))
+        return -TARGET_EFAULT;
+
+    FD_ZERO(fds);
+    k = 0;
+    for (i = 0; i < nw; i++) {
+        /* grab the abi_ulong */
+        __get_user(b, &target_fds[i]);
+        for (j = 0; j < TARGET_ABI_BITS; j++) {
+            /* check the bit inside the abi_ulong */
+            if ((b >> j) & 1)
+                FD_SET(k, fds);
+            k++;
         }
-        return fds;
-    } else {
-        return NULL;
     }
-#endif
+
+    unlock_user(target_fds, target_fds_addr, 0);
+
+    return 0;
 }
 
-static inline void host_to_target_fds(abi_long *target_fds,
-                                      fd_set *fds, int n)
+static inline abi_long copy_to_user_fdset(abi_ulong target_fds_addr,
+                                          fd_set *fds,
+                                          int n)
 {
-#if !defined(BSWAP_NEEDED) && !defined(WORDS_BIGENDIAN)
-    /* nothing to do */
-#else
     int i, nw, j, k;
     abi_long v;
+    abi_ulong *target_fds;
 
-    if (target_fds) {
-        nw = (n + TARGET_ABI_BITS - 1) / TARGET_ABI_BITS;
-        k = 0;
-        for(i = 0;i < nw; i++) {
-            v = 0;
-            for(j = 0; j < TARGET_ABI_BITS; j++) {
-                v |= ((FD_ISSET(k, fds) != 0) << j);
-                k++;
-            }
-            target_fds[i] = tswapl(v);
+    nw = (n + TARGET_ABI_BITS - 1) / TARGET_ABI_BITS;
+    if (!(target_fds = lock_user(VERIFY_WRITE,
+                                 target_fds_addr,
+                                 sizeof(abi_ulong) * nw,
+                                 0)))
+        return -TARGET_EFAULT;
+
+    k = 0;
+    for (i = 0; i < nw; i++) {
+        v = 0;
+        for (j = 0; j < TARGET_ABI_BITS; j++) {
+            v |= ((FD_ISSET(k, fds) != 0) << j);
+            k++;
         }
+        __put_user(v, &target_fds[i]);
     }
-#endif
+
+    unlock_user(target_fds, target_fds_addr, sizeof(abi_ulong) * nw);
+
+    return 0;
 }
 
 #if defined(__alpha__)
@@ -567,74 +583,57 @@
 
 /* do_select() must return target values and target errnos. */
 static abi_long do_select(int n,
-                          abi_ulong rfd_p, abi_ulong wfd_p,
-                          abi_ulong efd_p, abi_ulong target_tv)
+                          abi_ulong rfd_addr, abi_ulong wfd_addr,
+                          abi_ulong efd_addr, abi_ulong target_tv_addr)
 {
     fd_set rfds, wfds, efds;
     fd_set *rfds_ptr, *wfds_ptr, *efds_ptr;
-    abi_long *target_rfds, *target_wfds, *target_efds;
     struct timeval tv, *tv_ptr;
     abi_long ret;
-    int ok;
 
-    if (rfd_p) {
-        target_rfds = lock_user(VERIFY_WRITE, rfd_p, sizeof(abi_long) * n, 1);
-        if (!target_rfds) {
-            ret = -TARGET_EFAULT;
-            goto end;
-        }
-        rfds_ptr = target_to_host_fds(&rfds, target_rfds, n);
+    if (rfd_addr) {
+        if (copy_from_user_fdset(&rfds, rfd_addr, n))
+            return -TARGET_EFAULT;
+        rfds_ptr = &rfds;
     } else {
-        target_rfds = NULL;
         rfds_ptr = NULL;
     }
-    if (wfd_p) {
-        target_wfds = lock_user(VERIFY_WRITE, wfd_p, sizeof(abi_long) * n, 1);
-        if (!target_wfds) {
-            ret = -TARGET_EFAULT;
-            goto end;
-        }
-        wfds_ptr = target_to_host_fds(&wfds, target_wfds, n);
+    if (wfd_addr) {
+        if (copy_from_user_fdset(&wfds, wfd_addr, n))
+            return -TARGET_EFAULT;
+        wfds_ptr = &wfds;
     } else {
-        target_wfds = NULL;
         wfds_ptr = NULL;
     }
-    if (efd_p) {
-        target_efds = lock_user(VERIFY_WRITE, efd_p, sizeof(abi_long) * n, 1);
-        if (!target_efds) {
-            ret = -TARGET_EFAULT;
-            goto end;
-        }
-        efds_ptr = target_to_host_fds(&efds, target_efds, n);
+    if (efd_addr) {
+        if (copy_from_user_fdset(&efds, efd_addr, n))
+            return -TARGET_EFAULT;
+        efds_ptr = &efds;
     } else {
-        target_efds = NULL;
         efds_ptr = NULL;
     }
 
-    if (target_tv) {
-        target_to_host_timeval(&tv, target_tv);
+    if (target_tv_addr) {
+        target_to_host_timeval(&tv, target_tv_addr);
         tv_ptr = &tv;
     } else {
         tv_ptr = NULL;
     }
+
     ret = get_errno(select(n, rfds_ptr, wfds_ptr, efds_ptr, tv_ptr));
-    ok = !is_error(ret);
 
-    if (ok) {
-        host_to_target_fds(target_rfds, rfds_ptr, n);
-        host_to_target_fds(target_wfds, wfds_ptr, n);
-        host_to_target_fds(target_efds, efds_ptr, n);
+    if (!is_error(ret)) {
+        if (rfd_addr && copy_to_user_fdset(rfd_addr, &rfds, n))
+            return -TARGET_EFAULT;
+        if (wfd_addr && copy_to_user_fdset(wfd_addr, &wfds, n))
+            return -TARGET_EFAULT;
+        if (efd_addr && copy_to_user_fdset(efd_addr, &efds, n))
+            return -TARGET_EFAULT;
 
-        if (target_tv) {
-            host_to_target_timeval(target_tv, &tv);
-        }
+        if (target_tv_addr)
+            host_to_target_timeval(target_tv_addr, &tv);
     }
 
-end:
-    unlock_user(target_rfds, rfd_p, ok ? sizeof(abi_long) * n : 0);
-    unlock_user(target_wfds, wfd_p, ok ? sizeof(abi_long) * n : 0);
-    unlock_user(target_efds, efd_p, ok ? sizeof(abi_long) * n : 0);
-
     return ret;
 }
 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [Qemu-devel] Re: [PATCH] 06_efault.3.patch - copy_from_user_fdset()
  2007-11-20 19:08 ` [Qemu-devel] Re: [PATCH] 06_efault.3.patch - copy_from_user_fdset() Thayne Harbaugh
@ 2007-11-20 19:30   ` Thayne Harbaugh
  0 siblings, 0 replies; 5+ messages in thread
From: Thayne Harbaugh @ 2007-11-20 19:30 UTC (permalink / raw)
  To: qemu-devel

[-- Attachment #1: Type: text/plain, Size: 542 bytes --]


On Tue, 2007-11-20 at 12:08 -0700, Thayne Harbaugh wrote:
> This updates target_to_host_fds() to match the copy_from_user() code.
> It drops some unused variables, checks and handles return values for
> copy_from_user_fdset() and corrects an error where the "n" value was
> incorrectly multiplied with abi_long instead of used as one greater than
> the number of descriptors (which are bits).

The previous patch missed a minor detail.  This is an update to make the
"fd_set *fds" parameter of copy_to_user_fdset() into "const fd_set
*fds".

[-- Attachment #2: 06_efault.3.patch --]
[-- Type: text/x-patch, Size: 6473 bytes --]

Index: qemu/linux-user/syscall.c
===================================================================
--- qemu.orig/linux-user/syscall.c	2007-11-20 11:53:16.000000000 -0700
+++ qemu/linux-user/syscall.c	2007-11-20 12:52:33.000000000 -0700
@@ -443,50 +443,66 @@
     }
 }
 
-static inline fd_set *target_to_host_fds(fd_set *fds,
-                                         abi_long *target_fds, int n)
+static inline abi_long copy_from_user_fdset(fd_set *fds,
+                                            abi_ulong target_fds_addr,
+                                            int n)
 {
-#if !defined(BSWAP_NEEDED) && !defined(WORDS_BIGENDIAN)
-    return (fd_set *)target_fds;
-#else
-    int i, b;
-    if (target_fds) {
-        FD_ZERO(fds);
-        for(i = 0;i < n; i++) {
-            b = (tswapl(target_fds[i / TARGET_ABI_BITS]) >>
-                 (i & (TARGET_ABI_BITS - 1))) & 1;
-            if (b)
-                FD_SET(i, fds);
+    int i, nw, j, k;
+    abi_ulong b, *target_fds;
+
+    nw = (n + TARGET_ABI_BITS - 1) / TARGET_ABI_BITS;
+    if (!(target_fds = lock_user(VERIFY_READ,
+                                 target_fds_addr,
+                                 sizeof(abi_ulong) * nw,
+                                 1)))
+        return -TARGET_EFAULT;
+
+    FD_ZERO(fds);
+    k = 0;
+    for (i = 0; i < nw; i++) {
+        /* grab the abi_ulong */
+        __get_user(b, &target_fds[i]);
+        for (j = 0; j < TARGET_ABI_BITS; j++) {
+            /* check the bit inside the abi_ulong */
+            if ((b >> j) & 1)
+                FD_SET(k, fds);
+            k++;
         }
-        return fds;
-    } else {
-        return NULL;
     }
-#endif
+
+    unlock_user(target_fds, target_fds_addr, 0);
+
+    return 0;
 }
 
-static inline void host_to_target_fds(abi_long *target_fds,
-                                      fd_set *fds, int n)
+static inline abi_long copy_to_user_fdset(abi_ulong target_fds_addr,
+                                          const fd_set *fds,
+                                          int n)
 {
-#if !defined(BSWAP_NEEDED) && !defined(WORDS_BIGENDIAN)
-    /* nothing to do */
-#else
     int i, nw, j, k;
     abi_long v;
+    abi_ulong *target_fds;
 
-    if (target_fds) {
-        nw = (n + TARGET_ABI_BITS - 1) / TARGET_ABI_BITS;
-        k = 0;
-        for(i = 0;i < nw; i++) {
-            v = 0;
-            for(j = 0; j < TARGET_ABI_BITS; j++) {
-                v |= ((FD_ISSET(k, fds) != 0) << j);
-                k++;
-            }
-            target_fds[i] = tswapl(v);
+    nw = (n + TARGET_ABI_BITS - 1) / TARGET_ABI_BITS;
+    if (!(target_fds = lock_user(VERIFY_WRITE,
+                                 target_fds_addr,
+                                 sizeof(abi_ulong) * nw,
+                                 0)))
+        return -TARGET_EFAULT;
+
+    k = 0;
+    for (i = 0; i < nw; i++) {
+        v = 0;
+        for (j = 0; j < TARGET_ABI_BITS; j++) {
+            v |= ((FD_ISSET(k, fds) != 0) << j);
+            k++;
         }
+        __put_user(v, &target_fds[i]);
     }
-#endif
+
+    unlock_user(target_fds, target_fds_addr, sizeof(abi_ulong) * nw);
+
+    return 0;
 }
 
 #if defined(__alpha__)
@@ -567,74 +583,57 @@
 
 /* do_select() must return target values and target errnos. */
 static abi_long do_select(int n,
-                          abi_ulong rfd_p, abi_ulong wfd_p,
-                          abi_ulong efd_p, abi_ulong target_tv)
+                          abi_ulong rfd_addr, abi_ulong wfd_addr,
+                          abi_ulong efd_addr, abi_ulong target_tv_addr)
 {
     fd_set rfds, wfds, efds;
     fd_set *rfds_ptr, *wfds_ptr, *efds_ptr;
-    abi_long *target_rfds, *target_wfds, *target_efds;
     struct timeval tv, *tv_ptr;
     abi_long ret;
-    int ok;
 
-    if (rfd_p) {
-        target_rfds = lock_user(VERIFY_WRITE, rfd_p, sizeof(abi_long) * n, 1);
-        if (!target_rfds) {
-            ret = -TARGET_EFAULT;
-            goto end;
-        }
-        rfds_ptr = target_to_host_fds(&rfds, target_rfds, n);
+    if (rfd_addr) {
+        if (copy_from_user_fdset(&rfds, rfd_addr, n))
+            return -TARGET_EFAULT;
+        rfds_ptr = &rfds;
     } else {
-        target_rfds = NULL;
         rfds_ptr = NULL;
     }
-    if (wfd_p) {
-        target_wfds = lock_user(VERIFY_WRITE, wfd_p, sizeof(abi_long) * n, 1);
-        if (!target_wfds) {
-            ret = -TARGET_EFAULT;
-            goto end;
-        }
-        wfds_ptr = target_to_host_fds(&wfds, target_wfds, n);
+    if (wfd_addr) {
+        if (copy_from_user_fdset(&wfds, wfd_addr, n))
+            return -TARGET_EFAULT;
+        wfds_ptr = &wfds;
     } else {
-        target_wfds = NULL;
         wfds_ptr = NULL;
     }
-    if (efd_p) {
-        target_efds = lock_user(VERIFY_WRITE, efd_p, sizeof(abi_long) * n, 1);
-        if (!target_efds) {
-            ret = -TARGET_EFAULT;
-            goto end;
-        }
-        efds_ptr = target_to_host_fds(&efds, target_efds, n);
+    if (efd_addr) {
+        if (copy_from_user_fdset(&efds, efd_addr, n))
+            return -TARGET_EFAULT;
+        efds_ptr = &efds;
     } else {
-        target_efds = NULL;
         efds_ptr = NULL;
     }
 
-    if (target_tv) {
-        target_to_host_timeval(&tv, target_tv);
+    if (target_tv_addr) {
+        target_to_host_timeval(&tv, target_tv_addr);
         tv_ptr = &tv;
     } else {
         tv_ptr = NULL;
     }
+
     ret = get_errno(select(n, rfds_ptr, wfds_ptr, efds_ptr, tv_ptr));
-    ok = !is_error(ret);
 
-    if (ok) {
-        host_to_target_fds(target_rfds, rfds_ptr, n);
-        host_to_target_fds(target_wfds, wfds_ptr, n);
-        host_to_target_fds(target_efds, efds_ptr, n);
+    if (!is_error(ret)) {
+        if (rfd_addr && copy_to_user_fdset(rfd_addr, &rfds, n))
+            return -TARGET_EFAULT;
+        if (wfd_addr && copy_to_user_fdset(wfd_addr, &wfds, n))
+            return -TARGET_EFAULT;
+        if (efd_addr && copy_to_user_fdset(efd_addr, &efds, n))
+            return -TARGET_EFAULT;
 
-        if (target_tv) {
-            host_to_target_timeval(target_tv, &tv);
-        }
+        if (target_tv_addr)
+            host_to_target_timeval(target_tv_addr, &tv);
     }
 
-end:
-    unlock_user(target_rfds, rfd_p, ok ? sizeof(abi_long) * n : 0);
-    unlock_user(target_wfds, wfd_p, ok ? sizeof(abi_long) * n : 0);
-    unlock_user(target_efds, efd_p, ok ? sizeof(abi_long) * n : 0);
-
     return ret;
 }
 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [Qemu-devel] Re: [PATCH] 06_efault.4.patch - timeval
  2007-11-20 19:00 [Qemu-devel] [PATCH] additional EFAULT patches Thayne Harbaugh
  2007-11-20 19:08 ` [Qemu-devel] Re: [PATCH] 06_efault.3.patch - copy_from_user_fdset() Thayne Harbaugh
@ 2007-11-21  4:09 ` Thayne Harbaugh
  2007-11-21  4:11 ` [Qemu-devel] Re: [PATCH] 06_efault.5.timespec.patch Thayne Harbaugh
  2 siblings, 0 replies; 5+ messages in thread
From: Thayne Harbaugh @ 2007-11-21  4:09 UTC (permalink / raw)
  To: qemu-devel

[-- Attachment #1: Type: text/plain, Size: 156 bytes --]

This is the EFAULT for copy_{to,from}_user_timeval().  This updates to
use __get_user()/__put_user(), check return values of
copy_{to,from}_user_timeval().

[-- Attachment #2: 06_efault.4.patch --]
[-- Type: text/x-patch, Size: 5769 bytes --]

Index: qemu/linux-user/syscall.c
===================================================================
--- qemu.orig/linux-user/syscall.c	2007-11-20 12:52:33.000000000 -0700
+++ qemu/linux-user/syscall.c	2007-11-20 12:52:47.000000000 -0700
@@ -552,30 +552,34 @@
     return 0;
 }
 
-static inline abi_long target_to_host_timeval(struct timeval *tv,
-                                              abi_ulong target_addr)
+static inline abi_long copy_from_user_timeval(struct timeval *tv,
+                                              abi_ulong target_tv_addr)
 {
     struct target_timeval *target_tv;
 
-    if (!lock_user_struct(VERIFY_READ, target_tv, target_addr, 1))
+    if (!lock_user_struct(VERIFY_READ, target_tv, target_tv_addr, 1))
         return -TARGET_EFAULT;
-    tv->tv_sec = tswapl(target_tv->tv_sec);
-    tv->tv_usec = tswapl(target_tv->tv_usec);
-    unlock_user_struct(target_tv, target_addr, 0);
+
+    __get_user(tv->tv_sec, &target_tv->tv_sec);
+    __get_user(tv->tv_usec, &target_tv->tv_usec);
+
+    unlock_user_struct(target_tv, target_tv_addr, 0);
 
     return 0;
 }
 
-static inline abi_long host_to_target_timeval(abi_ulong target_addr,
-                                              const struct timeval *tv)
+static inline abi_long copy_to_user_timeval(abi_ulong target_tv_addr,
+                                            const struct timeval *tv)
 {
     struct target_timeval *target_tv;
 
-    if (!lock_user_struct(VERIFY_WRITE, target_tv, target_addr, 0))
+    if (!lock_user_struct(VERIFY_WRITE, target_tv, target_tv_addr, 0))
         return -TARGET_EFAULT;
-    target_tv->tv_sec = tswapl(tv->tv_sec);
-    target_tv->tv_usec = tswapl(tv->tv_usec);
-    unlock_user_struct(target_tv, target_addr, 1);
+
+    __put_user(tv->tv_sec, &target_tv->tv_sec);
+    __put_user(tv->tv_usec, &target_tv->tv_usec);
+
+    unlock_user_struct(target_tv, target_tv_addr, 1);
 
     return 0;
 }
@@ -614,7 +618,8 @@
     }
 
     if (target_tv_addr) {
-        target_to_host_timeval(&tv, target_tv_addr);
+        if (copy_from_user_timeval(&tv, target_tv_addr))
+            return -TARGET_EFAULT;
         tv_ptr = &tv;
     } else {
         tv_ptr = NULL;
@@ -630,8 +635,8 @@
         if (efd_addr && copy_to_user_fdset(efd_addr, &efds, n))
             return -TARGET_EFAULT;
 
-        if (target_tv_addr)
-            host_to_target_timeval(target_tv_addr, &tv);
+        if (target_tv_addr && copy_to_user_timeval(target_tv_addr, &tv))
+            return -TARGET_EFAULT;
     }
 
     return ret;
@@ -3392,9 +3397,10 @@
         {
             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)
+                    || copy_from_user_timeval(&tv[1],
+                                              arg2 + sizeof(struct target_timeval)))
+                    goto efault;
                 tvp = tv;
             } else {
                 tvp = NULL;
@@ -3934,14 +3940,16 @@
             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))
+                    goto efault;
             }
         }
         break;
     case TARGET_NR_settimeofday:
         {
             struct timeval tv;
-            target_to_host_timeval(&tv, arg1);
+            if (copy_from_user_timeval(&tv, arg1))
+                goto efault;
             ret = get_errno(settimeofday(&tv, NULL));
         }
         break;
@@ -4316,19 +4324,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)
+                    || copy_from_user_timeval(&pvalue->it_value,
+                                              arg2 + sizeof(struct target_timeval)))
+                    goto 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)
+                    || copy_to_user_timeval(arg3 + sizeof(struct target_timeval),
+                                            &ovalue.it_value))
+                    goto efault;
             }
         }
         break;
@@ -4338,10 +4347,11 @@
 
             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)
+                    || copy_to_user_timeval(arg2 + sizeof(struct target_timeval),
+                                            &value.it_value))
+                    goto efault;
             }
         }
         break;

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [Qemu-devel] Re: [PATCH] 06_efault.5.timespec.patch
  2007-11-20 19:00 [Qemu-devel] [PATCH] additional EFAULT patches Thayne Harbaugh
  2007-11-20 19:08 ` [Qemu-devel] Re: [PATCH] 06_efault.3.patch - copy_from_user_fdset() Thayne Harbaugh
  2007-11-21  4:09 ` [Qemu-devel] Re: [PATCH] 06_efault.4.patch - timeval Thayne Harbaugh
@ 2007-11-21  4:11 ` Thayne Harbaugh
  2 siblings, 0 replies; 5+ messages in thread
From: Thayne Harbaugh @ 2007-11-21  4:11 UTC (permalink / raw)
  To: qemu-devel

[-- Attachment #1: Type: text/plain, Size: 109 bytes --]

This uses __get_user()/__put_user() for copy_{to,from}_user_timespec().
It checks and handles return values.

[-- Attachment #2: 06_efault.5.timespec.patch --]
[-- Type: text/x-patch, Size: 4377 bytes --]

Index: qemu/linux-user/syscall.c
===================================================================
--- qemu.orig/linux-user/syscall.c	2007-11-20 13:21:38.000000000 -0700
+++ qemu/linux-user/syscall.c	2007-11-20 13:51:28.000000000 -0700
@@ -3026,28 +3026,36 @@
 }
 #endif
 
-static inline abi_long target_to_host_timespec(struct timespec *host_ts,
-                                               abi_ulong target_addr)
+static inline abi_long copy_from_user_timespec(struct timespec *host_ts,
+                                               abi_ulong target_ts_addr)
 {
     struct target_timespec *target_ts;
 
-    if (!lock_user_struct(VERIFY_READ, target_ts, target_addr, 1))
+    if (!lock_user_struct(VERIFY_READ, target_ts, target_ts_addr, 1))
         return -TARGET_EFAULT;
-    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);
+
+    __get_user(host_ts->tv_sec, &target_ts->tv_sec);
+    __get_user(host_ts->tv_nsec, &target_ts->tv_nsec);
+
+    unlock_user_struct(target_ts, target_ts_addr, 0);
+
+    return 0;
 }
 
-static inline abi_long host_to_target_timespec(abi_ulong target_addr,
-                                               struct timespec *host_ts)
+static inline abi_long copy_to_user_timespec(abi_ulong target_ts_addr,
+                                             const struct timespec *host_ts)
 {
     struct target_timespec *target_ts;
 
-    if (!lock_user_struct(VERIFY_WRITE, target_ts, target_addr, 0))
+    if (!lock_user_struct(VERIFY_WRITE, target_ts, target_ts_addr, 0))
         return -TARGET_EFAULT;
-    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);
+
+    __put_user(host_ts->tv_sec, &target_ts->tv_sec);
+    __put_user(host_ts->tv_nsec, &target_ts->tv_nsec);
+
+    unlock_user_struct(target_ts, target_ts_addr, 1);
+
+    return 0;
 }
 
 /* do_syscall() should always have a single exit point at the end so
@@ -3855,7 +3863,8 @@
             unlock_user(p, arg1, 0);
             if (arg3) {
                 puts = &uts;
-                target_to_host_timespec(puts, arg3);
+                if (copy_from_user_timespec(puts, arg3))
+                    goto efault;
             } else {
                 puts = NULL;
             }
@@ -4807,17 +4816,21 @@
             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))
+                    goto efault;
             }
         }
         break;
     case TARGET_NR_nanosleep:
         {
             struct timespec req, rem;
-            target_to_host_timespec(&req, arg1);
+
+            if (copy_from_user_timespec(&req, arg1))
+                goto efault;
             ret = get_errno(nanosleep(&req, &rem));
-            if (is_error(ret) && arg2) {
-                host_to_target_timespec(arg2, &rem);
+            if (!is_error(ret) && arg2) {
+                if (copy_to_user_timespec(arg2, &rem))
+                    goto efault;
             }
         }
         break;
@@ -5491,7 +5504,8 @@
         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))
+                goto efault;
         }
         break;
     }
@@ -5502,7 +5516,8 @@
         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))
+                goto efault;
         }
         break;
     }
@@ -5535,8 +5550,10 @@
     case TARGET_NR_utimensat:
         {
             struct timespec ts[2];
-            target_to_host_timespec(ts, arg3);
-            target_to_host_timespec(ts+1, arg3+sizeof(struct target_timespec));
+
+            if (copy_from_user_timespec(ts, arg3)
+                || copy_from_user_timespec(ts+1, arg3+sizeof(struct target_timespec)))
+                goto efault;
             if (!arg2)
                 ret = get_errno(sys_utimensat(arg1, NULL, ts, arg4));
             else {

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2007-11-21  4:19 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-11-20 19:00 [Qemu-devel] [PATCH] additional EFAULT patches Thayne Harbaugh
2007-11-20 19:08 ` [Qemu-devel] Re: [PATCH] 06_efault.3.patch - copy_from_user_fdset() Thayne Harbaugh
2007-11-20 19:30   ` Thayne Harbaugh
2007-11-21  4:09 ` [Qemu-devel] Re: [PATCH] 06_efault.4.patch - timeval Thayne Harbaugh
2007-11-21  4:11 ` [Qemu-devel] Re: [PATCH] 06_efault.5.timespec.patch Thayne Harbaugh

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).