[gentoo-commits] proj/sandbox:master commit in: libsandbox/

[Mike Frysinger]

commit:     9a927b63d0f76fb5edaf4abf43784419b944f21c
Author:     Mike Frysinger <vapier <AT> gentoo <DOT> org>
AuthorDate: Sun Sep 20 22:10:01 2015 +0000
Commit:     Mike Frysinger <vapier <AT> gentoo <DOT> org>
CommitDate: Sun Sep 20 22:10:01 2015 +0000
URL:        https://gitweb.gentoo.org/proj/sandbox.git/commit/?id=9a927b63

libsandbox: fix process_vm_readv addresses/lengths

The current logic calculates the lengths/base addresses incorrectly
leading to some kernels/mappings to reject accesses.  Make sure we
calculate the initial length properly, and then increment the base
by that value later on.

With those fixes in place, we can clean up the warning/exit paths.

URL: https://bugs.gentoo.org/560396
Reported-by: Jeroen Roovers <jer <AT> gentoo.org>
Signed-off-by: Mike Frysinger <vapier <AT> gentoo.org>

 libsandbox/trace.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/libsandbox/trace.c b/libsandbox/trace.c
index 47d2bdf..99ef8cd 100644
--- a/libsandbox/trace.c
+++ b/libsandbox/trace.c
@@ -93,10 +93,14 @@ static char *do_peekstr(unsigned long lptr)
 #ifdef HAVE_PROCESS_VM_READV
 	struct iovec liov, riov;
 
-	/* We can't cross remote page boundaries when using this :( */
+	/* We can't cross remote page boundaries when using this :(.
+	 * The first call reads up to the edge of the page boundary since we
+	 * have no (easy) way of knowing if the next page is valid.  This is
+	 * less effort than reading in the /proc/.../maps file and walking it.
+	 */
 	l = 0x1000;
 	riov.iov_base = (void *)lptr;
-	len = lptr % l;
+	len = l - (lptr % l);
 	if (!len)
 		len = l;
 	liov.iov_base = ret = xmalloc(len);
@@ -105,27 +109,23 @@ static char *do_peekstr(unsigned long lptr)
 	while (1) {
 		if (process_vm_readv(trace_pid, &liov, 1, &riov, 1, 0) == -1) {
 			int e = errno;
-			if (e == ENOSYS)
+			if (e == ENOSYS) {
+				/* This can happen if run on older kernels but built with newer ones. */
+				break;
+			} else if (e == EFAULT) {
+				/* This can happen if the target process uses a bad pointer. #560396 */
 				break;
-			sb_eqawarn("process_vm_readv(%i, %p{%p, %zu}, 1, %p{%p, %zu}, 1, 0) failed: %s\n",
+			}
+			sb_ebort("ISE:do_peekstr:process_vm_readv(%i, %p{%p, %#zx}, 1, %p{%p, %#zx}, 1, 0) failed: %s\n",
 				trace_pid,
 				&liov, liov.iov_base, liov.iov_len,
 				&riov, riov.iov_base, riov.iov_len,
 				strerror(e));
-			if (e == EFAULT) {
-				/* This sometimes happens, but it's not clear why.
-				 * Throw some extended debugging info before falling
-				 * back to the ptrace code. #560396
-				 */
-				break;
-			}
-			sb_ebort("ISE:do_peekstr: process_vm_readv() hates us: %s\n",
-				strerror(e));
 		}
 
 		if (memchr(liov.iov_base, '\0', liov.iov_len) != NULL)
 			return ret;
-		riov.iov_base += l;
+		riov.iov_base += riov.iov_len;
 		riov.iov_len = liov.iov_len = l;
 		len += l;
 		ret = xrealloc(ret, len);