Increase the size of the shadow call stack guard region to 16MB.
Increasing the size of the guard region helps with the security of SCS,
but it's blocked on landing [1], which in turn is blocked on landing
[2]. Once those two CLs land we will be able to land this one.
[1] https://android-review.googlesource.com/c/platform/frameworks/av/+/837745
[2] https://android-review.googlesource.com/c/platform/bionic/+/818973
Bug: 118642754
Change-Id: I35409cbb6bfcd77e632567dd755376e345cfe67b
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_
