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

[Mike Frysinger]

commit:     0a9188fd0a812cb864819d37a6a7217a135b85f0
Author:     Mike Frysinger <vapier <AT> gentoo <DOT> org>
AuthorDate: Sun Sep 20 22:25:33 2015 +0000
Commit:     Mike Frysinger <vapier <AT> gentoo <DOT> org>
CommitDate: Sun Sep 20 22:25:33 2015 +0000
URL:        https://gitweb.gentoo.org/proj/sandbox.git/commit/?id=0a9188fd

libsandbox: do not abort when the target uses bad pointers

If the target passes a bad pointer to the kernel, then trying to extract
the data via ptrace will also throw an error.  The tracing code should not
abort though as there's no valid address to check, and kernel itself will
return an error for us.  Simply return and move on.

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              | 14 ++++++++++
 localdecls.h                    |  4 +++
 tests/Makefile.am               |  3 +-
 tests/script-13.sh              |  5 ++++
 tests/script.at                 |  1 +
 tests/tests.h                   |  4 +++
 tests/trace-memory_static_tst.c | 62 +++++++++++++++++++++++++++++++++++++++++
 7 files changed, 92 insertions(+), 1 deletion(-)

diff --git a/libsandbox/trace.c b/libsandbox/trace.c
index 99ef8cd..f9194fe 100644
--- a/libsandbox/trace.c
+++ b/libsandbox/trace.c
@@ -59,6 +59,11 @@ static long _do_ptrace(enum __ptrace_request request, const char *srequest, void
 			}
 			sched_yield();
 			goto try_again;
+		} else if (errno == EIO || errno == EFAULT) {
+			/* This comes up when the child itself tries to use a bad pointer.
+			 * That's not something the sandbox should abort on. #560396
+			 */
+			return ret;
 		} else if (!errno)
 			if (request == PTRACE_PEEKDATA ||
 			    request == PTRACE_PEEKTEXT ||
@@ -140,7 +145,16 @@ static char *do_peekstr(unsigned long lptr)
 	while (1) {
 		a = lptr & (sizeof(long) - 1);
 		lptr -= a;
+		errno = 0;
 		s.val = do_peekdata(lptr);
+		if (unlikely(errno)) {
+			if (errno == EIO || errno == EFAULT) {
+				ret[0] = '\0';
+				return ret;
+			}
+			sb_ebort("ISE:do_peekstr:do_peekdata(%#lx) failed: %s\n",
+				lptr, strerror(errno));
+		}
 		for (i = a; i < sizeof(long); ++i) {
 			ret[l++] = s.x[i];
 			if (!s.x[i])

diff --git a/localdecls.h b/localdecls.h
index 0e11fea..ecc5856 100644
--- a/localdecls.h
+++ b/localdecls.h
@@ -102,6 +102,10 @@ typedef struct user_regs_struct trace_regs;
 #ifndef offsetof
 #define offsetof(TYPE, MEMBER) ((size_t) &((TYPE *)0)->MEMBER)
 #endif
+#define ALIGN_DOWN(base, size)     ((base) & -(size))
+#define ALIGN_UP(base, size)       ALIGN_DOWN((base) + (size) - 1, (size))
+#define PTR_ALIGN_DOWN(base, size) ((__typeof__(base))ALIGN_DOWN((uintptr_t)(base), (size)))
+#define PTR_ALIGN_UP(base, size)   ((__typeof__(base))ALIGN_UP((uintptr_t)(base), (size)))
 
 /* If the system is old and does not support *at funcs, then define
  * it ourself.  Shouldn't matter based on how we use it.

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 0f0c249..3627344 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -76,7 +76,8 @@ check_PROGRAMS = \
 	pipe-fork_static_tst \
 	sb_printf_tst \
 	sigsuspend-zsh_tst \
-	sigsuspend-zsh_static_tst
+	sigsuspend-zsh_static_tst \
+	trace-memory_static_tst
 
 dist_check_SCRIPTS = \
 	$(wildcard $(srcdir)/*-[0-9]*.sh) \

diff --git a/tests/script-13.sh b/tests/script-13.sh
new file mode 100755
index 0000000..24a0e3b
--- /dev/null
+++ b/tests/script-13.sh
@@ -0,0 +1,5 @@
+#!/bin/sh
+# make sure poking remote addresses works even when they're bad/unaligned #560396
+[ "${at_xfail}" = "yes" ] && exit 77 # see script-0
+
+trace-memory_static_tst

diff --git a/tests/script.at b/tests/script.at
index f07a8f1..58a5077 100644
--- a/tests/script.at
+++ b/tests/script.at
@@ -10,3 +10,4 @@ SB_CHECK(9, [wait errpipe... done OK!])
 SB_CHECK(10)
 SB_CHECK(11)
 SB_CHECK(12)
+SB_CHECK(13)

diff --git a/tests/tests.h b/tests/tests.h
index 72c678c..51dc68a 100644
--- a/tests/tests.h
+++ b/tests/tests.h
@@ -1,5 +1,9 @@
 #include "headers.h"
 
+/* Make sure assert() works in tests. */
+#undef NDEBUG
+#include <assert.h>
+
 #define _msg(std, fmt, args...) fprintf(std, "%s:%s():%i: " fmt "\n", __FILE__, __func__, __LINE__, ##args)
 #define _stderr_msg(fmt, args...) _msg(stderr, fmt, ##args)
 #define _stderr_pmsg(fmt, args...) _msg(stderr, fmt ": %s", ##args, strerror(errno))

diff --git a/tests/trace-memory_static_tst.c b/tests/trace-memory_static_tst.c
new file mode 100644
index 0000000..14c6477
--- /dev/null
+++ b/tests/trace-memory_static_tst.c
@@ -0,0 +1,62 @@
+/*
+ * Make sure the process_vm_readv func can work with strings with different
+ * alignments and lengths.
+ *
+ * https://bugs.gentoo.org/560396
+ */
+
+/* We want to make some bad calls. */
+#undef _FORTIFY_SOURCES
+
+#include "tests.h"
+
+/* Make sure the buffer spans multiple pages. */
+#define SIZE 0x1000
+/* Make sure the buffer has plenty of slack space before/after. */
+static char buf[SIZE * 8];
+
+/* The smaller the span, the # of calls goes up: O(N*N*2+N). */
+#define COUNT 0x20
+#define STRIDE (SIZE / COUNT)
+
+/* Some hacks to defeat gcc warnings so we can use bad pointers. */
+volatile uintptr_t offset = 0;
+#define non_const_ptr(ptr) ((void *)((uintptr_t)(ptr) + offset))
+
+#define check_ptr(addr) \
+({ \
+	printf("  open(%p)\n", addr); \
+	ret = open(non_const_ptr(addr), O_RDONLY); \
+	assert(ret == -1 && errno == EFAULT); \
+})
+
+int main(int argc, char *argv[])
+{
+	int ret;
+	char *path = PTR_ALIGN_UP((char *)buf, SIZE);
+	size_t start, end;
+
+	setbuf(stdout, NULL);
+
+	printf("some bad pointers\n");
+	check_ptr(NULL);
+	check_ptr((void *)-1);
+
+	printf("lots of good pointers\n");
+	printf("  buf = %p\n", buf);
+	printf("  path = %p\n", path);
+
+	for (start = 0; start < SIZE * 2 + STRIDE; start += STRIDE) {
+		char *p = path + start;
+		for (end = start + STRIDE; end < SIZE * 2 + STRIDE; end += STRIDE) {
+			size_t len = end - start;
+			printf("  open(%p -> %p [+%#zx])\n", p, p + len, len);
+			memset(p, 'a', len);
+			path[end] = '\0';
+			ret = open(p, O_RDONLY);
+			assert(ret == -1 && (errno == ENOENT || errno == ENAMETOOLONG));
+		}
+	}
+
+	return 0;
+}