Add a hook that's called upon file descriptor creation to libc, and a
library that uses it to capture backtraces for file descriptor creation,
to make it easier to hunt down file descriptor leaks.
Currently, this doesn't capture all of the ways of creating a file
descriptor, but completeness isn't required for this to be useful as
long as leaked file descriptors are created with a function that is
tracked. The primary unhandled case is binder, which receives file
descriptors as a payload in a not-trivially-parsable byte blob, but
there's a chance that the leak we're currently trying to track down
isn't of a file descriptor received over binder, so leave that for
later.
Bug: http://b/140703823
Test: manual
Change-Id: I308a14c2e234cdba4207157b634ab6b8bc539dd9
(cherry picked from commit b7eccd4b15)
This patch introduces tagged pointers to bionic. We add a static tag to
all pointers on arm64 compatible platforms (needs requisite
top-byte-ignore hardware feature and relevant kernel patches).
We dynamically detect TBI-compatible devices (a device with the TBI feature and
kernel support) at process start time, and insert an implementation-dependent
tag into the top byte of the pointer for all heap allocations. We then check
that the tag has not been truncated when deallocating the memory.
If an application incorrectly writes to the top byte of the pointer, we
terminate the process at time of detection. This will allow MTE-incompatible
applications to be caught early.
Bug: 135754954
Bug: 147147490
Test: cd bionic && atest .
Change-Id: Ie424325ba1e3c4443040ac265aeaa28d9e405d28
This reverts commit 43d5f9d4dd.
Bug: 135754954
Bug: 147147490
Exempt-From-Owner-Approval: clean revert
Reason for revert: Breaks ART gtest, see:
https://ci.chromium.org/p/art/builders/ci/angler-armv8-non-gen-cc/561
The crash happens on mprotect of a page, the test crashes with ENOMEM.
Change-Id: I52eea1abbfaf8d8e2226f92d30aa55aba3810528
This patch introduces tagged pointers to bionic. We add a static tag to
all pointers on arm64 compatible platforms (needs requisite
top-byte-ignore hardware feature and relevant kernel patches).
We dynamically detect TBI-compatible devices (a device with the TBI feature and
kernel support) at process start time, and insert an implementation-dependent
tag into the top byte of the pointer for all heap allocations. We then check
that the tag has not been truncated when deallocating the memory.
If an application incorrectly writes to the top byte of the pointer, we
terminate the process at time of detection. This will allow MTE-incompatible
applications to be caught early.
Bug: 135754954
Bug: 147147490
Test: cd bionic && atest .
Change-Id: I6e5b809fc81f55dd517f845eaf20f3c0ebd4d86e
This patch refactors heapprofd_malloc to make it easier to reuse the
reserved signal for multiple purposes. We define a new generic signal
handler for profilers, which dispatches to more specific logic based on
the signal's payload (si_value).
The profiler signal handler is installed during libc preinit, after
malloc initialization (so races against synchronous heapprofd
initialization need not be considered). In terms of code organization, I
copied the existing approach with a loosely referenced function in
bionic_globals.h. Do tell if you'd rather a different approach here.
The profileability of a process is quite tied to the malloc
files/interfaces in bionic - in particular, it's set through
android_mallopt. I do not change that, but instead introduce a new
android_mallopt option to be able to query profileability of the
process (which is now used by the new profiler signal handler). As part
of that, gZygoteChildProfileable is moved from heapprofd_malloc to
common (alongside gZygoteChild).
I've removed the masking and reraising of the heapprofd signal when
racing against malloc_limit init. We're ok with taking a simpler
approach and dropping the heapprofd signal in such an unlikely race.
Note: this requires a corresponding change in heapprofd to use sigqueue()
instead of kill(), as the latter leaves the si_value uninitialized(?) on
the receiving side.
Bug: 144281346
Change-Id: I93bb2e82cff5870e5ca499cf86439860aca9dfa5
I have no idea why I used the iterate name internally which is
completely unlike every other function name. Change this to match
everyone else so that it's now malloc_iterate everywhere.
This is probably the last chance to change this before mainline
modules begin, so make everything consistent.
Test: Compiles, unit tests passes.
Change-Id: I56d293377fa0fe1a3dc3dd85d6432f877cc2003c
When using a FILE object for some malloc debug functions, calling
fprintf will trigger an allocation to be put in the object. The problem
is that these allocations were not allocated by the malloc debug
wrapper and they get freed during the fclose as if they are malloc
debug allocation. In most cases, the code will detect the bad pointer
and leak the memory, but it might also cause a crash.
The fix is to avoid using fprintf so that no allocations are made
in the object that survive and need to be freed in the fclose call.
Change the MallocXmlElem.h to use a file decsriptor not a FILE object.
Add new unit and system tests to detect this case.
Bug: 143742907
Test: Ran unit and system tests.
Test: Ran bionic unit tests.
Change-Id: I524392de822a29483aa5be8f14c680e70033eba2
The underlying descriptor is supposed to be closed when the ScopedFd
goes out of scope but due to a typo in reset() that never happens.
This change was inspired by an earlier implementation of ScopedFd in:
04dc91ae7 Load library using file handle.
Issue: 143918215
Test: Manual (verify fd usage on device)
Change-Id: I5664fa82d3e732113732e34a7ae6df3ca79d3cee
This is actually for the new change I'm working on, but let's retrofit
it first to separate any bugs in these changes from those in the new
change...
Test: treehugger
Change-Id: I890aeb61f9792810a77ad0da3f9674c9cc5db7bb
As it turns out, our "generic" arm64 implementations of certain string.h
functions are not actually generic, since they will eagerly read memory
possibly outside of the bounds of an MTE granule, which may lead to a segfault
on MTE-enabled hardware. Therefore, move the implementations into a "default"
directory and use ifuncs to select between them and a new set of "mte"
implementations, conditional on whether the hardware and kernel support MTE.
The MTE implementations are currently naive implementations written in C
but will later be replaced with a set of optimized assembly implementations.
Bug: 135772972
Change-Id: Ife37c4e0e6fd60ff20a34594cc09c541af4d1dd7
In order for an ifunc resolver to detect the presence of certain CPU features,
access to getauxval(AT_HWCAP) or getauxval(AT_HWCAP2) may be required. In order
for getauxval() to work, it needs to access the pointer to the auxiliary vector
stored by the linker in the libc shared globals data structure. Accessing the
shared globals requires libc to call the __libc_shared_globals() function
exported by the linker. However, in order to call this function, libc must
be fully relocated, which is not guaranteed to be the case at the point when
ifunc resolvers are called.
glibc solves this problem by passing the values of getauxval(AT_HWCAP)
(and getauxval(AT_HWCAP2) on aarch64) as arguments to the ifunc resolver.
Since this seems to be not only the most straightforward way to solve the
problem but also improves our compatibility with glibc, we adopt their
calling convention.
This change is ABI compatible with old resolvers because the arguments are
passed in registers, so the old resolvers will simply ignore the new arguments.
Bug: 135772972
Change-Id: Ie65bd6e7067f0c878df3d348c815fda61dc12de2
There are places in frameworks and art code that directly included
private bionic header files. Move these files to the new platform
include files.
This change also moves the __get_tls.h header file to tls.h and includes
the tls defines header so that there is a single header that platform
code can use to get __get_tls and the defines.
Also, simplify the visibility rules for platform includes.
Bug: 141560639
Test: Builds and bionic unit tests pass.
Change-Id: I9e5e9c33fe8a85260f69823468bc9d340ab7a1f9
Merged-In: I9e5e9c33fe8a85260f69823468bc9d340ab7a1f9
(cherry picked from commit 44631c919a)
We're going to have to add ifuncs to libm, and there will be some SVE
ones for arm64 soon too, so let's start sharing the absolute minimum...
Test: builds
Change-Id: Idbb9dd9477291ed3c15dc3902f65e593b766dfb9
Instead of having platform directories directly include the
private header, create a platform header directory and export it.
Bug: 130763340
Test: Builds.
Change-Id: Ie0f092b3fe077a3de8b90266c0b28bfbc20d0dfa
Merged-In: Ie0f092b3fe077a3de8b90266c0b28bfbc20d0dfa
(cherry picked from commit 8f582ef2f8)
The arm32 case was omitted when the macro was originally added due to
clang issues (b/15469122#comment5).
Testing at ToT master, ".cfi_undefined r14" appears to compile & work as
expected. This is the same directive as used by the linker's begin.S.
Test: ran a 32 bit binary on crosshatch-userdebug, verified that
"debuggerd -b" (which uses libunwindstack) stops at __libc_init as in
the arm64 case (before it would unwind past _start_main, with an
erroneous frame at the bottom).
Test: ran unwind_reg_info tool on /system/lib/libc.so with an address
within __libc_init, confirmed that the "debug_frame" section includes
"r14 = undefine" (arm exidx still contains "r14 = [cfa - 4]", but that
looked correct to cferris@).
Bug: 140008396
Change-Id: I6eb0bc6cf516fddee2df40492350678a0547fee3
The call to the load hook needs to be moved before the call to link_image()
because the latter calls ifunc resolvers which might access global
variables. This fixes a bunch of ifunc tests.
The dlfcn.segment_gap test is currently failing. One problem is that the name
of the .bss.end_of_gap section changes as a result of global instrumentation.
Add some wildcards in so that we match both names. The other problem seems
to be the same as b/139089152.
It turns out that we need to untag pointers in a few more places. Since we have
quite a few of these now it seems worth creating a function for it.
Test: bionic-unit-tests
Change-Id: I44e2b0904faacdda7cc0c5e844ffc09de01dea2d
These hooks notify the HWASan runtime library whenever a library is loaded
or unloaded so that it can update its shadow memory.
Bug: 138159093
Test: walleye_hwasan-userdebug boots with+without https://reviews.llvm.org/D65770
Change-Id: I6caf2a6540ed2c0d94db444e806a3c7ba504cabb
We will put instrumentation in ART to write out Java heap memory
information on receipt of this signal.
Bug: 136210868
Test: Flash blueline-userdebug and boot
Change-Id: I63173a4051c46b8569faaf3064e1db1dbd0ea3fe
A future version of HWASAN will set pointer tags when taking the address of
a global. This means that we need to untag pointers in a couple of cases
where potential global pointers are passed to an interface that expects
untagged pointers:
- The WriteProtected class, whose only instances are globals, passes its
own address to mprotect. However, our device kernels do not currently
untag pointers passed to mprotect (the proposed upstream kernel patches
do, however, untag these pointers), so once HWASAN starts tagging global
pointers, this will start failing.
- The shadow_load function loads from a shadow that corresponds to the
address space bounds of loaded binaries. Since these address space
bounds are untagged, the pointer needs to be untagged to match.
Test: boots
Change-Id: I3f11ce6eb7261752e5ff6d039d04dd45516b236f
Add a couple of comments explaining that a mutex can be freed during
the unlock call, immediately after the unlock's atomic exchange call but
before its futex wakeup call.
Bug: http://b/129744706
Test: bionic unit tests
Change-Id: I2d290ebde880f46866098d022720896039e7022e
Bug: 130028357
Test: malloc_hooks unit tests.
Test: Enable backtrace for mediaserver, run dumpsys media.player -m
Test: Enable backtrace for calendar, run am dumpheap -n <PID> <FILE>
Change-Id: I6774e28ccd9b3f2310127a5b39ccd15fe696a787
Merged-In: I6774e28ccd9b3f2310127a5b39ccd15fe696a787
(cherry picked from commit 3aadc5e80a)
Remove this global variable and change the setting of it to non-zero
to a call to android_mallopt.
In addition, change the initialize function to use pass a bool* instead of
int*.
Bug: 130028357
Test: Ran malloc_debug/malloc_hooks/perfetto tests.
Change-Id: I20d382bdeaaf38aac6b9dcabea5b3dfab3c945f6
Merged-In: I20d382bdeaaf38aac6b9dcabea5b3dfab3c945f6
(cherry picked from commit 5225b342f0)
Issue:
Process is crashed near the end (startup_handshake_lock.unlock()) in
pthread_create().
The newly created child thread passes this handshake_lock unexpectedly
=> its stack is unmapped & its associated pthread_internal_t data
structure can’t be accessed.
Analysis:
The created child thread should be blocked by startup_handshake_lock.lock()
and enter __futex_wait_ex()
But if the parent thread is in the middle of startup_handshake_lock.unlock():
void unlock() {
if (atomic_exchange_explicit(&state, Unlocked, memory_order_seq_cst) == LockedWithWaiter) { // => the state is modified to Unlocked
// (a) if the child thread is back to running and pass the while() check in Lock::lock()
// (b) the child thread executes its start_routine and then pthread_exit
// (c) the stack of the child thread (where its pthread_internal_t (so the startup_handshake_lock) is located) will be unmapped
__futex_wake_ex(&state, process_shared, 1); // => when the parent thread is back to running
// the “state” & “process_shared” of startup_handshake_lock can’t be accessed (unmapped)
// so the process will be crashed
}
}
Bug: 129744706
Test: Monkey
Change-Id: I55175e8c7ebc2b3b52de8a5602def0667076b974
Bug: http://b/128524141
This signal is used by libprofile-extras to trigger flush of gcov
coverage data.
Test: libprofile-extras in system/extras/toolchain-extras
Change-Id: I77dd5b1d67371a9cfaad54ad60254bfb2dd18ba9
We regressed on this recently: code under the upstream-* directories has
_PATH_BSHELL defined as a call to __bionic_get_shell_path(). In our own
code, we may as well just call it directly.
Bug: https://issuetracker.google.com/129030706
Test: ran tests
Change-Id: Ic2423f521272be95e67f94771772fe8072636ef0
Introduce an M_SET_ALLOCATION_LIMIT enumerator for android_mallopt(),
which can be used to set an upper bound on the total size of all
allocations made using the memory allocation APIs.
This is useful for programs such as audioextractor and mediaserver
which need to set such a limit as a security mitigation. Currently
these programs are using setrlimit(RLIMIT_AS) which isn't exactly
what these programs want to control. RLIMIT_AS is also problematic
under sanitizers which allocate large amounts of address space as
shadow memory, and is especially problematic under shadow call stack,
which requires 16MB of address space per thread.
Add new unit tests for bionic.
Add new unit tests for malloc debug that verify that when the limit
is enabled, malloc debug still functions for nearly every allocation
function.
Bug: 118642754
Test: Ran bionic-unit-tests/bionic-unit-tests-static.
Test: Ran malloc debug tests and perfetto integration tests.
Change-Id: I735403c4d2c87f00fb2cdef81d00af0af446b2bb
malloc_info needs to be per native allocator, but the code treated it
like a global function that doesn't depend on the native memory allocator.
Update malloc debug to dump the actual pointers that it has been tracking.
Test: bionic-unit-tests pass.
Test: malloc debug tests pass.
Test: malloc hook tests pass.
Change-Id: I3b0d4d748489dd84c16d16933479dc8b8d79013e
Merged-In: I3b0d4d748489dd84c16d16933479dc8b8d79013e
(cherry picked from commit a3656a98b1)
When the linker is invoked on itself, (`linker64 /system/bin/linker64`),
the linker prints an error, because self-invocation isn't allowed. The
current method for detecting self-invocation fails because the second
linker instance can crash in a constructor function before reaching
__linker_init.
Fix the problem by moving the error check into a constructor function,
which finishes initializing libc sufficiently to call async_safe_fatal.
The only important thing missing is __libc_sysinfo on 32-bit x86. The aux
vector isn't readily accessible, so use the fallback int 0x80.
Bug: http://b/123637025
Test: bionic unit tests (32-bit x86)
Change-Id: I8be6369e8be3938906628ae1f82be13e6c510119
Instead of every function being its own atomic, have a single
pointer that can be used to flip all pointers at once. This avoid cases
where the set of pointers can be in an partial switched state.
Also fix a few inconsistent naming of functions in the file.
Test: Ran unit tests (malloc debug, malloc hooks, perfetto).
Change-Id: I3f66da395414586a3fa87874d80dcdf5f702ed39
Merged-In: I3f66da395414586a3fa87874d80dcdf5f702ed39
(cherry picked from commit 77184aedaf)
* changes:
Implement dynamic TLS accesses and allocation
Implement TLS_DTPMOD and TLS_DTPREL relocations
Ignore DT_TLSDESC_GOT / DT_TLSDESC_PLT
Disable the dlfcn.dlopen_library_with_ELF_TLS test
Add BionicAllocator::memalign
Move the linker allocator into libc
Replace some of linker_allocator's header includes
This is used to prevent the additional indirection even after heap
profiling has finished, preventing any performance impact on processes
that are not currently being profiled.
Test: m
Test: flash sailfish
Test: try tearing down & re-enabling hooks
Bug: 120186127
Change-Id: Idc5988111a47870d2c093fd6a017b47e65f5616b
Initialize a thread's DTV to an empty zeroed DTV. Allocate the DTV and
any ELF module's TLS segment on-demand in __tls_get_addr. Use a generation
counter, incremented in the linker, to signal when threads should
update/reallocate their DTV objects.
A generation count of 0 always indicates the constant zero DTV.
Once a DTV is allocated, it isn't freed until the thread exits, because
a signal handler could interrupt the fast path of __tls_get_addr between
accessing the DTV slot and reading a field of the DTV. Bionic keeps a
linked list of DTV objects so it can free them at thread-exit.
Dynamic TLS memory is allocated using a BionicAllocator instance in
libc_shared_globals. For async-signal safety, access to the
linker/libc-shared state is protected by first blocking signals, then by
acquiring the reader-writer lock, TlsModules::rwlock. A write lock is
needed to allocate or free memory.
In pthread_exit, unconditionally block signals before freeing dynamic
TLS memory or freeing the shadow call stack.
ndk_cruft.cpp: Avoid including pthread_internal.h inside an extern "C".
(The header now includes a C++ template that doesn't compile inside
extern "C".)
Bug: http://b/78026329
Bug: http://b/123094171
Test: bionic unit tests
Change-Id: I3c9b12921c9e68b33dcc1d1dd276bff364eff5d7
Bionic needs this functionality to allocate a TLS segment with greater
than 16-byte alignment. For simplicity, this allocator only supports up
to one page of alignment.
The memory layout changes slightly when allocating an object of exactly
PAGE_SIZE alignment. Instead of allocating the page_info header at the
start of the page containing the pointer, it is allocated at the start
of the preceding page.
Bug: http://b/78026329
Test: linker-unit-tests{32,64}
Change-Id: I1c8d1cd7ca72d113bced5ee15ba8d831426b0081
