Add some slack at the end of large allocations when target SDK level < S.
This works around buggy applications that read a few bytes past the
end of their allocation, which would otherwise cause a segfault with
the concurrent Scudo change that aligns large allocations to the right.
Because the implementation of
android_set_application_target_sdk_version() lives in the linker,
we need to introduce a hook so that libc is notified when the target
SDK version changes.
Bug: 181344545
Change-Id: Id4be6645b94fad3f64ae48afd16c0154f1de448f
diff --git a/libc/bionic/libc_init_common.cpp b/libc/bionic/libc_init_common.cpp
index 01cd2e5..d9dcdc4 100644
--- a/libc/bionic/libc_init_common.cpp
+++ b/libc/bionic/libc_init_common.cpp
@@ -131,6 +131,14 @@
pthread_atfork(arc4random_fork_handler, _thread_arc4_unlock, _thread_arc4_unlock);
}
+extern "C" void scudo_malloc_set_add_large_allocation_slack(int add_slack);
+
+__BIONIC_WEAK_FOR_NATIVE_BRIDGE void __libc_set_target_sdk_version(int target __unused) {
+#if defined(USE_SCUDO)
+ scudo_malloc_set_add_large_allocation_slack(target < __ANDROID_API_S__);
+#endif
+}
+
__noreturn static void __early_abort(int line) {
// We can't write to stdout or stderr because we're aborting before we've checked that
// it's safe for us to use those file descriptors. We probably can't strace either, so
diff --git a/libc/bionic/libc_init_common.h b/libc/bionic/libc_init_common.h
index a899089..15c747e 100644
--- a/libc/bionic/libc_init_common.h
+++ b/libc/bionic/libc_init_common.h
@@ -66,4 +66,6 @@
// pthread_atfork may call malloc() during its once-init.
__LIBC_HIDDEN__ void __libc_init_fork_handler();
+__LIBC_HIDDEN__ void __libc_set_target_sdk_version(int target);
+
#endif
diff --git a/libc/bionic/libc_init_dynamic.cpp b/libc/bionic/libc_init_dynamic.cpp
index 175fa3e..4625fa1 100644
--- a/libc/bionic/libc_init_dynamic.cpp
+++ b/libc/bionic/libc_init_dynamic.cpp
@@ -46,6 +46,7 @@
#include <elf.h>
#include "libc_init_common.h"
+#include "private/bionic_defs.h"
#include "private/bionic_elf_tls.h"
#include "private/bionic_globals.h"
#include "platform/bionic/macros.h"
@@ -107,6 +108,8 @@
__libc_shared_globals()->unload_hook = __hwasan_library_unloaded;
#endif
+ __libc_shared_globals()->set_target_sdk_version_hook = __libc_set_target_sdk_version;
+
netdClientInit();
}
diff --git a/libc/bionic/libc_init_static.cpp b/libc/bionic/libc_init_static.cpp
index 2e4ee11..069ebb0 100644
--- a/libc/bionic/libc_init_static.cpp
+++ b/libc/bionic/libc_init_static.cpp
@@ -400,6 +400,7 @@
extern "C" void android_set_application_target_sdk_version(int target) {
g_target_sdk_version = target;
+ __libc_set_target_sdk_version(target);
}
// This function is called in the dynamic linker before ifunc resolvers have run, so this file is
diff --git a/libc/private/bionic_globals.h b/libc/private/bionic_globals.h
index 8132261..0d35db8 100644
--- a/libc/private/bionic_globals.h
+++ b/libc/private/bionic_globals.h
@@ -95,9 +95,10 @@
TlsModules tls_modules;
BionicAllocator tls_allocator;
- // Values passed from the HWASan runtime (via libc.so) to the loader.
+ // Values passed from libc.so to the loader.
void (*load_hook)(ElfW(Addr) base, const ElfW(Phdr)* phdr, ElfW(Half) phnum) = nullptr;
void (*unload_hook)(ElfW(Addr) base, const ElfW(Phdr)* phdr, ElfW(Half) phnum) = nullptr;
+ void (*set_target_sdk_version_hook)(int target) = nullptr;
// Values passed from the linker to libc.so.
const char* init_progname = nullptr;
diff --git a/linker/linker_sdk_versions.cpp b/linker/linker_sdk_versions.cpp
index 29c0f4a..0d5796e 100644
--- a/linker/linker_sdk_versions.cpp
+++ b/linker/linker_sdk_versions.cpp
@@ -31,6 +31,8 @@
#include <android/api-level.h>
#include <android/fdsan.h>
+#include "private/bionic_globals.h"
+
#include "linker.h"
static std::atomic<int> g_target_sdk_version(__ANDROID_API__);
@@ -45,6 +47,9 @@
if (target < 30) {
android_fdsan_set_error_level_from_property(ANDROID_FDSAN_ERROR_LEVEL_WARN_ONCE);
}
+ if (__libc_shared_globals()->set_target_sdk_version_hook) {
+ __libc_shared_globals()->set_target_sdk_version_hook(target);
+ }
}
int get_application_target_sdk_version() {
diff --git a/tests/malloc_test.cpp b/tests/malloc_test.cpp
index 3a09258..d73f243 100644
--- a/tests/malloc_test.cpp
+++ b/tests/malloc_test.cpp
@@ -46,6 +46,7 @@
#if defined(__BIONIC__)
#include "SignalUtils.h"
+#include "dlext_private.h"
#include "platform/bionic/malloc.h"
#include "platform/bionic/mte.h"
@@ -1351,3 +1352,22 @@
GTEST_SKIP() << "bionic extension";
#endif
}
+
+TEST(malloc, allocation_slack) {
+#if defined(__BIONIC__)
+ bool allocator_scudo;
+ GetAllocatorVersion(&allocator_scudo);
+ if (!allocator_scudo) {
+ GTEST_SKIP() << "scudo allocator only test";
+ }
+
+ // Test that older target SDK levels let you access a few bytes off the end of
+ // a large allocation.
+ android_set_application_target_sdk_version(29);
+ auto p = std::make_unique<char[]>(131072);
+ volatile char *vp = p.get();
+ volatile char oob ATTRIBUTE_UNUSED = vp[131072];
+#else
+ GTEST_SKIP() << "bionic extension";
+#endif
+}