Adds support for the dynamic entries to specify MTE enablement. This is
now the preferred way for dynamically linked executables to specify to
the loader what mode MTE should be in, and whether stack MTE should be
enabled. In future, this is also needed for MTE globals support.
Leave the existing ELF note parsing as a backup option because dynamic
entries are not supported for fully static executables, and there's
still a bunch of glue sitting around in the build system and tests that
explicitly include the note. When -fsanitize=memtag* is specified, lld
will create the note implicitly (along with the new dynamic entries),
but at some point once we've cleaned up all the old references to the
note, we can remove the notegen from lld.
Bug: N/A
Test: atest bionic-unit-tests CtsBionicTestCases --test-filter=*Memtag*
Test: Build/boot the device under _fullmte.
Change-Id: I954b7e78afa5ff4274a3948b968cfad8eba94d88
The alignment of kShadowSize to a page sized multiple is
not explicitly needed, since mmap() will return a page-sized
multiple mapping.
kCfiCheckAlign remains 4k as this is chosen by the clang
compiler. [1] [2]
[1] 3568976375/clang/lib/CodeGen/CGExpr.cpp (L3433)
[2] https://clang.llvm.org/docs/ControlFlowIntegrityDesign.html#cfi-shadow
Bug: 296275298
Test: Boot 16kb device, check no cfi failures.
Test: atest -c bionic-unit-tests
Change-Id: Iac0c129c413afe01389f529f5c64051c4ffff2df
Signed-off-by: Kalesh Singh <kaleshsingh@google.com>
In the 4k targets there is no fucntional difference since
max_page_size() == page_size() == 4096.
On a 16kb device max_page_size() == 65536 and page_size() == 16384.
However, aligning up does not incur any memory regressions
since the .bss section is still be backes in page-sizeed chunks.
See: go/16k-page-aligned-variables
Bug: 296275298
Test: mma
Change-Id: I41c3e410f3b84c24eeb969c9aeca4b33a8d6170a
Signed-off-by: Kalesh Singh <kaleshsingh@google.com>
This lets us remove the riscv64 "sys/asm.h" file. It turns out everyone
loves this macro --- tons of x86 and arm assembler is already using it!
I'll clean up some of the now-duplicate definitions separately, and I'll
move the assembler we wrote ourselves over to this macro (rather than
the current `.L_foo` style) too.
Test: built riscv64 _and_ arm/arm64 _and_ x86/x86-64
Change-Id: If3f93c9b71094a8bed1fd1bb81bb83ec60ce409e
We've had these backward all this time. The relevant quote is in a
code comment in the implementation, but the first call after
completely decoding a code point that requires a surrogate pair should
return the number of bytes decoded by the most recent call, and the
second call should return -3 (if only C had given those some named
constants that might have been more obviously wrong).
Bug: https://issuetracker.google.com/289419882
Test: Fixed the test, tests run against glibc and musl to confirm
Change-Id: Idabf01075b1cad35b604ede8d676d6f0b1dc91e6
The magic numbers that C defines are obnoxious. We had partial
definitions for these internally. Add the missing one and move them to
a public header for anyone else that may want to use them.
Bug: None
Test: None
Change-Id: Ia6b8cff4310bcccb23078c52216528db668ac966
Not necessary (as demonstrated by the lack of this for x86), but this
saves one stack frame in aborts, which gets you one more useful stack
frame in logs and clustering etc, which improves your chances of finding
your bug.
Test: crasher64 abort
Change-Id: Ieb214f3b46520161edc1e53c0d766353b777d8ba
Also de-pessimize time(), where the vdso entrypoint only exists on
x86/x86-64 anyway.
Bug: https://github.com/google/android-riscv64/issues/8
Test: strace
Change-Id: I14cb2a3130b6ff88d06d43ea13d3a825a26de290
While looking at the disassembly for the epoll stuff I noticed that this
expands to quite a lot of code that the compiler can't optimize out for
LP64 (because it doesn't know that the "copy the argument into a local
and then use the local" bit isn't important).
There are two obvious options here. Something like this:
```
int signalfd64(int fd, const sigset64_t* mask, int flags) {
return __signalfd4(fd, mask, sizeof(*mask), flags);
}
int signalfd(int fd, const sigset_t* mask, int flags) {
#if defined(__LP64__)
return signalfd64(fd, mask, flags);
#else
SigSetConverter set = {.sigset = *mask};
return signalfd64(fd, &set.sigset64, flags);
#endif
}
```
Or something like this:
```
int signalfd64(int fd, const sigset64_t* mask, int flags) {
return __signalfd4(fd, mask, sizeof(*mask), flags);
}
#if defined(__LP64__)
__strong_alias(signalfd, signalfd64);
#else
int signalfd(int fd, const sigset_t* mask, int flags) {
SigSetConverter set = {};
set.sigset = *mask;
return signalfd64(fd, &set.sigset64, flags);
}
#endif
```
The former is slightly more verbose, but seems a bit more obvious, so I
initially went with that. (The former is more verbose in the generated
code too, given that the latter expands to _no_ code, just another symbol
pointing to the same code address.)
Having done that, I realized that slight changes to the interface would
let clang optimize away most/all of the overhead for LP64 with the only
preprocessor hackery being in SigSetConverter itself.
I also pulled out the legacy bsd `int` conversions since they're only
used in two (secret!) functions, so it's clearer to just have a separate
union for them. While doing so, I suppressed those functions for
riscv64, since there's no reason to keep carrying that mistake forward.
posix_spawn() is another simple case that doesn't actually benefit from
SigSetConverter, so I've given that its own anonymous union too.
Test: treehugger
Change-Id: Iaf67486da40d40fc53ec69717c3492ab7ab81ad6
shadowstack implicitly assumes that SCS_SIZE is a multiple of page size.
Currently, SCS_SIZE is set to 8K. This assumption is broken on 16K
platforms.
Test: launch_cvd --use_16k
Bug: 253652966
Bug: 279808236
Change-Id: I1180cfba32c98d638e18615ccfdc369beb390ea7
GWP-ASan's recoverable mode was landed upstream in
https://reviews.llvm.org/D140173.
This mode allows for a use-after-free or a buffer-overflow bug to be
detected by GWP-ASan, a crash report dumped, but then GWP-ASan (through
the preCrashReport() and postCrashReportRecoverableOnly() hooks) will
patch up the memory so that the process can continue, in spite of the
memory safety bug.
This is desirable, as it allows us to consider migrating non-system apps
from opt-in GWP-ASan to opt-out GWP-ASan. The major concern was "if we
make it opt-out, then bad apps will start crashing". If we don't crash,
problem solved :). Obviously, we'll need to do this with an amount of
process sampling to mitigate against the 70KiB memory overhead.
The biggest problem is that the debuggerd signal handler isn't the first
signal handler for apps, it's the sigchain handler inside of libart.
Clearly, the sigchain handler needs to ask us whether the crash is
GWP-ASan's fault, and if so, please patch up the allocator. Because of
linker namespace restrictions, libart can't directly ask the linker
(which is where debuggerd lies), so we provide a proxy function in libc.
Test: Build the platform, run sanitizer-status and various test apps
with recoverable gwp-asan. Assert that it doesn't crash, and we get a
debuggerd report.
Bug: 247012630
Change-Id: I86d5e27a9ca5531c8942e62647fd377c3cd36dfd
This is a no-op but will be used in upcoming scudo changes that allow to
change the buffer size at process startup time, and as such we will no
longer be able to call __scudo_get_ring_buffer_size in debuggerd.
Bug: 263287052
Change-Id: I18f166fc136ac8314d748eb80a806defcc25c9fd
I don't think we need or want to do this. Most other architectures
don't.
Test: bionic-unit-tests-static
Change-Id: I3ad31926909caf0a37e73ac6cbac1fecd02ea6de
Map all stacks (primary, thread, and sigaltstack) as PROT_MTE when the
binary requests it through the ELF note.
For the reference, the note is produced by the following toolchain changes:
https://reviews.llvm.org/D118948https://reviews.llvm.org/D119384https://reviews.llvm.org/D119381
Bug: b/174878242
Test: fvp_mini with ToT LLVM (more tests in a separate change)
Change-Id: I04a4e21c966e7309b47b1f549a2919958d93a872
Currently, the ELF note parsing in the loader is not permissive. This
patch relaxes the restrictions on the bits of the ELF note that could be
extended in the future.
This may allow more MTE options in apexes. If we add some extra metadata
bit (say, in bit 5) in the future, and then build MTE into every apex,
we don't want it to crash on Android 13 devices (we'd much rather it
just be a no-op).
Remove the (unused anywhere) NT_MEMTAG_DEFAULT and call it
NT_MEMTAG_NONE.
And finally, make the tests work on bionic-unit-tests-static. We
previously didn't deploy the test binaries, so add them as a data
dependency.
Bug: N/A
Test: atest bionic-unit-tests-static
Change-Id: I13530faad55c719c6eb848297f8ce378e18afbfc
Normally, platform-specific note types in the toolchain are prefixed
with the platform name. Because we're exposing the NT_TYPE_MEMTAG and
synthesizing the note in the toolchain in an upcoming patch
(https://reviews.llvm.org/D118948), it's been requested that we change
the name to include the platform prefix.
While NT_TYPE_IDENT and NT_TYPE_KUSER aren't known about or synthesized
by the toolchain, update those references as well for consistency.
Bug: N/A
Test: Build Android
Change-Id: I7742e4917ae275d59d7984991664ea48028053a1
The first time should_trace() returns true, bionic_trace_begin() calls
open() on trace_marker.
The problem is that open() can call bionic_trace_begin(). We've observed
this happening, for example when:
* fdtrack is enabled. dlopen("libfdtrack.so") can be used to enable
fdtrack on a process.
* ThreadA is busy unwinding inside fdtrack and is holding an fdtrack
internal mutex.
* ThreadB calls bionic_trace_begin() for the first time since the
property "debug.atrace.tags.enableflags" contains ATRACE_TAG_BIONIC.
* ThreadB calls open("/sys/kernel/tracing/trace_marker"). Since fdtrack
is enabled, ThreadB tries to do unwinding as well.
* ThreadB, inside fdtrack's unwinding tries to grab the same mutex that
ThreadA is holding.
* Mutex contention is reported using bionic_systrace, therefore
bionic_trace_begin() is called again on ThreadB.
* ThreadB tries to grab g_lock in bionin_systrace.cpp, but that's
already held by ThreadB itself, earlier on the stack. Therefore
ThreadB is stuck.
I managed to reproduce the above scenario by manually pausing ThreadA
inside unwinding with a debugger and letting ThreadB hitting
bionic_trace_begin() for the first time.
We could avoid using g_lock while calling open() (either by releasing
g_lock and reacquiring it later, or by using atomics), but
bionic_trace_begin() would try to call open() again. In my tests, open()
does not call bionic_trace_begin() a third time, because fdtrack has
reentrancy protection, but there might be another code path inside open
that calls bionic_trace_begin again (it could be racy or only happen in
certain configurations).
This commit fixes the problem by implementing reentrancy protection in
bionic_systrace.
Sample callstack from ThreadA deadlocked before the fix:
```
* frame #0: 0x0000007436db077c libc.so`syscall at syscall.S:41
frame #1: 0x0000007436db0ba0 libc.so`bionic_trace_begin(char const*) [inlined] __futex(ftx=0x000000743737a548, op=<unavailable>, value=2, timeout=0x0000000000000000, bitset=-1) at bionic_futex.h:45:16
frame #2: 0x0000007436db0b8c libc.so`bionic_trace_begin(char const*) [inlined] __futex_wait_ex(ftx=0x000000743737a548, value=2) at bionic_futex.h:66:10
frame #3: 0x0000007436db0b78 libc.so`bionic_trace_begin(char const*) [inlined] Lock::lock(this=0x000000743737a548) at bionic_lock.h:67:7
frame #4: 0x0000007436db0b74 libc.so`bionic_trace_begin(char const*) [inlined] should_trace() at bionic_systrace.cpp:38:10
frame #5: 0x0000007436db0b74 libc.so`bionic_trace_begin(message="Contending for pthread mutex") at bionic_systrace.cpp:59:8
frame #6: 0x0000007436e193e4 libc.so`NonPI::MutexLockWithTimeout(pthread_mutex_internal_t*, bool, timespec const*) [inlined] NonPI::NormalMutexLock(mutex=0x0000007296cae9f0, shared=0, use_realtime_clock=false, abs_timeout_or_null=0x0000000000000000) at pthread_mutex.cpp:592:17
frame #7: 0x0000007436e193c8 libc.so`NonPI::MutexLockWithTimeout(mutex=0x0000007296cae9f0, use_realtime_clock=false, abs_timeout_or_null=0x0000000000000000) at pthread_mutex.cpp:719:16
frame #8: 0x0000007436e1912c libc.so`::pthread_mutex_lock(mutex_interface=<unavailable>) at pthread_mutex.cpp:839:12 [artificial]
frame #9: 0x00000071a4e5b290 libfdtrack.so`std::__1::mutex::lock() [inlined] std::__1::__libcpp_mutex_lock(__m=<unavailable>) at __threading_support:256:10
frame #10: 0x00000071a4e5b28c libfdtrack.so`std::__1::mutex::lock(this=<unavailable>) at mutex.cpp:31:14
frame #11: 0x00000071a4e32634 libfdtrack.so`unwindstack::Elf::Step(unsigned long, unwindstack::Regs*, unwindstack::Memory*, bool*, bool*) [inlined] std::__1::lock_guard<std::__1::mutex>::lock_guard(__m=0x0000007296cae9f0) at __mutex_base:104:27
frame #12: 0x00000071a4e32618 libfdtrack.so`unwindstack::Elf::Step(this=0x0000007296cae9c0, rel_pc=66116, regs=0x0000007266ca0470, process_memory=0x0000007246caa130, finished=0x0000007ff910efb4, is_signal_frame=0x0000007ff910efb0) at Elf.cpp:206:31
frame #13: 0x00000071a4e2b3b0 libfdtrack.so`unwindstack::LocalUnwinder::Unwind(this=0x00000071a4ea1528, frame_info=<unavailable>, max_frames=34) at LocalUnwinder.cpp:102:22
frame #14: 0x00000071a4e2a3ec libfdtrack.so`fd_hook(event=<unavailable>) at fdtrack.cpp:119:18
frame #15: 0x0000007436dbf684 libc.so`::__open_2(pathname=<unavailable>, flags=<unavailable>) at open.cpp:72:10
frame #16: 0x0000007436db0a04 libc.so`bionic_trace_begin(char const*) [inlined] open(pathname=<unavailable>, flags=524289) at fcntl.h:63:12
frame #17: 0x0000007436db09f0 libc.so`bionic_trace_begin(char const*) [inlined] get_trace_marker_fd() at bionic_systrace.cpp:49:25
frame #18: 0x0000007436db09c0 libc.so`bionic_trace_begin(message="pthread_create") at bionic_systrace.cpp:63:25
```
Bug: 213642769
Change-Id: I10d331859045cb4a8609b007f5c6cf2577ff44df
musl libc doesn't provide fts, but elfutils and libabigail need it.
Export bionic's fts as a staic library that can be linked into elfutils
and libabigail when compiling against musl.
fts uses recallocarray, which musl doesn't provide, so also include
recallocarray.c in libfts.a.
Requires minor tweaks to fts.c and a wrapper around fts.h to make them
compatible with musl, primarily by providing local definitions of macros
provided in bionic's sys/cdefs.h.
Bug: 190084016
Test: m libfts
Change-Id: Ifac9a59e7504c0c1f5f8a3a5bd3c19a13980b83c
Revert submission 1403568-sysprop_trace
Reason for revert: makes property get/set non-reentrant
Reverted Changes:
I6f85f3f52:Add systrace tag for system property
Id2b93acb2:Adding system property tracing
Id78992d23:Add systrace tag for system property
I1ba9fc7bd:Add systrace tag for system property
Bug: 193050299
Test: build and boot a device
Change-Id: Ic7a83fb01a39113d408ed0c95d27f694d5a2649c
Merged-In: Ic7a83fb01a39113d408ed0c95d27f694d5a2649c
(cherry picked from commit 61a5a8380d)
Also delete some fdsan code that attempts to check for the post-fork
state, but never will, because we update the cached pid upon fork.
Bug: http://b/174542867
Test: /data/nativetest64/bionic-unit-tests/bionic-unit-tests
Test: treehugger
Change-Id: I9b748dac9de9b4c741897d93e64d31737e52bf8e
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
This matches what we do for arm and arm64. 32-bit x86 is too big a mess
to warrant the effort still, but the more testing is done on cuttlefish,
the more value there is to making every stack frame count.
Before:
#00 pc 00000000000596d8 .../libc.so (syscall+24)
#01 pc 000000000005d072 .../libc.so (abort+194)
#02 pc 000000000005f1f0 .../libc.so (__fortify_fatal(char const*, ...)+160)
After:
#00 pc 000000000005d07d .../libc.so (abort+205)
#01 pc 000000000005f1e0 .../libc.so (__fortify_fatal(char const*, ...)+160)
Test: crasher64 fortify
Change-Id: Ib74cb8b36341093c268872e26020f35eb2d8ef66
Bug: http://b/157081822
If __libc_int0x80 is in a C/C++ file, Clang's coverage instrumentation
adds instructions to count the number of times it gets executed [1].
With coverage instrumentation, __libc_sysinfo, used on 32-bit x86, is
initialized to the wrong value, causing dl.preinit_system_calls to fail.
Moving the function to an assembly file leaves __libc_sysinfo properly
initialized.
[1] We could change clang so it doesn't instrument functions marked
__attribute__((naked)) as a followup.
Test: `m CLANG_COVERAGE=true NATIVE_COVERAGE_PATHS=bionic` and run
bionic-unit-tests
Change-Id: I73558253512392d345de8d5b66d38bb14b308fdf
With this change we can report memory errors involving secondary
allocations. Update the existing crasher tests to also test
UAF/overflow/underflow on allocations with sizes sufficient to trigger
the secondary allocator.
Bug: 135772972
Change-Id: Ic8925c1f18621a8f272e26d5630e5d11d6d34d38
"#" there is incorrect: macro parameter is expected following a hashmark.
Test: build libc with -mbranch-protection=standard
Change-Id: Ib8e7ddf260b4cdbd36246cc70f69970f33dee200
Technically, std::atomic does not have an operator(). Previously, this
code was relying on an implicit behavior of our C++ standard library in
order to compile. When compiling this code against a different C++
standard library, I encountered a compiler error on these lines.
This CL makes the std::atomic load() operation explicit, makes it
clearer what this code is actually doing and makes it conform better to
the C++ standard library spec rather than a particular implmentation.
Change-Id: I7f255dffc0a3d8e07c973c18e9ba4098c4b5843e
Use a note in executables to specify
(none|sync|async) heap tagging level. To be extended with (heap x stack x
globals) in the future. A missing note disables all tagging.
Bug: b/135772972
Test: bionic-unit-tests (in a future change)
Change-Id: Iab145a922c7abe24cdce17323f9e0c1063cc1321
In native build of libc it would be inlined and in native bridge mode
it's noinline, extern "C" and thus could be easily intercepted.
Test: m (without weak symbols in native bridge mode x86+arm build would be broken)
Change-Id: I67759858a5bc2174dce1db9732fdbd89ba7689cc
Now, when we can detect native bridge mode is sources we can do that.
Test: m (without weak symbols in native bridge mode x86+arm build would be broken)
Change-Id: I360e7df8211d03636bbe716dc14655ee8d765493
