Module Name:    src
Committed By:   riastradh
Date:           Mon Nov 27 22:15:08 UTC 2023

Modified Files:
        src/tests/lib/libpthread: t_stack.c

Log Message:
libpthread/t_stack: Make this more robust to the guard size bug.

Make sure to allocate enough space for the thread's stack for a guard
even though there shouldn't be one, so that when we run the thread,
it doesn't start with the stack pointer pointing into someone else's
allocation (like malloc) causing stack frames to trash another data
structure -- or causing the user of that data structure to trash the
stack frames.

PR lib/57721

XXX pullup-10
XXX pullup-9
XXX pullup-8


To generate a diff of this commit:
cvs rdiff -u -r1.1 -r1.2 src/tests/lib/libpthread/t_stack.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/tests/lib/libpthread/t_stack.c
diff -u src/tests/lib/libpthread/t_stack.c:1.1 src/tests/lib/libpthread/t_stack.c:1.2
--- src/tests/lib/libpthread/t_stack.c:1.1	Fri Nov 24 16:21:17 2023
+++ src/tests/lib/libpthread/t_stack.c	Mon Nov 27 22:15:08 2023
@@ -1,4 +1,4 @@
-/*	$NetBSD: t_stack.c,v 1.1 2023/11/24 16:21:17 riastradh Exp $	*/
+/*	$NetBSD: t_stack.c,v 1.2 2023/11/27 22:15:08 riastradh Exp $	*/
 
 /*-
  * Copyright (c) 2023 The NetBSD Foundation, Inc.
@@ -29,11 +29,15 @@
 #define	_KMEMUSER		/* __MACHINE_STACK_GROWS_UP */
 
 #include <sys/cdefs.h>
-__RCSID("$NetBSD: t_stack.c,v 1.1 2023/11/24 16:21:17 riastradh Exp $");
+__RCSID("$NetBSD: t_stack.c,v 1.2 2023/11/27 22:15:08 riastradh Exp $");
 
 #include <sys/mman.h>
+#include <sys/param.h>
+#include <sys/sysctl.h>
 #include <sys/types.h>
 
+#include <uvm/uvm_param.h>	/* VM_THREAD_GUARD_SIZE */
+
 #include <atf-c.h>
 #include <pthread.h>
 #include <setjmp.h>
@@ -52,6 +56,7 @@ struct jmp_ctx {
  */
 struct ctx {
 	size_t size;		/* default stack size */
+	size_t guardsize;	/* default guard size */
 	void *addr;		/* user-allocated stack */
 	pthread_key_t jmp_key;	/* jmp_ctx to return from SIGSEGV handler */
 } ctx, *C = &ctx;
@@ -101,6 +106,34 @@ getnondefaultstacksize(void)
 }
 
 /*
+ * getdefaultguardsize()
+ *
+ *	Return the default guard size for threads created with
+ *	pthread_create.
+ */
+static size_t
+getdefaultguardsize(void)
+{
+	const int mib[2] = { CTL_VM, VM_THREAD_GUARD_SIZE };
+	unsigned guardsize;
+	size_t len = sizeof(guardsize);
+
+	RL(sysctl(mib, __arraycount(mib), &guardsize, &len, NULL, 0));
+	ATF_REQUIRE_EQ_MSG(len, sizeof(guardsize),
+	    "len=%zu sizeof(guardsize)=%zu", len, sizeof(guardsize));
+
+	/*
+	 * Verify this matches what libpthread determined.
+	 */
+	extern size_t pthread__guardsize; /* pthread_int.h */
+	ATF_CHECK_EQ_MSG(guardsize, pthread__guardsize,
+	    "guardsize=%zu pthread__guardsize=%zu",
+	    guardsize, pthread__guardsize);
+
+	return guardsize;
+}
+
+/*
  * alloc(nbytes)
  *
  *	Allocate an nbytes-long page-aligned read/write region and
@@ -124,17 +157,35 @@ alloc(size_t nbytes)
  *
  *	Initialize state used by various tests with the specified
  *	stacksize.
+ *
+ *	Make sure to allocate enough space that even if there shouldn't
+ *	be a stack guard (i.e., it should be empty), adjusting the
+ *	requested bounds by the default stack guard size will leave us
+ *	inside allocated memory.
  */
 static void
 init(size_t stacksize)
 {
 
 	C->size = stacksize;
-	C->addr = alloc(C->size);
+	C->guardsize = getdefaultguardsize();
+	C->addr = (char *)alloc(C->size + C->guardsize);
 	RZ(pthread_key_create(&C->jmp_key, NULL));
 }
 
 /*
+ * stack_pointer()
+ *
+ *	Return the stack pointer.  This is used to verify whether the
+ *	stack pointer lie within a certain address range.
+ */
+static __noinline void *
+stack_pointer(void)
+{
+	return __builtin_frame_address(0);
+}
+
+/*
  * sigsegv_ok(signo)
  *
  *	Signal handler for SIGSEGV to return to the jmp ctx, to verify
@@ -276,10 +327,37 @@ checkaddraccessthread(void *cookie)
 {
 	pthread_t t = pthread_self();
 	pthread_attr_t attr;
+	void *sp;
 	void *addr;
 	size_t size, size0;
 
 	/*
+	 * Verify the stack pointer lies somewhere in the allocated
+	 * range.
+	 */
+	sp = stack_pointer();
+	ATF_CHECK_MSG(C->addr <= sp, "sp=%p not in [%p,%p + 0x%zu) = [%p,%p)",
+	    sp, C->addr, C->addr, C->size, C->addr, (char *)C->addr + C->size);
+	ATF_CHECK_MSG(sp <= (void *)((char *)C->addr + C->size),
+	    "sp=%p not in [%p,%p + 0x%zu) = [%p,%p)",
+	    sp, C->addr, C->addr, C->size, C->addr, (char *)C->addr + C->size);
+
+	/*
+	 * Verify, if not that, then the stack pointer at least lies
+	 * within the extra buffer we allocated for slop to address a
+	 * bug NetBSD libpthread used to have of spuriously adding the
+	 * guard size to a user-allocated stack address.  This is
+	 * ATF_REQUIRE, not ATF_CHECK, because if this doesn't hold, we
+	 * might be clobbering some other memory like malloc pages,
+	 * causing the whole test to crash with useless diagnostics.
+	 */
+	ATF_REQUIRE_MSG(sp <= (void *)((char *)C->addr + C->size +
+		C->guardsize),
+	    "sp=%p not even in buffer [%p,%p + 0x%zu + 0x%zu) = [%p,%p)",
+	    sp, C->addr, C->addr, C->size, C->guardsize,
+	    C->addr, (char *)C->addr + C->size + C->guardsize);
+
+	/*
 	 * Get the stack parameters -- both via pthread_attr_getstack
 	 * and via pthread_attr_getstacksize, to make sure they agree
 	 * -- and verify that they are what we expect from the caller.

Reply via email to