diff options
author | Android Build Coastguard Worker <android-build-coastguard-worker@google.com> | 2024-05-07 23:24:44 +0000 |
---|---|---|
committer | Gerrit Code Review <noreply-gerritcodereview@google.com> | 2024-05-07 23:24:44 +0000 |
commit | 47ab3e4d2e5f76a4889c1e8ba4ec28a88def2556 (patch) | |
tree | e5f799d04c795e3fdb3fba53c12e5a6e452dc1e0 | |
parent | 48e27eecbdddeb945a7d461b5fe69f77e0545314 (diff) | |
parent | c523d369c11a5520d137ad9374aa65bd40f8787d (diff) | |
download | bionic-sdk-release.tar.gz |
Merge "Snap for 11812660 from 3aa32e2c81aeb6d649a0a5420fc8f0eccaf2e756 to sdk-release" into sdk-releasesdk-release
-rw-r--r-- | docs/status.md | 29 | ||||
-rw-r--r-- | libc/Android.bp | 46 | ||||
-rw-r--r-- | libc/bionic/heap_tagging.cpp | 9 | ||||
-rw-r--r-- | libc/include/android/versioning.h | 14 | ||||
-rw-r--r-- | libc/include/stdio.h | 2 | ||||
-rw-r--r-- | libc/system_properties/system_properties.cpp | 21 | ||||
-rw-r--r-- | linker/linker_crt_pad_segment_test.cpp | 9 | ||||
-rw-r--r-- | linker/linker_phdr.cpp | 59 | ||||
-rw-r--r-- | linker/linker_phdr.h | 2 | ||||
-rw-r--r-- | tests/clang_fortify_tests.cpp | 8 |
10 files changed, 142 insertions, 57 deletions
diff --git a/docs/status.md b/docs/status.md index 2919471e8..bc8ab6a4f 100644 --- a/docs/status.md +++ b/docs/status.md @@ -397,22 +397,25 @@ automatic bounds checking for common libc functions. If a buffer overrun is detected, the program is safely aborted as in this [example](https://source.android.com/devices/tech/debug/native-crash#fortify). -Note that in recent releases Android's FORTIFY has been extended to -cover other issues. It can now detect, for example, passing `O_CREAT` -to open(2) without specifying a mode. It also performs some checking -regardless of whether the caller was built with FORTIFY enabled. In P, -for example, calling a `pthread_mutex_` function on a destroyed mutex, -calling a `<dirent.h>` function on a null pointer, using `%n` with the -printf(3) family, or using the scanf(3) `m` modifier incorrectly will -all result in FORTIFY failures even for code not built with FORTIFY. +Note that Android's FORTIFY has been extended to cover other issues. It can +detect, for example, passing `O_CREAT` to open(2) without specifying a mode. It +also performs some checking regardless of whether the caller was built with +FORTIFY enabled. From API level 28, for example, calling a `pthread_mutex_` +function on a destroyed mutex, calling a `<dirent.h>` function on a null +pointer, using `%n` with the printf(3) family, or using the scanf(3) `m` +modifier incorrectly will all result in FORTIFY failures even for code not built +with FORTIFY. More background information is available in our [FORTIFY in Android](https://android-developers.googleblog.com/2017/04/fortify-in-android.html) -blog post. - -The Android platform is built with `-D_FORTIFY_SOURCE=2`, but NDK users -need to manually enable FORTIFY by setting that themselves in whatever -build system they're using. The exact subset of FORTIFY available to +blog post, and there's more detail about the implementation in +[The Anatomy of Clang FORTIFY](clang_fortify_anatomy.md). + +The Android platform is built with `-D_FORTIFY_SOURCE=2`. Users of ndk-build +or the NDK's CMake toolchain file also get this by default with NDK r21 or +newer. Users of other build systems +need to manually enable FORTIFY by setting `_FORTIFY_SOURCE` themselves in +whatever build system they're using. The exact subset of FORTIFY available to NDK users will depend on their target ABI level, because when a FORTIFY check can't be guaranteed at compile-time, a call to a run-time `_chk` function is added. diff --git a/libc/Android.bp b/libc/Android.bp index 84fa498d3..2efca6855 100644 --- a/libc/Android.bp +++ b/libc/Android.bp @@ -55,7 +55,9 @@ libc_common_flags = [ cc_defaults { name: "libc_defaults", defaults: ["linux_bionic_supported"], - cflags: libc_common_flags, + cflags: libc_common_flags + [ + "-DUSE_SCUDO", + ], asflags: libc_common_flags, conlyflags: ["-std=gnu99"], cppflags: [], @@ -98,8 +100,8 @@ cc_defaults { malloc_pattern_fill_contents: { cflags: ["-DSCUDO_PATTERN_FILL_CONTENTS"], }, - malloc_not_svelte: { - cflags: ["-DUSE_SCUDO"], + malloc_low_memory: { + cflags: ["-UUSE_SCUDO"], }, }, @@ -112,32 +114,31 @@ cc_defaults { tidy_disabled_srcs: ["upstream-*/**/*.c"], } -libc_scudo_product_variables = { - malloc_not_svelte: { - cflags: ["-DUSE_SCUDO"], - whole_static_libs: ["libscudo"], - exclude_static_libs: [ - "libjemalloc5", - "libc_jemalloc_wrapper", - ], - }, -} - // Defaults for native allocator libs/includes to make it // easier to change. -// To disable scudo for the non-svelte config remove the line: -// product_variables: libc_scudo_product_variables, -// in the cc_defaults below. // ======================================================== cc_defaults { name: "libc_native_allocator_defaults", whole_static_libs: [ - "libjemalloc5", - "libc_jemalloc_wrapper", + "libscudo", + ], + cflags: [ + "-DUSE_SCUDO", ], header_libs: ["gwp_asan_headers"], - product_variables: libc_scudo_product_variables, + product_variables: { + malloc_low_memory: { + cflags: ["-UUSE_SCUDO"], + whole_static_libs: [ + "libjemalloc5", + "libc_jemalloc_wrapper", + ], + exclude_static_libs: [ + "libscudo", + ], + }, + }, } // Functions not implemented by jemalloc directly, or that need to @@ -2990,3 +2991,8 @@ filegroup { name: "versioner-dependencies", srcs: ["versioner-dependencies/**/*"], } + +filegroup { + name: "linux_capability_header", + srcs: ["kernel/uapi/linux/capability.h"], +} diff --git a/libc/bionic/heap_tagging.cpp b/libc/bionic/heap_tagging.cpp index 4d1981c30..c8a025f57 100644 --- a/libc/bionic/heap_tagging.cpp +++ b/libc/bionic/heap_tagging.cpp @@ -38,6 +38,11 @@ extern "C" void scudo_malloc_disable_memory_tagging(); extern "C" void scudo_malloc_set_track_allocation_stacks(int); +extern "C" const char* __scudo_get_stack_depot_addr(); +extern "C" const char* __scudo_get_ring_buffer_addr(); +extern "C" size_t __scudo_get_ring_buffer_size(); +extern "C" size_t __scudo_get_stack_depot_size(); + // Protected by `g_heap_tagging_lock`. static HeapTaggingLevel heap_tagging_level = M_HEAP_TAGGING_LEVEL_NONE; @@ -158,6 +163,10 @@ bool SetHeapTaggingLevel(HeapTaggingLevel tag_level) { set_tcf_on_all_threads(PR_MTE_TCF_SYNC); #if defined(USE_SCUDO) && !__has_feature(hwaddress_sanitizer) scudo_malloc_set_track_allocation_stacks(1); + __libc_shared_globals()->scudo_ring_buffer = __scudo_get_ring_buffer_addr(); + __libc_shared_globals()->scudo_ring_buffer_size = __scudo_get_ring_buffer_size(); + __libc_shared_globals()->scudo_stack_depot = __scudo_get_stack_depot_addr(); + __libc_shared_globals()->scudo_stack_depot_size = __scudo_get_stack_depot_size(); #endif } break; diff --git a/libc/include/android/versioning.h b/libc/include/android/versioning.h index cd61f3393..64528e1b5 100644 --- a/libc/include/android/versioning.h +++ b/libc/include/android/versioning.h @@ -22,8 +22,8 @@ #define __INTRODUCED_IN(api_level) __attribute__((__annotate__("introduced_in=" #api_level))) #define __INTRODUCED_IN_NO_GUARD_FOR_NDK(api_level) __attribute__((__annotate__("introduced_in=" #api_level))) __VERSIONER_NO_GUARD -#define __DEPRECATED_IN(api_level) __attribute__((__annotate__("deprecated_in=" #api_level))) -#define __REMOVED_IN(api_level) __attribute__((__annotate__("obsoleted_in=" #api_level))) +#define __DEPRECATED_IN(api_level, ...) __attribute__((__annotate__("deprecated_in=" #api_level))) +#define __REMOVED_IN(api_level, ...) __attribute__((__annotate__("obsoleted_in=" #api_level))) #define __INTRODUCED_IN_32(api_level) __attribute__((__annotate__("introduced_in_32=" #api_level))) #define __INTRODUCED_IN_64(api_level) __attribute__((__annotate__("introduced_in_64=" #api_level))) @@ -47,16 +47,16 @@ // libc++ doesn't currently guard these calls. There's no risk to the apps though because using // those APIs will still cause a link error. #if defined(__ANDROID_UNAVAILABLE_SYMBOLS_ARE_WEAK__) -#define __BIONIC_AVAILABILITY(__what) __attribute__((__availability__(android,__what))) +#define __BIONIC_AVAILABILITY(__what, ...) __attribute__((__availability__(android,__what __VA_OPT__(,) __VA_ARGS__))) #define __INTRODUCED_IN_NO_GUARD_FOR_NDK(api_level) __INTRODUCED_IN(api_level) #else -#define __BIONIC_AVAILABILITY(__what) __attribute__((__availability__(android,strict,__what))) +#define __BIONIC_AVAILABILITY(__what, ...) __attribute__((__availability__(android,strict,__what __VA_OPT__(,) __VA_ARGS__))) #define __INTRODUCED_IN_NO_GUARD_FOR_NDK(api_level) #endif #define __INTRODUCED_IN(api_level) __BIONIC_AVAILABILITY(introduced=api_level) -#define __DEPRECATED_IN(api_level) __BIONIC_AVAILABILITY(deprecated=api_level) -#define __REMOVED_IN(api_level) __BIONIC_AVAILABILITY(obsoleted=api_level) +#define __DEPRECATED_IN(api_level, ...) __BIONIC_AVAILABILITY(deprecated=api_level __VA_OPT__(,message=) __VA_ARGS__) +#define __REMOVED_IN(api_level, ...) __BIONIC_AVAILABILITY(obsoleted=api_level __VA_OPT__(,message=) __VA_ARGS__) // The same availability attribute can't be annotated multiple times. Therefore, the macros are // defined for the configuration that it is valid for so that declarations like the below doesn't @@ -80,5 +80,5 @@ // Vendor modules do not follow SDK versioning. Ignore NDK guards for vendor modules. #if defined(__ANDROID_VENDOR__) #undef __BIONIC_AVAILABILITY -#define __BIONIC_AVAILABILITY(x) +#define __BIONIC_AVAILABILITY(api_level, ...) #endif // defined(__ANDROID_VENDOR__) diff --git a/libc/include/stdio.h b/libc/include/stdio.h index 32264d6d9..78114c38c 100644 --- a/libc/include/stdio.h +++ b/libc/include/stdio.h @@ -69,7 +69,7 @@ extern FILE* _Nonnull stderr __INTRODUCED_IN(23); #define stderr stderr #else /* Before M the actual symbols for stdin and friends had different names. */ -extern FILE __sF[] __REMOVED_IN(23); +extern FILE __sF[] __REMOVED_IN(23, "Use stdin/stdout/stderr"); #define stdin (&__sF[0]) #define stdout (&__sF[1]) diff --git a/libc/system_properties/system_properties.cpp b/libc/system_properties/system_properties.cpp index 1dedb61e7..e0d38a822 100644 --- a/libc/system_properties/system_properties.cpp +++ b/libc/system_properties/system_properties.cpp @@ -337,31 +337,42 @@ int SystemProperties::Update(prop_info* pi, const char* value, unsigned int len) int SystemProperties::Add(const char* name, unsigned int namelen, const char* value, unsigned int valuelen) { - if (valuelen >= PROP_VALUE_MAX && !is_read_only(name)) { + if (namelen < 1) { + async_safe_format_log(ANDROID_LOG_ERROR, "libc", + "__system_property_add failed: name length 0"); return -1; } - if (namelen < 1) { + if (valuelen >= PROP_VALUE_MAX && !is_read_only(name)) { + async_safe_format_log(ANDROID_LOG_ERROR, "libc", + "__system_property_add failed: \"%s\" value too long: %d >= PROP_VALUE_MAX", + name, valuelen); return -1; } if (!initialized_) { + async_safe_format_log(ANDROID_LOG_ERROR, "libc", + "__system_property_add failed: properties not initialized"); return -1; } prop_area* serial_pa = contexts_->GetSerialPropArea(); if (serial_pa == nullptr) { + async_safe_format_log(ANDROID_LOG_ERROR, "libc", + "__system_property_add failed: property area not found"); return -1; } prop_area* pa = contexts_->GetPropAreaForName(name); if (!pa) { - async_safe_format_log(ANDROID_LOG_ERROR, "libc", "Access denied adding property \"%s\"", name); + async_safe_format_log(ANDROID_LOG_ERROR, "libc", + "__system_property_add failed: access denied for \"%s\"", name); return -1; } - bool ret = pa->add(name, namelen, value, valuelen); - if (!ret) { + if (!pa->add(name, namelen, value, valuelen)) { + async_safe_format_log(ANDROID_LOG_ERROR, "libc", + "__system_property_add failed: add failed for \"%s\"", name); return -1; } diff --git a/linker/linker_crt_pad_segment_test.cpp b/linker/linker_crt_pad_segment_test.cpp index 5a219f8ee..c11df50fe 100644 --- a/linker/linker_crt_pad_segment_test.cpp +++ b/linker/linker_crt_pad_segment_test.cpp @@ -72,13 +72,22 @@ bool GetPadSegment(const std::string& elf_path) { }; // anonymous namespace TEST(crt_pad_segment, note_absent) { + if (!page_size_migration_supported()) { + GTEST_SKIP() << "Kernel does not support page size migration"; + } ASSERT_FALSE(GetPadSegment("no_crt_pad_segment.so")); } TEST(crt_pad_segment, note_present_and_enabled) { + if (!page_size_migration_supported()) { + GTEST_SKIP() << "Kernel does not support page size migration"; + } ASSERT_TRUE(GetPadSegment("crt_pad_segment_enabled.so")); } TEST(crt_pad_segment, note_present_and_disabled) { + if (!page_size_migration_supported()) { + GTEST_SKIP() << "Kernel does not support page size migration"; + } ASSERT_FALSE(GetPadSegment("crt_pad_segment_disabled.so")); } diff --git a/linker/linker_phdr.cpp b/linker/linker_phdr.cpp index ef7671cee..fa712a10a 100644 --- a/linker/linker_phdr.cpp +++ b/linker/linker_phdr.cpp @@ -46,6 +46,8 @@ #include "private/CFIShadow.h" // For kLibraryAlignment #include "private/elf_note.h" +#include <android-base/file.h> + static int GetTargetElfMachine() { #if defined(__arm__) return EM_ARM; @@ -707,8 +709,28 @@ bool ElfReader::ReserveAddressSpace(address_space_params* address_space) { return true; } +/* + * Returns true if the kernel supports page size migration, else false. + */ +bool page_size_migration_supported() { + static bool pgsize_migration_enabled = []() { + std::string enabled; + if (!android::base::ReadFileToString("/sys/kernel/mm/pgsize_migration/enabled", &enabled)) { + return false; + } + return enabled.find("1") != std::string::npos; + }(); + return pgsize_migration_enabled; +} + // Find the ELF note of type NT_ANDROID_TYPE_PAD_SEGMENT and check that the desc value is 1. bool ElfReader::ReadPadSegmentNote() { + if (!page_size_migration_supported()) { + // Don't attempt to read the note, since segment extension isn't + // supported; but return true so that loading can continue normally. + return true; + } + // The ELF can have multiple PT_NOTE's, check them all for (size_t i = 0; i < phdr_num_; ++i) { const ElfW(Phdr)* phdr = &phdr_table_[i]; @@ -773,7 +795,16 @@ static inline void _extend_load_segment_vma(const ElfW(Phdr)* phdr_table, size_t const ElfW(Phdr)* next = nullptr; size_t next_idx = phdr_idx + 1; - if (phdr->p_align == kPageSize || !should_pad_segments) { + // Don't do segment extension for p_align > 64KiB, such ELFs already existed in the + // field e.g. 2MiB p_align for THPs and are relatively small in number. + // + // The kernel can only represent padding for p_align up to 64KiB. This is because + // the kernel uses 4 available bits in the vm_area_struct to represent padding + // extent; and so cannot enable mitigations to avoid breaking app compatibility for + // p_aligns > 64KiB. + // + // Don't perform segment extension on these to avoid app compatibility issues. + if (phdr->p_align <= kPageSize || phdr->p_align > 64*1024 || !should_pad_segments) { return; } @@ -887,10 +918,28 @@ bool ElfReader::LoadSegments() { // 2) Break the COW backing, faulting in new anon pages for a region // that will not be used. - // _seg_file_end = unextended seg_file_end - uint64_t _seg_file_end = seg_start + phdr->p_filesz; - if ((phdr->p_flags & PF_W) != 0 && page_offset(_seg_file_end) > 0) { - memset(reinterpret_cast<void*>(_seg_file_end), 0, kPageSize - page_offset(_seg_file_end)); + uint64_t unextended_seg_file_end = seg_start + phdr->p_filesz; + if ((phdr->p_flags & PF_W) != 0 && page_offset(unextended_seg_file_end) > 0) { + memset(reinterpret_cast<void*>(unextended_seg_file_end), 0, + kPageSize - page_offset(unextended_seg_file_end)); + } + + // Pages may be brought in due to readahead. + // Drop the padding (zero) pages, to avoid reclaim work later. + // + // NOTE: The madvise() here is special, as it also serves to hint to the + // kernel the portion of the LOAD segment that is padding. + // + // See: [1] https://android-review.googlesource.com/c/kernel/common/+/3032411 + // [2] https://android-review.googlesource.com/c/kernel/common/+/3048835 + uint64_t pad_start = page_end(unextended_seg_file_end); + uint64_t pad_end = page_end(seg_file_end); + CHECK(pad_start <= pad_end); + uint64_t pad_len = pad_end - pad_start; + if (page_size_migration_supported() && pad_len > 0 && + madvise(reinterpret_cast<void*>(pad_start), pad_len, MADV_DONTNEED)) { + DL_WARN("\"%s\": madvise(0x%" PRIx64 ", 0x%" PRIx64 ", MADV_DONTNEED) failed: %m", + name_.c_str(), pad_start, pad_len); } seg_file_end = page_end(seg_file_end); diff --git a/linker/linker_phdr.h b/linker/linker_phdr.h index 61242eb4c..aab9018b4 100644 --- a/linker/linker_phdr.h +++ b/linker/linker_phdr.h @@ -154,3 +154,5 @@ void phdr_table_get_dynamic_section(const ElfW(Phdr)* phdr_table, size_t phdr_co const char* phdr_table_get_interpreter_name(const ElfW(Phdr)* phdr_table, size_t phdr_count, ElfW(Addr) load_bias); + +bool page_size_migration_supported(); diff --git a/tests/clang_fortify_tests.cpp b/tests/clang_fortify_tests.cpp index 544af4308..f4ef4ac4a 100644 --- a/tests/clang_fortify_tests.cpp +++ b/tests/clang_fortify_tests.cpp @@ -164,9 +164,7 @@ FORTIFY_TEST(string) { const char large_string[] = "Hello!!!"; static_assert(sizeof(large_string) > sizeof(small_buffer), ""); -#if __clang_major__ > 13 - // expected-error@+3{{will always overflow}} -#endif + // expected-error@+2{{will always overflow}} // expected-error@+1{{string bigger than buffer}} EXPECT_FORTIFY_DEATH(strcpy(small_buffer, large_string)); // expected-error@+1{{string bigger than buffer}} @@ -204,9 +202,7 @@ FORTIFY_TEST(string) { static_assert(sizeof(small_string) > sizeof(split.tiny_buffer), ""); #if _FORTIFY_SOURCE > 1 -#if __clang_major__ > 13 - // expected-error@+4{{will always overflow}} -#endif + // expected-error@+3{{will always overflow}} // expected-error@+2{{string bigger than buffer}} #endif EXPECT_FORTIFY_DEATH_STRUCT(strcpy(split.tiny_buffer, small_string)); |