Set the glibc thread's pointer_guard (XCC)
authorBarret Rhoden <brho@cs.berkeley.edu>
Mon, 12 Sep 2016 20:09:33 +0000 (16:09 -0400)
committerBarret Rhoden <brho@cs.berkeley.edu>
Fri, 16 Sep 2016 18:35:09 +0000 (14:35 -0400)
Every thread needs to have the same pointer_guard.  We inherit it from the
creating parent (TLS, really).  And while we're here, set the stack_guard
too.  This is what glibc does when it creates a pthread.

All threads must have the same pointer_guard.  The issue is that if a
thread other than thread0 sets an atexit() function pointer, then the
pointer gets mangled with the wrong pointer_guard value.  Then when thread0
exits, it will improperly demangle the value.

I considered just turning off pointer mangling, but this is fine as is.

Note that we set the pointer_guard on every TLS, not on every thread.  This
is fine.  Basically this is a global value that every context should see,
so it's fine for it to be in every TLS.  I think glibc doesn't really make
a distinction between TLS and threads; every TLS region has the glibc
thread struct sitting at fs:0, for instance.

I'm not sure why glibc didn't use a global.  Maybe it'd be easier for an
attacker to find the global than the TLS value (to do their own
demangling).  Though fs:0x30 is probably just as easy to find, and you
don't have to do any linking to find the global.

Rebuild glibc.

Signed-off-by: Barret Rhoden <brho@cs.berkeley.edu>
tools/compilers/gcc-glibc/glibc-2.19-akaros/sysdeps/akaros/tls.c
user/utest/atexit.c [new file with mode: 0644]

index 6c0b6e7..d50acab 100644 (file)
@@ -28,6 +28,8 @@ void *allocate_tls(void)
        tcbhead_t *head = (tcbhead_t*)tcb;
        head->tcb = tcb;
        head->self = tcb;
+       head->pointer_guard = THREAD_SELF->header.pointer_guard;
+       head->stack_guard = THREAD_SELF->header.stack_guard;
 #endif
        return tcb;
 }
diff --git a/user/utest/atexit.c b/user/utest/atexit.c
new file mode 100644 (file)
index 0000000..118a838
--- /dev/null
@@ -0,0 +1,65 @@
+/* Copyright (c) 2016 Google, Inc.
+ * Barret Rhoden <brho@cs.berkeley.edu>
+ * See LICENSE for details. */
+
+#include <utest/utest.h>
+#include <pthread.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+/* This tests a bug where if a thread other than thread0 registered an atexit()
+ * function, we'd GPF when thread0 exited.  Due to the way our infrastructure
+ * prints tests results before the program exited, you may see this say that the
+ * test succeeded, but then fail later with a GPF or other assertion. */
+
+TEST_SUITE("AT-EXIT");
+
+/* <--- Begin definition of test cases ---> */
+
+static bool child_ran_atexit = FALSE;
+
+static void child_atexit(void)
+{
+       child_ran_atexit = TRUE;
+}
+
+static void *child_func(void *arg)
+{
+       atexit(child_atexit);
+       return 0;
+}
+
+static void main_atexit(void)
+{
+       /* Using the non-UT assert, since the test_atexit_threads() has already
+        * returned.  Also, since the child called atexit() after main, its handler
+        * should run first, according to the man page. */
+       assert(child_ran_atexit);
+}
+
+bool test_atexit_threads(void)
+{
+       pthread_t child;
+       void *child_ret;
+
+       atexit(main_atexit);
+       pthread_create(&child, NULL, &child_func, NULL);
+       pthread_join(child, &child_ret);
+       return TRUE;
+}
+
+/* <--- End definition of test cases ---> */
+
+struct utest utests[] = {
+       UTEST_REG(atexit_threads),
+};
+int num_utests = sizeof(utests) / sizeof(struct utest);
+
+int main(int argc, char *argv[])
+{
+       // Run test suite passing it all the args as whitelist of what tests to run.
+       char **whitelist = &argv[1];
+       int whitelist_len = argc - 1;
+
+       RUN_TEST_SUITE(utests, num_utests, whitelist, whitelist_len);
+}