Merge "Increase the size of the shadow call stack guard region to 16MB."
am: b62888b709
Change-Id: Ie969f206bf517509c709a64dd324014b4596569f
diff --git a/libc/bionic/pthread_create.cpp b/libc/bionic/pthread_create.cpp
index b8784b8..4cf14ad 100644
--- a/libc/bionic/pthread_create.cpp
+++ b/libc/bionic/pthread_create.cpp
@@ -127,18 +127,25 @@
static void __init_shadow_call_stack(pthread_internal_t* thread __unused) {
#ifdef __aarch64__
- // Allocate the stack and store its address in register x18. The address is aligned to SCS_SIZE so
- // that we only need to store the lower log2(SCS_SIZE) bits in jmp_buf.
- // TODO(pcc): We ought to allocate a larger guard region here and then allocate the SCS at a
- // random location within it. This will provide greater security since it would mean that an
- // attacker who can read the pthread_internal_t won't be able to discover the address of the SCS.
- // However, doing so is blocked on a solution to b/118642754.
+ // Allocate the stack and the guard region.
char* scs_guard_region = reinterpret_cast<char*>(
mmap(nullptr, SCS_GUARD_REGION_SIZE, 0, MAP_PRIVATE | MAP_ANON, -1, 0));
thread->shadow_call_stack_guard_region = scs_guard_region;
- char* scs =
+ // The address is aligned to SCS_SIZE so that we only need to store the lower log2(SCS_SIZE) bits
+ // in jmp_buf.
+ char* scs_aligned_guard_region =
reinterpret_cast<char*>(align_up(reinterpret_cast<uintptr_t>(scs_guard_region), SCS_SIZE));
+
+ // We need to ensure that [scs_offset,scs_offset+SCS_SIZE) is in the guard region and that there
+ // is at least one unmapped page after the shadow call stack (to catch stack overflows). We can't
+ // use arc4random_uniform in init because /dev/urandom might not have been created yet.
+ size_t scs_offset =
+ (getpid() == 1) ? 0 : (arc4random_uniform(SCS_GUARD_REGION_SIZE / SCS_SIZE - 1) * SCS_SIZE);
+
+ // Make the stack readable and writable and store its address in register x18. This is
+ // deliberately the only place where the address is stored.
+ char *scs = scs_aligned_guard_region + scs_offset;
mprotect(scs, SCS_SIZE, PROT_READ | PROT_WRITE);
__asm__ __volatile__("mov x18, %0" ::"r"(scs));
#endif
diff --git a/libc/private/bionic_constants.h b/libc/private/bionic_constants.h
index e64c826..09294b6 100644
--- a/libc/private/bionic_constants.h
+++ b/libc/private/bionic_constants.h
@@ -26,7 +26,6 @@
// guard region must be large enough that we can allocate an SCS_SIZE-aligned SCS while ensuring
// that there is at least one guard page after the SCS so that a stack overflow results in a SIGSEGV
// instead of corrupting the allocation that comes after it.
-// TODO(b/118642754): Use a larger guard region.
-#define SCS_GUARD_REGION_SIZE (SCS_SIZE * 2)
+#define SCS_GUARD_REGION_SIZE (16 * 1024 * 1024)
#endif // _BIONIC_CONSTANTS_H_