Merge "[GWP-ASan] Fix bugs in realloc()."
diff --git a/OWNERS b/OWNERS
index e02ad9f..670f88d 100644
--- a/OWNERS
+++ b/OWNERS
@@ -2,9 +2,5 @@
cferris@google.com
danalbert@google.com
-hhb@google.com
rprichard@google.com
yabinc@google.com
-
-# Still the best reviewer for changes related to the dynamic linker.
-dimitry@google.com
diff --git a/README.md b/README.md
index d96608e..f397fee 100644
--- a/README.md
+++ b/README.md
@@ -147,11 +147,11 @@
The answer is "yes" if the system call is part of the POSIX standard.
The answer is probably "yes" if the system call has a wrapper in at
-least one other C library.
+least one other C library (typically glibc/musl or Apple's libc).
The answer may be "yes" if the system call has three/four distinct
-users in different projects, and there isn't a more specific library
-that would make more sense as the place to add the wrapper.
+users in different projects, and there isn't a more specific higher-level
+library that would make more sense as the place to add the wrapper.
In all other cases, you should use
[syscall(3)](http://man7.org/linux/man-pages/man2/syscall.2.html) instead.
@@ -166,14 +166,47 @@
the appropriate POSIX header file in libc/include/ includes the
relevant file or files.
3. Add function declarations to the appropriate header file. Don't forget
- to include the appropriate `__INTRODUCED_IN()`.
- 4. Add the function name to the correct section in libc/libc.map.txt.
- 5. Add at least basic tests. Even a test that deliberately supplies
- an invalid argument helps check that we're generating the right symbol
- and have the right declaration in the header file, and that you correctly
- updated the maps in step 5. (You can use strace(1) to confirm that the
- correct system call is being made.)
+ to include the appropriate `__INTRODUCED_IN()`. If you need to create a new
+ header file, libc/include/sys/sysinfo.h is a good short example to copy and
+ paste from.
+ 4. Add basic documentation to the header file. libc/include/sys/sysinfo.h is a
+ good short example that shows the expected style. Most of the detail
+ should actually be left to the man7.org page, with only a brief
+ one-sentence explanation in our documentation. Alway include the return
+ value/error reporting details. Explicitly say which version of Android the
+ function was added to. Explicitly call out any Android-specific
+ changes/additions/limitations because they won't be on the man7.org page.
+ 5. Add the function name to the correct section in libc/libc.map.txt.
+ 6. Add a basic test. Don't try to test everything; concentrate on just testing
+ the code that's actually in *bionic*, not all the functionality that's
+ implemented in the kernel. For simple syscalls, that's just the
+ auto-generated argument and return value marshalling.
+ A trivial test that deliberately supplies an invalid argument helps check
+ that we're generating the right symbol and have the right declaration in
+ the header file, and that the change to libc.map.txt from step 5 is
+ correct. (You can use strace(1) manually to confirm that the correct
+ system call is being made.)
+
+ For testing the *kernel* side of things, we should prefer to rely on
+ https://github.com/linux-test-project/ltp for kernel testing, but you'll
+ want to check that external/ltp does contain tests for the syscall you're
+ adding. Also check that external/ltp is using the libc wrapper for the
+ syscall rather than calling it "directly" via syscall(3)!
+
+Some system calls are harder than others. The most common problem is a 64-bit
+argument such as `off64_t` (a *pointer* to a 64-bit argument is fine, since
+pointers are always the "natural" size for the architecture regardless of the
+size of the thing they point to). Whenever you have a function that takes
+`off_t` or `off64_t`, you'll need to consider whether you actually need a foo()
+and a foo64(), and whether they will use the same underlying system call or are
+implemented as two different system calls. It's usually easiest to find a
+similar system call and copy and paste from that. You'll definitely need to test
+both on 32-bit and 64-bit. (These special cases warrant more testing than the
+easy cases, even if only manual testing with strace. Sadly it isn't always
+feasible to write a working test for the interesting cases -- offsets larger
+than 2GiB, say -- so you may end up just writing a "meaningless" program whose
+only purpose is to give you patterns to look for when run under strace(1).)
## Updating kernel header files
diff --git a/TEST_MAPPING b/TEST_MAPPING
index 8b01c4e..65fc12b 100644
--- a/TEST_MAPPING
+++ b/TEST_MAPPING
@@ -13,6 +13,9 @@
"name": "CtsBionicTestCases"
},
{
+ "name": "CtsGwpAsanTestCases"
+ },
+ {
"name": "CtsTaggingHostTestCases"
},
{
diff --git a/libc/async_safe/Android.bp b/libc/async_safe/Android.bp
index e4a5837..531317d 100644
--- a/libc/async_safe/Android.bp
+++ b/libc/async_safe/Android.bp
@@ -36,6 +36,7 @@
"com.android.art.debug",
"com.android.media",
"com.android.media.swcodec",
+ "com.android.virt",
],
min_sdk_version: "apex_inherit",
}
diff --git a/libc/include/android/legacy_signal_inlines.h b/libc/include/android/legacy_signal_inlines.h
index 95c2320..f2bdcf6 100644
--- a/libc/include/android/legacy_signal_inlines.h
+++ b/libc/include/android/legacy_signal_inlines.h
@@ -89,7 +89,7 @@
errno = EINVAL;
return -1;
}
- return (int)((local_set[bit / LONG_BIT] >> (bit % LONG_BIT)) & 1);
+ return (int)((local_set[bit / (8 * sizeof(long))] >> (bit % (8 * sizeof(long)))) & 1);
}
static __inline int sigaddset(sigset_t *set, int signum) {
@@ -100,7 +100,7 @@
errno = EINVAL;
return -1;
}
- local_set[bit / LONG_BIT] |= 1UL << (bit % LONG_BIT);
+ local_set[bit / (8 * sizeof(long))] |= 1UL << (bit % (8 * sizeof(long)));
return 0;
}
@@ -112,7 +112,7 @@
errno = EINVAL;
return -1;
}
- local_set[bit / LONG_BIT] &= ~(1UL << (bit % LONG_BIT));
+ local_set[bit / (8 * sizeof(long))] &= ~(1UL << (bit % (8 * sizeof(long))));
return 0;
}
diff --git a/libc/include/bits/signal_types.h b/libc/include/bits/signal_types.h
index e1a155f..699e257 100644
--- a/libc/include/bits/signal_types.h
+++ b/libc/include/bits/signal_types.h
@@ -61,7 +61,7 @@
#if defined(__LP64__)
typedef sigset_t sigset64_t;
#else
-typedef struct { unsigned long __bits[_KERNEL__NSIG/LONG_BIT]; } sigset64_t;
+typedef struct { unsigned long __bits[_KERNEL__NSIG/(8*sizeof(long))]; } sigset64_t;
#endif
#if defined(__LP64__)
diff --git a/libc/malloc_debug/Android.bp b/libc/malloc_debug/Android.bp
index f779b73..c7c88e1 100644
--- a/libc/malloc_debug/Android.bp
+++ b/libc/malloc_debug/Android.bp
@@ -133,6 +133,7 @@
include_dirs: [
"bionic/libc",
"bionic/libc/async_safe/include",
+ "bionic", // For SKIP_WITH_HWASAN.
],
header_libs: [
diff --git a/libc/malloc_debug/tests/malloc_debug_unit_tests.cpp b/libc/malloc_debug/tests/malloc_debug_unit_tests.cpp
index 46de3e9..84f0645 100644
--- a/libc/malloc_debug/tests/malloc_debug_unit_tests.cpp
+++ b/libc/malloc_debug/tests/malloc_debug_unit_tests.cpp
@@ -43,6 +43,7 @@
#include <platform/bionic/macros.h>
#include <private/bionic_malloc_dispatch.h>
+#include <tests/utils.h>
#include <unwindstack/Unwinder.h>
@@ -2063,6 +2064,7 @@
}
TEST_F(MallocDebugTest, debug_mallinfo) {
+ SKIP_WITH_HWASAN;
Init("guard");
void* pointer = debug_malloc(150);
@@ -2475,6 +2477,7 @@
}
TEST_F(MallocDebugTest, malloc_info_no_pointer_tracking) {
+ SKIP_WITH_HWASAN;
Init("fill");
TemporaryFile tf;
diff --git a/libc/malloc_hooks/tests/malloc_hooks_tests.cpp b/libc/malloc_hooks/tests/malloc_hooks_tests.cpp
index ca064c2..d16270f 100644
--- a/libc/malloc_hooks/tests/malloc_hooks_tests.cpp
+++ b/libc/malloc_hooks/tests/malloc_hooks_tests.cpp
@@ -178,6 +178,7 @@
}
TEST_F(MallocHooksTest, other_malloc_functions) {
+ SKIP_WITH_HWASAN; // HWASan does not implement mallinfo.
RunTest("*.DISABLED_other_malloc_functions");
}
diff --git a/linker/arch/arm_neon/linker_gnu_hash_neon.cpp b/linker/arch/arm_neon/linker_gnu_hash_neon.cpp
index 11cf5a3..159c66e 100644
--- a/linker/arch/arm_neon/linker_gnu_hash_neon.cpp
+++ b/linker/arch/arm_neon/linker_gnu_hash_neon.cpp
@@ -81,6 +81,8 @@
// return h;
// }
//
+// This does an within-alignment out-of-bounds read for performance reasons.
+__attribute__((no_sanitize("hwaddress")))
std::pair<uint32_t, uint32_t> calculate_gnu_hash_neon(const char* name) {
// The input string may be misaligned by 0-7 bytes (K). This function loads the first aligned
diff --git a/linker/linker.cpp b/linker/linker.cpp
index 3488f5c..c6588d2 100644
--- a/linker/linker.cpp
+++ b/linker/linker.cpp
@@ -3351,18 +3351,15 @@
}
// Given an `executable_path` starting with "/apex/<name>/bin/, return
-// "/linkerconfig/<name>/ld.config.txt" (or "/apex/<name>/etc/ld.config.txt", if
-// the former does not exist).
+// "/linkerconfig/<name>/ld.config.txt", which is the auto-generated config file for the APEX by the
+// linkerconfig tool.
static std::string get_ld_config_file_apex_path(const char* executable_path) {
std::vector<std::string> paths = android::base::Split(executable_path, "/");
if (paths.size() >= 5 && paths[1] == "apex" && paths[3] == "bin") {
- // Check auto-generated ld.config.txt first
std::string generated_apex_config = "/linkerconfig/" + paths[2] + "/ld.config.txt";
if (file_exists(generated_apex_config.c_str())) {
return generated_apex_config;
}
-
- return std::string("/apex/") + paths[2] + "/etc/ld.config.txt";
}
return "";
}
diff --git a/linker/linker_relocate.cpp b/linker/linker_relocate.cpp
index 72be5f7..c7c7bfb 100644
--- a/linker/linker_relocate.cpp
+++ b/linker/linker_relocate.cpp
@@ -229,6 +229,12 @@
auto get_addend_norel = [&]() -> ElfW(Addr) { return 0; };
#endif
+ if (!IsGeneral && __predict_false(is_tls_reloc(r_type))) {
+ // Always process TLS relocations using the slow code path, so that STB_LOCAL symbols are
+ // diagnosed, and ifunc processing is skipped.
+ return process_relocation_general(relocator, reloc);
+ }
+
if (IsGeneral && is_tls_reloc(r_type)) {
if (r_sym == 0) {
// By convention in ld.bfd and lld, an omitted symbol on a TLS relocation
@@ -242,8 +248,15 @@
// - https://groups.google.com/d/topic/generic-abi/dJ4_Y78aQ2M/discussion
// - https://sourceware.org/bugzilla/show_bug.cgi?id=17699
sym = &relocator.si_symtab[r_sym];
- DL_ERR("unexpected TLS reference to local symbol \"%s\" in \"%s\": sym type %d, rel type %u",
- sym_name, relocator.si->get_realpath(), ELF_ST_TYPE(sym->st_info), r_type);
+ auto sym_type = ELF_ST_TYPE(sym->st_info);
+ if (sym_type == STT_SECTION) {
+ DL_ERR("unexpected TLS reference to local section in \"%s\": sym type %d, rel type %u",
+ relocator.si->get_realpath(), sym_type, r_type);
+ } else {
+ DL_ERR(
+ "unexpected TLS reference to local symbol \"%s\" in \"%s\": sym type %d, rel type %u",
+ sym_name, relocator.si->get_realpath(), sym_type, r_type);
+ }
return false;
} else if (!lookup_symbol<IsGeneral>(relocator, r_sym, sym_name, &found_in, &sym)) {
return false;
diff --git a/tests/Android.bp b/tests/Android.bp
index 3061142..48149c7 100644
--- a/tests/Android.bp
+++ b/tests/Android.bp
@@ -295,6 +295,29 @@
},
}
+cc_prebuilt_test_library_shared {
+ name: "libtest_invalid-local-tls",
+ strip: {
+ none: true,
+ },
+ check_elf_files: false,
+ relative_install_path: "bionic-loader-test-libs/prebuilt-elf-files",
+ arch: {
+ arm: {
+ srcs: ["prebuilt-elf-files/arm/libtest_invalid-local-tls.so"],
+ },
+ arm64: {
+ srcs: ["prebuilt-elf-files/arm64/libtest_invalid-local-tls.so"],
+ },
+ x86: {
+ srcs: ["prebuilt-elf-files/x86/libtest_invalid-local-tls.so"],
+ },
+ x86_64: {
+ srcs: ["prebuilt-elf-files/x86_64/libtest_invalid-local-tls.so"],
+ },
+ },
+}
+
// -----------------------------------------------------------------------------
// All standard tests.
// -----------------------------------------------------------------------------
@@ -994,6 +1017,7 @@
"libtest_init_fini_order_root",
"libtest_init_fini_order_root2",
"libtest_invalid-empty_shdr_table",
+ "libtest_invalid-local-tls",
"libtest_invalid-rw_load_segment",
"libtest_invalid-textrels",
"libtest_invalid-textrels2",
diff --git a/tests/dlfcn_test.cpp b/tests/dlfcn_test.cpp
index e3664fd..940e726 100644
--- a/tests/dlfcn_test.cpp
+++ b/tests/dlfcn_test.cpp
@@ -1654,6 +1654,21 @@
ASSERT_SUBSTR(expected_dlerror.c_str(), dlerror());
}
+TEST(dlfcn, dlopen_invalid_local_tls) {
+ const std::string libpath = GetPrebuiltElfDir() + "/libtest_invalid-local-tls.so";
+
+ void* handle = dlopen(libpath.c_str(), RTLD_NOW);
+ ASSERT_TRUE(handle == nullptr);
+#if defined(__arm__)
+ const char* referent = "local section";
+#else
+ const char* referent = "local symbol \"tls_var_2\"";
+#endif
+ std::string expected_dlerror = std::string("dlopen failed: unexpected TLS reference to ") +
+ referent + " in \"" + libpath + "\"";
+ ASSERT_SUBSTR(expected_dlerror.c_str(), dlerror());
+}
+
TEST(dlfcn, dlopen_df_1_global) {
void* handle = dlopen("libtest_dlopen_df_1_global.so", RTLD_NOW);
ASSERT_TRUE(handle != nullptr) << dlerror();
diff --git a/tests/headers/posix/stdio_h.c b/tests/headers/posix/stdio_h.c
index cbeb0bc..57be0a0 100644
--- a/tests/headers/posix/stdio_h.c
+++ b/tests/headers/posix/stdio_h.c
@@ -110,7 +110,8 @@
FUNCTION(getchar_unlocked, int (*f)(void));
FUNCTION(getdelim, ssize_t (*f)(char**, size_t*, int, FILE*));
FUNCTION(getline, ssize_t (*f)(char**, size_t*, FILE*));
- FUNCTION(gets, char* (*f)(char*));
+ // gets() was removed in C11.
+ // FUNCTION(gets, char* (*f)(char*));
FUNCTION(open_memstream, FILE* (*f)(char**, size_t*));
FUNCTION(pclose, int (*f)(FILE*));
FUNCTION(perror, void (*f)(const char*));
diff --git a/tests/prebuilt-elf-files/arm/libtest_invalid-local-tls.so b/tests/prebuilt-elf-files/arm/libtest_invalid-local-tls.so
new file mode 100755
index 0000000..a42848d
--- /dev/null
+++ b/tests/prebuilt-elf-files/arm/libtest_invalid-local-tls.so
Binary files differ
diff --git a/tests/prebuilt-elf-files/arm64/libtest_invalid-local-tls.so b/tests/prebuilt-elf-files/arm64/libtest_invalid-local-tls.so
new file mode 100755
index 0000000..20c5765
--- /dev/null
+++ b/tests/prebuilt-elf-files/arm64/libtest_invalid-local-tls.so
Binary files differ
diff --git a/tests/prebuilt-elf-files/gen-libtest_invalid-local-tls.sh b/tests/prebuilt-elf-files/gen-libtest_invalid-local-tls.sh
new file mode 100755
index 0000000..0f3e736
--- /dev/null
+++ b/tests/prebuilt-elf-files/gen-libtest_invalid-local-tls.sh
@@ -0,0 +1,30 @@
+#!/bin/bash
+#
+# Bionic doesn't support the references to STB_LOCAL symbols of type STT_TLS
+# and STT_SECTION that ld.gold generates. Set NDK21E to the path to a copy of
+# NDK r21e, which still has ld.gold (unlike the platform build or newer NDKs).
+
+set -e
+
+cat >test.c <<EOF
+ static __thread int tls_var_1;
+ extern __thread int tls_var_2;
+ int* getaddr1() { return &tls_var_1; }
+ int* getaddr2() { return &tls_var_2; }
+EOF
+cat >test2.c <<EOF
+ __attribute__((visibility("hidden"))) __thread int tls_var_2;
+EOF
+
+build() {
+ arch=$1
+ target=$2
+ $NDK21E/toolchains/llvm/prebuilt/linux-x86_64/bin/clang -O2 --target=$target \
+ -fpic -shared -o $arch/libtest_invalid-local-tls.so -fno-emulated-tls \
+ -fuse-ld=gold test.c test2.c
+}
+
+build arm armv7a-linux-androideabi29
+build arm64 aarch64-linux-android29
+build x86 i686-linux-android29
+build x86_64 x86_64-linux-android29
diff --git a/tests/prebuilt-elf-files/x86/libtest_invalid-local-tls.so b/tests/prebuilt-elf-files/x86/libtest_invalid-local-tls.so
new file mode 100755
index 0000000..879f6a1
--- /dev/null
+++ b/tests/prebuilt-elf-files/x86/libtest_invalid-local-tls.so
Binary files differ
diff --git a/tests/prebuilt-elf-files/x86_64/libtest_invalid-local-tls.so b/tests/prebuilt-elf-files/x86_64/libtest_invalid-local-tls.so
new file mode 100755
index 0000000..5b689ba
--- /dev/null
+++ b/tests/prebuilt-elf-files/x86_64/libtest_invalid-local-tls.so
Binary files differ
diff --git a/tests/unistd_test.cpp b/tests/unistd_test.cpp
index c306a08..6d7e687 100644
--- a/tests/unistd_test.cpp
+++ b/tests/unistd_test.cpp
@@ -1529,11 +1529,21 @@
}
TEST(UNISTD_TEST, exec_argv0_null) {
- // http://b/33276926
+ // http://b/33276926 and http://b/227498625.
+ //
+ // With old kernels, bionic will see the null pointer and use "<unknown>" but
+ // with new (5.18+) kernels, the kernel will already have substituted the
+ // empty string, so we don't make any assertion here about what (if anything)
+ // comes before the first ':'.
+ //
+ // If this ever causes trouble, we could change bionic to replace _either_ the
+ // null pointer or the empty string. We could also use the actual name from
+ // readlink() on /proc/self/exe if we ever had reason to disallow programs
+ // from trying to hide like this.
char* args[] = {nullptr};
char* envs[] = {nullptr};
ASSERT_EXIT(execve("/system/bin/run-as", args, envs), testing::ExitedWithCode(1),
- "<unknown>: usage: run-as");
+ ": usage: run-as");
}
TEST(UNISTD_TEST, fexecve_failure) {