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
Soft-limits apps from calling bind() on NETLINK_ROUTE sockets, and
getting link info through getifaddrs(), while still allowing apps on the
allowlist to temporarily perform these actions.
This is different from existing behavior, where apps targeting an API
level < 30 were exempted from this restriction.
Actual enforcement will happen through SELinux (as is currently the
case for apps targeting API >= 30). This temporary change will then be
reverted.
If you arrived at this change due to an app showing unexpected behavior,
please file a bug at go/netlink-bug.
Bug: 170188668
Bug: 170214442
Test: Call bind() on NETLINK_ROUTE for an app on the allowlist.
Test: Call bind() on NETLINK_ROUTE for an app not on the allowlist.
Test: Call getifaddrs() for an app on the allowlist.
Test: Call getifaddrs() for an app not on the allowlist.
Test: Call bind() on a AF_UNIX socket with its protocol set to
NETLINK_ROUTE, confirm it can succeed.
Test: Verify that previously broken apps are no longer broken.
Change-Id: I8738f7912fdc816e0d30205557728ff9e84bf7e6
Revert "Updates CTS tests for MAC address restrictions."
Revert submission 1518603-soft-restrict-mac
Reason for revert: Missing type check
Reverted Changes:
I0488932de:Soft-enable MAC address restrictions with allowlis...
Idb9d940e4:Updates CTS tests for MAC address restrictions.
I9461f287e:Return anonymized MAC for apps targeting SDK < 30
Change-Id: I7e8e593518088ff5c8f6083b34e6966852475b9b
This was a little confusing because a quick glance suggested that
'err' was getting the value of getsockopt(), when actually it was
getting the result of the '<' operation. Since 'err' wasn't
being used, we clarify by just removing it altogether.
Test: TreeHugger
Change-Id: I1ff80812b460973e9d512a558623e0c44c2ecf09
Soft-limits apps from calling bind() on NETLINK_ROUTE sockets, and
getting link info through getifaddrs(), while still allowing apps on the
allowlist to temporarily perform these actions.
This is different from existing behavior, where apps targeting an API
level < 30 were exempted from this restriction.
Actual enforcement will happen through SELinux (as is currently the
case for apps targeting API >= 30). This temporary change will then be
reverted.
If you arrived at this change due to an app showing unexpected behavior,
please file a bug at go/netlink-bug.
Bug: 170188668
Bug: 170214442
Test: Call bind() on NETLINK_ROUTE for an app on the allowlist.
Test: Call bind() on NETLINK_ROUTE for an app not on the allowlist.
Test: Call getifaddrs() for an app on the allowlist.
Test: Call getifaddrs() for an app not on the allowlist.
Change-Id: I0488932deea2a7211e55a24bc33bfa3cfb16fba2
They're both obsolescent in POSIX.1-2008, and you really shouldn't be
using them, but since we can't actually delete them...
This change makes them both obey $TMPDIR if set, and fall back to
/data/local/tmp otherwise. That's as good as we've managed for anything
else such as tmpfile(3).
Also add some tests.
Bug: http://b/174682340
Test: treehugger
Change-Id: Ieef99dcc2062f84b2b7cbae046787fdfe975e772
We can sometimes fail to link a static executable because of
duplicate definitions of std::nothrow in bionic/libc/bionic/new.cpp
and external/libcxx/src/new.cpp. Fix it by making our definition
weak since it doesn't matter which one ends up being chosen.
Change-Id: Iec02ae89f4a3d2ffe298817240f404e54b109a52
aosp/1484976 introduced a breaking change where
DisableMemoryMitigations() now indiscriminately turns tagged pointers
off. When android_mallopt(M_DISABLE_MEMORY_MITIGATIONS) is called, the
correct behaviour is:
- In SYNC/ASYNC MTE mode -> disable all tagged pointers.
- If all tagged pointers are already disabled -> nop.
- If we're in TBI mode -> nop (keep the TBI mode as-is).
In order to do that we have to allow probing of the current heap tagging
mode. In order to prevent TOCTOU between GetHeapTaggingLevel() and
SetHeapTaggingLevel(), we expose a global mutex that should be held when
calling these functions.
Bug: 174263432
Test: atest CtsTaggingHostTestCases on Flame
Change-Id: Ia96f7269d542c9041270458806aee36766d2fbbb
Unnecessary include, and the file was deleted with upstream changes.
Bug: N/A
Test: cd external/gwp_asan && atest
Change-Id: Id8facff0c5e382aa639ce158b7b5862ce157aa25
- Make it apply to every thread, and thus remove the restriction
that it must be called while the program is single threaded.
- Make it change TCF0 itself (on all threads), instead of requiring
callers to do it themselves, which can be error prone.
And update all of the call sites.
Change the implementation of
android_mallopt(M_DISABLE_MEMORY_MITIGATIONS) to call
android_mallopt(M_SET_HEAP_TAGGING_LEVEL) internally. This avoids
crashes during startup that were observed when the two mallopts
updated TCF0 unaware of each other.
I wouldn't expect there to be any out-of-tree callers at this point,
but it's worth noting that the new interface is backwards compatible
with the old one because it strictly expands the set of situations in
which the API can be used (i.e. situations where there are multiple
threads running or where TCF0 hadn't been updated beforehand).
Bug: 135772972
Change-Id: I7746707898ff31ef2e0af01c4f55ba90b72bef51
259776d8ff
The original change was reverted due to InterfaceParamsTest failing.
This test has now been fixed in r.android.com/1498525.
The original change message is below.
Updates getifaddrs() to behave as if RTM_GETLINK requests are not
allowed for non-system apps. This is different from previous behavior,
where apps targeting an API level < 30 were exempted from this
restriction.
Actual enforcement happens in SELinux. This change:
- Soft-enables the behavior until SELinux changes are in place
- Prevents logspam from SELinux denials after they are in place
Bug: 170188668
Test: atest bionic-unit-tests-static
Test: atest NetworkInterfaceTest
Test: Connect to Wi-Fi network
Test: Call getifaddrs() directly from within an app.
Test: Call NetworkInterface#getNetworkInterfaces() from within an app.
Change-Id: Ia9f61819f8fdf878c0e67e57de7e893fb3b1c233
Revert "Enforce RTM_GETLINK restrictions on all 3p apps"
Revert "Updates tests for untrusted app MAC address restrictions"
Revert submission 1450615-mac-address-restrictions
Reason for revert: DroidMonitor: Potential culprit for Bug 173243616 - verifying through Forrest before revert submission. This is part of the standard investigation process, and does not mean your CL will be reverted
Reverted Changes:
I08c709b2b:Enforce RTM_GETLINK restrictions on all 3p apps
I95d124ae8:Soft-enables new MAC address restrictions.
I5392f8339:Updates tests for untrusted app MAC address restri...
I9d214c5d0:Return anonymized MAC for apps targeting SDK < 30
Change-Id: If09920b24370077d647d60cfda7fc0a110b77851
A constructor or destructor function with an integral priority is
placed in an .init_array or .fini_array section with the priority
suffixed to the section name:
- __attribute__((constructor)) ==> .init_array
- __attribute__((constructor(42))) ==> .init_array.42
The suffixed init/fini sections appear before the unsuffixed sections,
so the prioritized functions appeared before the __{INIT,FINI}_ARRAY__
symbols and were dropped when the symbols were used.
The (static) linker doesn't recognize priority suffixes on
.preinit_array.
This bug affected .init_array and .fini_array for static executables.
For dynamic executables, only .fini_array was affected, because
.init_array is handled by the dynamic loader instead, which uses
DT_INIT_ARRAY[SZ]. For DSOs, neither is affected, because the two
sections are only handled by the dynamic loader.
This patch also fixes a minor inconsistency where dynamic init/preinit
were passed argc/argv/envp, but static were not.
Bug: http://b/170983066
Test: bionic-unit-tests
Change-Id: I0fffa776e5d9bdb6f8af06b4c1af148236742fef
The functions now read /etc/group and /etc/passwd on host machines.
Android-specific IDs are not recognized in the host.
getpwent and getgrent are still not working though. They require a
bigger refactoring to sequentially advance in the database files.
Bug: 171718702
Test: run assemble_cvd in aosp_cf_arm64_phone
Change-Id: Ie8da382a467bbd0bffac7b4b8592cd871db80181
/system/bin/sh exists only on Android. When Bionic is built for the
host, use the standard /bin/sh.
Bug: 159685774
Test: run aosp_cf_arm64_phone on rockpi4
Change-Id: Idf4028c134abc128f203fc4d3be591f06b8fe8ff
When an array element is added or removed, make only the relevant page
writable, rather than the entire array. The entire array is still made
writable during recompaction and expansion.
This change fixes most of a large regression in __cxa_atexit runtime
(blueline, taskset 10, performance governor, 100000 registrations,
times are in seconds)
- Q: _Exit=0.292380, exit=0.626801
- R: _Exit=28.435082, exit=95.785110
- new: _Exit=0.352285, exit=0.713893
Test: bionic unit tests
Test: adb shell taskset 10 \
/data/benchmarktest64/bionic-spawn-benchmarks/bionic-spawn-benchmarks \
--benchmark_filter='atexit' \
--benchmark_display_aggregates_only=true \
--benchmark_repetitions=10
Bug: http://b/168043760
Change-Id: I88cc15c29c9890b422b7f621f29f98a03ca1f886
pthread_cond_pulse would unconditionally call futex, which meant that
a number of processes were spending a significant amount of time in
pthread_cond_broadcast and pthread_cond_signal when there were no
threads waiting on that pthread_cond_t.
This change adds a counter to the 64-bit pthread_cond_t struct and
only calls futex() in cases where there is a nonzero waiter count. The
32-bit pthread_cond_t is unchanged due to compatibility reasons.
Test: no pthread_cond_broadcast/signal stacks without try_to_wake_up
in SurfaceFlinger
bug: 168831708
Change-Id: I105e1345cd2a3a75f98cd0acf316e790ba1716f0
Introduce an android_mallopt(M_DISABLE_MEMORY_MITIGATIONS) API call
that may be used to disable zero- or pattern-init on non-MTE hardware,
or memory tagging on MTE hardware. The intent is that this function
may be called at any time, including when there are multiple threads
running.
Disabling zero- or pattern-init is quite trivial, we just need to set
a global variable to 0 via a Scudo API call (although there will be
some separate work required on the Scudo side to make this operation
thread-safe).
It is a bit more tricky to disable MTE across a process, because
the kernel does not provide an API for disabling tag checking in all
threads in a process, only per-thread. We need to send a signal to each
of the process's threads with a handler that issues the required prctl
call, and lock thread creation for the duration of the API call to
avoid races between thread enumeration and calls to pthread_create().
Bug: 135772972
Change-Id: I81ece86ace916eb6b435ab516cd431ec4b48a3bf
Updates getifaddrs() to behave as if RTM_GETLINK requests are not
allowed for non-system apps. This is different from previous behavior,
where apps targeting an API level < 30 were exempted from this
restriction.
Actual enforcement happens in SELinux. This change:
- Soft-enables the behavior until SELinux changes are in place
- Prevents logspam from SELinux denials after they are in place
Bug: 170188668
Test: atest bionic-unit-tests-static
Test: atest NetworkInterfaceTest
Test: Connect to Wi-Fi network
Test: Call getifaddrs() directly from within an app.
Test: Call NetworkInterface#getNetworkInterfaces() from within an app.
Change-Id: I95d124ae81bcb26583db5ad01d95d441d52f973c
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
This is already covered by the existing test by virtue of being used for
all threads.
Bug: http://b/168258494
Test: treehugger
Change-Id: I5c872fd7f30a4c79de1d70e7702f4b12d4e94cd3
Some global static variables are only used within a single function,
so moving them inside the function which requires them. This also
ensures those static variables of class types are initialized before
they are being used.
For example, this is needed for CachedProperty(), as some of the
member method, e.g., CachedProperty().Get(), might be invoked,
e.g., in __libc_preinit(), before the constructor is invoked.
This happens after we added property trace points in commit
Id2b93acb2ce02b308c0e4889f836159151af3b46.
For g_lock, we don't move them because it can be initialized by setting
its memory to 0. And it's used in two functions.
https://android.googlesource.com/platform/bionic/+/refs/heads/master/libc/private/bionic_lock.h
Bug: 147275573
Test: atest CtsBionicTestCases
Test: adb shell perfetto -o /data/misc/perfetto-traces/test_trace -t 10s bionic sysprop
Change-Id: I99565ce2442d34f33830778915c737eed834f8b3
Introducing a new systrace tag, TRACE_TAG_SYSPROP, for use with
system property.
For property set, the tracing is added in __system_property_set() instead of
__system_property_update() / __system_property_add() so we can record
control properties like ctl.*, sys.powerctl.*, etc.., which won't be
updated via the latter two functions.
Bug: 147275573
Test: atest CtsBionicTestCases
Test: adb shell perfetto -o /data/misc/perfetto-traces/test_trace -t 10s bionic
Test: adb shell perfetto -o /data/misc/perfetto-traces/test_trace -t 10s sysprop
Test: adb shell /data/benchmarktest64/bionic-benchmarks/bionic-benchmarks \
--benchmark_filter=BM_property --bionic_cpu=4, then compares the results
of property benchmarks before and after the change, didn't see
significant difference.
Change-Id: Id2b93acb2ce02b308c0e4889f836159151af3b46
Merged-In: Id2b93acb2ce02b308c0e4889f836159151af3b46
(cherry picked from commit 26970c3493)
Mostly from extra test cases, but also:
* Move the fgets size < 0 assertion into fgets.
* Use ELF aliases for strtoq/strtouq rather than duplicating code.
* Don't check uname() succeeded, since it can't fail.
Test: treehugger
Change-Id: I2e6b3b88b0a3eb16bd68be68b9bc9f40d8043291
(Based on proposal at https://sourceware.org/glibc/wiki/ThreadPropertiesAPI)
This includes API to:
- locate static and dynamic TLS
- register thread-exit and dynamic TLS creation/destruction callbacks
Change-Id: Icd9d29a5b2f47495395645e19d3b2c96826f19c8
Now we're being marked down for our poor coverage, we may as well remove
more broken cruft. Despite the amount of effort that seems to have gone
into pututline(), it wasn't working with the other utmp.h functions (in
particular, utmpname()), and wasn't declared in the header file!
Test: treehugger
Change-Id: I1a583984189c751168c11c01431433f96f8c548b
Similar to the musl strstr. This patch also increases test coverage for
memmem, again similar to the strstr tests.
Test: treehugger
Change-Id: I7f4a2ab93a610cb692994d06d2512976e657ae9f
For vanity reasons, this CL should be reverted after b/158870657, since
we want to make sure zero/pattern fill always happen when they are
requested.
Bug: 131355925
Bug: 158870657
Test: check build on normal devices hits USE_SCUDO branch with #error
Change-Id: I22d19dbc75004c8f7437e1247226bc3275cd4df5
This was initially just to try on "pty" and "tty" for size, while
discussing with other projects to try to align on vocabulary, but -- in
the implemention and tests at least -- but these work out so well that
I'm tempted to go with them anyway if we can't come to a concensus. We
can always come back and change them again later.
What I really like is that you pass the pty to functions like ptsname()
and grantpt() with a 'p' in the name, and the tty to functions like
ttyname() and tcsetattr() with just 't's.
The use of "parent" and "child" in forkpty() seems helpful too.
Also fix the documentation of forkpty(), which wasn't quite right.
Test: treehugger
Change-Id: Ic010c4b669f6528591c653e3701f4e41e0d0df9e
Normally, getifaddrs() gets its information about interface flags from
RTM_NEWLINK messages. With the ability for non-system apps to send
RTM_GETLINK messages going away, resolve these flags by performing a
SIOCGIFFLAGS ioctl call instead.
This fixes a bug where the flags were erroneously copied over from an
ifaddrmsg's ifa_flags.
Bug: 141455849
Test: atest bionic-unit-tests-static
Test: Connect to wireless network.
Test: Call getifaddrs() from a non-system app targeting API R+, verify
that the interface flags are correct.
Change-Id: I552f3b9d21072f59ee6f0890f47cb1bdf3cfcf20
As scudo is now the default native allocator, there is no need to
keep this library around.
Test: Builds and walleye boots.
Change-Id: Id2918544651348854956d1348d2a634f6425b952
Merged-In: Id2918544651348854956d1348d2a634f6425b952
(cherry picked from commit 941ad56299)
This means that execle() doesn't have to drag in all the path code and
shell script special case code from its relatives, for branches that
will never be taken. Specifically, it means that they disappear from the
monstrously large linker binary after this change.
A drop in the ocean of bloat, yes, but you have to start somewhere.
Bug: http://b/25200716
Test: treehugger, objdump
Change-Id: I21126823d869cee07bf7320bc0a65ea2aaee65a1
We are removing native bridge copy of fork.cpp, but need to
replace call to clone() when it's done for bionic's fork.
The code here will run all pre-/post-clone routines for *guest*,
while native bridge implementation will need to run the
corresponding *host* routines.
Bug: 145028007
Test: bionic-unit-tests
Change-Id: Ic5524e743caa287d7aaa8dc7e5d34acd1c7e1170
clock_gettime() with known arguments can't fail (and we ignore its
return value in most other places already).
Test: treehugger
Change-Id: I2374ae5ba1598a01d4c4f689b9c75c4e7dc926b6
Add SCUDO_OPTIONS to the list of the environment variables that should
not be preserved across a security boundary.
Bug: 157484128
Test: Builds and boots.
Change-Id: Id8644608114ad2fd49baedbdbbe1c899768bd54d
Ensure we don't call sigchain's sigaction64 from bionic's sigaction by
extracting sigaction64 to a static function.
Test: treehugger
Change-Id: I16226c6ac580ece7000c335beb1d3b76429d2a06
It turns out that we weren't actually enabling stack traces due
to a typo in one of the if statements. Fix it.
Bug: 135772972
Change-Id: Ic19352280f65acd621b9d88c361c55680b3327f0
Our various fd debugging facilities get extremely confused by a vforked
process closing file descriptors in preparation to exec: fdsan can
abort, and fdtrack will delete backtraces for any file descriptors that
get closed. Keep track of whether we're in a vforked child in order to
be able to detect this.
Bug: http://b/153926671
Test: 32/64-bit bionic-unit-tests on blueline, x86_64 emulator
Change-Id: I8a082fd06bfdfef0e2a88dbce350b6f667f7df9f
The SYNC tagging level enables stack trace collection for allocations and
deallocations, which allows allocation and deallocation stack traces to
appear in tombstones when encountering a tag check fault in synchronous tag
checking mode.
Bug: 135772972
Change-Id: Ibda9f51b29d2c8e2c993fc74425dea7bfa23ab1e
Some applications may incorrectly assume that a "negative" pointer
indicates error. By setting the top bit in TBI mode, we catch these
cases as well. Although they don't directly indicate incompatibility
with MTE (as MTE uses the 2nd-highest nibble) - this ensures we catch
HWASan errors as well as reserve the bits for future MTE expansion.
Bug: 154272452
Test: Boot flame, device boots successfully.
Change-Id: I94a6821a82ce7ead59e6ca2dd113a5a50ee9e56b
This also removes the ScopedSignalBlocker, which doesn't seem to have
made any sense since threads were invented.
Test: treehugger
Change-Id: I9a323ab4a0b43f14fd5d1f0df1f80184aef63770
1) "fix the system properties design" is written for the old protocol,
so we've already changed the design. There are no other further
planned changes.
2) "don't drag in all the macros, just the types." is not likely to
happen or be particularly impactful.
3) "Find a location suitable for these functions ..." is refering to
legacy code. More likely that this code will be removed before we
find a serious reason to fix this TODO.
4) "(73062966) We still don't have a good way ..." is stale; we fixed
this bug and added the appropriate mechanism.
Test: n/a
Change-Id: I23991692cdeb81ad00844a6a1680900ff384208b
These solves the issue that targetting non-profilable apps crashed them.
There is still a race condition between starting the app and the SIG_IGN
being installed, but that will be fixed in follow-ups.
This also does not cover programs that statically link libc, but those are
rare.
This might be reverted if we find a more general solution to
b/151835887.
Bug: 151328035
Test: java profile sysui, doesn't crash
Test: atest CtsBionicTestCases
Change-Id: I6b99352ed50afe15a609f7ddb85312c2676ddf11
POSIX added these GNU extensions for issue 8.
I've made these always inline without the usual "until API level X"
proviso because they're single instructions that the compiler can inline
and there's really no point providing these if they add function call
overhead --- everyone should just use __builtin_ffs() and friends
instead in that case.
Bug: https://austingroupbugs.net/view.php?id=617
Test: treehugger
Change-Id: I33fc4b8648ea25917329e81c1b4c60eb9a66d667
The double hooking is currently controlled by two separate atomic
booleans. In this state, it is very hard to guard against very subtle
race conditions. Adding an explicit enum to encode the state makes it
easier to reason about the behaviour of the program.
Also introduce a MaybeModifyGlobals to consistently guard accesses to
globals. We would sometimes modify them without swapping the
gGlobalsMutating, introducing a window for race with
HandleHeapprofdSignal.
Test: while true; do atest HeapprofdCtsTest; done
Test: flash blueline and manually test profile
Bug: 150741354
Change-Id: I77102b0bd906b8ec2d6806bd43ba6cbf7191e37c
Simplify:
- Use a single memory-mapped region to hold the table of destructors.
Double its capacity each time it is expanded.
- Add a recompaction pass at the end of __cxa_finalize that shifts
entries forward and uses madvise to clean pages.
Bug: http://b/148055738
Test: bionic-unit-tests
Change-Id: Ieb9da2b88640a8a5277d217b43826b5b7e246781
Bug: 137795072
Test: Built using jemalloc, and verified that the two processes that
Test: use libc_scudo do not crash.
Change-Id: Icf773b656d7e2bcdf41b4979e9b6cd980b02d34d
When the heapprofd lazy-initialization dispatch table is installed, two
threads can call malloc() at the same time. One will do the
lazy-initialization, the other will fail the atomic_exchange of
gHeapprofdInitHookInstalled and go to system malloc. The system malloc
still contains the lazy-init malloc, and will recurse.
Fix it so the second thread goes directly to the previous dispatch
table, or to the system allocator directly (instead of through libc
malloc()).
Bug: 150085813
Test: atest perfetto_integrationtests
Test: TracedPerfCtsTest.SystemWideDebuggableApp on cuttlefish x86.
Change-Id: Ia85ad619a0d5e3f558136d84c34dbada4e8b845d
The values of some constants have changed, and the tag mask has changed from
being an exclusion mask to an inclusion mask.
Bug: 135772972
Change-Id: I322fceae7003bee6b391e7187194ed4c0cb28c58
If an alternate signal stack is set and the frame record is in bounds of
that stack, we need to use the top of the alternate signal stack for bounds
checking rather than the normal stack.
Bug: 150215618
Change-Id: I78b760d61b27da44f8e0cfee3fe94a791011fe58
To prevent unnecessary SELinux denials, the check referred to by the
comment is kept in place. This allows us to keep auditing the
SELinux denial in order to catch apps that intentionally send
RTM_GETLINK messages.
Fix: 141455849
Test: atest bionic-unit-tests-static
Test: atest NetworkInterfaceTest
Change-Id: I41e5ad6f071c820a8246177a0c629b3be788a942
Heapprofd uses an ephemeral dispatch table to ensure that GWP-ASan can
be called during heapprofd initialization. Previously, we grabbed the
backup dispatch table from the globals, which is wrong if GWP-ASan isn't
installed (as the malloc_dispatch_table is linker-initialised and not
set to a known good value if a malloc hooker isn't used).
Instead, grab the correct ephemeral dispatch table. When GWP-ASan is
installed, use a copy of its table as the dispatch. When nothing is
installed, fall back to using the native allocator dispatch.
Bug: 135634846
Bug: 150085813
Test: atest perfetto_integrationtests
Test: TracedPerfCtsTest.SystemWideDebuggableApp on cuttlefish x86.
Change-Id: I4beaf9192acadbe00adc02de2e0b0aab6f7a2190
This is a best-effort mitigation for potential crashes when the
profiling signal handler is triggered within certain secomp'd processes.
In particular, we're working around cases where the seccomp policy
doesn't allow some of the handler syscalls, and has a crashing
disposition towards violations via SECCOMP_RET_TRAP, plus a crashing
SIGSYS handler. While not general, this covers the configurations seen
in practice on Android (which are all using minijail in the same way).
By overriding the SIGSYS handling for the duration of the profiling
handler, we can instead receive such SIGSYS signals, and instead recover
from them in a non-crashing manner (the handler is responsible for
filling the syscall return register, since the syscall itself was
skipped).
For simplicity, we're swallowing all SIGSYS signals during this window,
without trying to figure out whether they're something that could be
caused by the profiling signal handler. I've quite convinced myself that
-ENOSYS seems to be safe to return to all of bionic's syscall wrappers
across the four architectures (looking at gensyscalls + the
special-cases like vfork and clone). It is theoretically possible for
all kinds of conflicting (ab)uses of SIGSYS to exist, but I'm assuming
it's not a realistic concern until proven otherwise.
Tested: manually sigqueue'd configstore on crosshatch, confirmed that
the SIGSYS override log was printed, and the process did not
crash (as it does on master).
Bug: 149328505
Change-Id: Iab8f09e51169807c9d3e1e0bcfd042f09f7df6a4
For interfaces that didn't get their name set, try to resolve their name
using if_indextoname first, before removing them from the list.
This allows getifaddrs() to keep returning interfaces that only have an
IPv6 address set (as opposed to the previous behavior where only
interfaces with an IPv4 address would be returned).
Change-Id: I0e4e6611948b12794cd3e354538f2964fbf31078
Fix: 148886805
Bug: 141455849
Test: atest NetworkInterfaceTest
Test: atest bionic-unit-tests-static
Test: atest IpSecManagerTunnelTest
The tag level may now be async, which is now the default. When the tag level
is set to none, memory tagging is disabled in the allocator using the new
API proposed in https://reviews.llvm.org/D70762 .
Bug: 135772972
Change-Id: I847f5822a70913c446ed9ffa13792177bbfc96af
Current APEX Namespace is named with APEX name itself, which also uses
.(dot) so linker configuration can keep the syntax safe.
For example, if there are APEX modules named 'A' and 'A.link.A', then
'namespace.A.link.A.link.A = a.so' phrase can be ambiguous from the
linker. To allow any additional linker syntax in the future, we should
avoid dot separator from the namespace name.
Bug: 148826508
Test: m -j passed
Test: boot succeeded from cuttlefish and walleye
Change-Id: Ic3fe396aef6366fc6c7a0677bc7f92a57fd4e229
Exports GWP-ASan allocator information callbacks to libdebuggerd so that
tombstoned can get information from the GWP-ASan allocator in the case
of a crash.
Bug: 135634846
Test: atest bionic-unit-tests
Change-Id: Ie16426af55602fb2a76c4e69217773354c365843
The notification that GWP-ASan is enabled causes ART tests to break.
Remove the log for now.
Bug: 135634846
Bug: 149790891
Test: (Attempt to fix the ART tests).
Change-Id: I8a7751a838a64f160b3b7b9f07752bb64644b9db
This patch enables GWP-ASan with process sampling.
**Note**: If you are visiting this patch because this broke a test or
otherwise is causing failures, please contact mitchp@ directly (or
respond to this patchset). GWP-ASan is designed to cause heap-based
memory safety bugs to manifest in SEGV on a sampled basis.
Bug: 135634846
Test: atest bionic-unit-tests gwp_asan_unittest
Change-Id: I58ca9373def105fdd718cf283482b3220b770698
GWP-ASan + heapprofd don't currently play nice together in some
circumstances. heapprofd thinks it's still an only child, and refuses to
accept the existence of its little brother, GWP-ASan.
If GWP-ASan is installed before heapprofd, then heapprofd is *required*
to respect that libc has a favourite child. If an allocation/free is passed
to heapprofd, then heapprofd *must* (eventually) pass that allocation/free to
GWP-ASan. If heapprofd doesn't do this, then a free() of a GWP-ASan
allocation can be passed to the system allocator.
This can happen in two places right now:
1. The heapprofd hooks simply clobber any trace of what was
previously in the default_dispatch_table when enabled through the
heapprofd signal.
2. Heapprofd can die when the system is under significant pressure.
Some pipes can timeout, which ends up in the client calling ShutdownLazy()
-> mallopt(M_RESET_HOOKS) -> DispatchReset(). This also clobbers any
trace of the previous default_dispatch_table.
To fix both these problems, we fix heapprofd to restore the previous
default_dispatch_table whenever either circumstance happens. We do some
tricky copying to avoid race conditions on the malloc_dispatch_table in
fixing #1.
Bug: 135634846
Test: Run HeapprofdEndToEnd.NativeProfilingActiveAtProcessExit/ForkMode
a significant number of times with large amounts of system pressure (I
just run bionic-unit-tests-scudo in parallel). You will see some test
failures where heapprofd died due to system pressure, but never a death
from the allocator. Tests should never fail when the system isn't under
immense pressure.
Change-Id: I20ab340d4bdc35d6d1012da5ee1a25634428d097
When enabled, GWP-ASan sets the current dispatch table. Then, when a
shim layer (malloc_debug, malloc_hooks, heapprofd) comes along, they
should (by design) overwrite the current dispatch table.
Currently, these shim layers check to see whether malloc_limit is
installed by checking the current dispatch table against nullptr.
Because GWP-ASan owns the current dispatch table, the shim thinks that
malloc_limit is installed and falls back to only use the default
dispatch, thinking that malloc_limit will call them. This is not the
case, and they should take over the current dispatch pointer.
Bug: 135634846
Test: atest bionic
Change-Id: Ifb6f8864a15af9ac7f20d9364c40f73c5dd9d870
The WriteProtected mutator for __libc_globals isn't reentrant.
Previously we were calling __libc_globals.mutate() inside of GWP-ASan's
libc initialisation, which is called inside the __libc_globals.mutate().
This causes problems with malloc_debug and other malloc shims, as they
fail to install when GWP-ASan is sampling their processes.
Bug: 135634846
Test: atest bionic
Change-Id: Iae51faa8d78677eeab6204b6ab4f3ae1b7517ba5
Scudo still isn't quite at the same RSS as jemalloc for the svelte config
so only enable this for normal config.
Bug: 137795072
Test: Built svelte config and verified it is still jemalloc.
Test: Ran performance tests on normal config (bionic benchmarks).
Test: Ran trace tests (system/extras/memory_replay).
Test: Ran scudo unit tests.
Test: Ran bionic unit tests.
Test: Ran libmemunreachable tests.
Test: Ran atest CtsRsBlasTestCases on cuttlefish.
Test: Ran atest AslrMallocTest.
Test: Ran atest CtsHiddenApiKillswitchWildcardTestCases and verified it has
Test: the same runtime as the jemalloc.
Change-Id: I241165feb8fe9ea814b7b166e3aaa6563d18524a
This patch introduces GWP-ASan - a sampled allocator framework that
finds use-after-free and heap-buffer-overflow bugs in production
environments.
GWP-ASan is being introduced in an always-disabled mode. This means that
GWP-ASan will be permanently disabled until a further patch turns on
support. As such, there should be no visible functional change for the
time being.
GWP-ASan requires -fno-emulated-tls wherever it's linked from. We
intentionally link GWP-ASan into libc so that it's part of the initial
set of libraries, and thus has static TLS storage (so we can use
Initial-Exec TLS instead of Global-Dynamic). As a benefit, this reduces
overhead for a sampled process.
GWP-ASan is always initialised via. a call to
mallopt(M_INITIALIZE_GWP_ASAN, which must be done before a process is
multithreaded).
More information about GWP-ASan can be found in the upstream
documentation: http://llvm.org/docs/GwpAsan.html
Bug: 135634846
Test: atest bionic
Change-Id: Ib9bd33337d17dab39ac32f4536bff71bd23498b0
tracefs will be mounted at /sys/kernel/tracing when debugfs
is not mounted.
Bug: 134669095
Test: atest bionic-unit-tests-static
Change-Id: Ic224cf13500efc570da8b6a27ce925bbcf068fdd
For consistency, linker namespace for apex modules use its apex name
instead of hard-coded short name.
Bug: 148826508
Test: m / boot
Change-Id: I4bf565cd528d744fc42841fd2d9f8bf652d4d346
We've optimized the ctype functions to the point where they're pretty
much all down to one instruction. This change takes the obvious next
step of just inlining them.
On Android these function have only ever been for ASCII. You need the
<wctype.h> functions for non-ASCII.
libc++ currently has its own inlines for the _l variants, so if we want
to just inline them in bionic directly, we'll need to coordinate that.
Bug: http://b/144165498
Test: treehugger plus benchmarks
Change-Id: I4cc8aa96f7994ae710a562cfc9d4f220ab7babd6
This function will be used by Scudo and GWP-ASan to efficiently collect
stack traces for frames built with frame pointers.
Bug: 135634846
Bug: 135772972
Change-Id: Ic63efdbafe11dfbb1226b5b4b403d53c4dbf28f3
Merged-In: Ic63efdbafe11dfbb1226b5b4b403d53c4dbf28f3
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)
mte_supported() lets code efficiently detect the presence of MTE, and
ScopedDisableMTE lets code disable MTE RAII-style in a particular region
of code.
Bug: 135772972
Change-Id: I628a054b50d79f67f39f35d44232b7a2ae166afb
Fixes includes in heap tagging to ensure that bionic under MTE builds
successfully.
Thanks Kevin for finding this!
Test: TARGET_EXPERIMENTAL_MTE=true mmma bionic
Bug: N/A
Change-Id: Idd1b9ed3737e48a35f8d8628d13e85f1d58f5c93
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
setprogname() does a basename, but we were initializing __progname
directly. Stop doing that, and add some tests.
Test: treehugger
Change-Id: I06f306ade4161b2f0c7e314a3b1b30c9420117b7
Updates getifaddrs() to behave as if RTM_GETLINK requests are not
allowed for non-system apps that have their target SDK set to R.
This change will be reverted when kernel changes enforcing this behavior
are merged, and is purely meant to check for potential appcompat issues
beforehand.
Bug: 141455849
Test: atest bionic-unit-tests-static
Test: atest NetworkInterfaceTest
Test: Connect to Wi-Fi network
Test: Set up hotspot
Test: Cast from device
Test: Pair Bluetooth device
Test: Call getifaddrs() directly from within an app.
Test: Call NetworkInterface#getNetworkInterfaces() from within an app.
Test: Repeat above tests with an app that targets Android R.
Change-Id: I472891d3e8a18c86ae478be1bab1048636aa95b4
The previous implementation of getifaddrs() depended on RTM_GETLINK requests being allowed, returning an error otherwise. This change makes getifaddrs() attempt to get all necessary information from RTM_NEWADDR messages when RTM_NEWLINK messages are not available.
The code is functionally the same when RTM_GETLINK requests are allowed. When RTM_GETLINK requests are denied, only interfaces that have a network address are returned, and physical addresses for these interfaces remain unset.
In addition, this change updates the copyright notice because repohooks asked nicely.
Bug: 141455849
Test: atest bionic-unit-tests-static
Test: atest NetworkInterfaceTest
Test: Connect to Wi-Fi network
Test: Set up hotspot
Test: Cast from device
Test: Pair Bluetooth device
Test: Call getifaddrs() directly from within an app.
Test: Call NetworkInterface#getNetworkInterfaces() from within an app.
Change-Id: Ia47e037d181ca5df6d9fdae19b405cabfafc6b0f
Use O_PATH like musl to let the kernel do the hard work, rather than the
traditional BSD manual scheme.
Also add the most obvious missing tests from reading the man page, plus
a non-obvious test for deleted files.
Bug: http://b/131435126
Test: treehugger
Change-Id: Ie8a8986fea55f045952a81afee377ce8288a49d5
The BSD "Not a typewriter" translation of ENOTTY looks very weird in
2020. The glibc "Inappropriate ioctl for device" is more generic, and
so much less likely to be inappropriate.
Test: strace on a failed fs ioctl
Change-Id: Iad374d6b91ca9f2e4fa1079986fd698feef8359f
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 adds a case for the profiling signal handler (previously just
for native heapprofd profiling) when si_value == 1, corresponding to
traced_perf being the requesting party.
The handler opens /proc/self/{maps,mem}, connects to (init-created)
/dev/socket/traced_perf, and then sends the fds over the socket.
Everything happens synchronously within the signal handler. Socket is
made non-blocking, and we do not retry.
Bug: 144281346
Change-Id: Iea904694caeefe317ed8818e5b150e8819af91c2
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
This is attempt number two, all known failures and issues have
been fixed.
Bug: 137795072
Test: Built both svelte and non-svelte versions. Ran enormous numbers
Test: of performance testing.
Test: Ran scudo unit tests.
Test: Ran bionic unit tests.
Test: Ran libmemunreachable tests.
Test: Ran atest CtsRsBlasTestCases on cuttlefish instance.
Change-Id: Ib0c6ef38b63b7a1f39f4431ed8414afe3a92f9b5
This doesn't add any functionality for now, but there are
a couple of changes in flight that will want to add enumerators
to the mallopt, so let's give them a place to add them.
Bug: 135772972
Bug: 135754954
Change-Id: I6e810020f66070e844500c6fa99b703963365659
Historically we've made a few mistakes where they haven't matched the
right number. And most non-Googlers are much more familiar with the
numbers, so it seems to make sense to rely more on them. Especially in
header files, which we actually expect real people to have to read from
time to time.
Test: treehugger
Change-Id: I0d4a97454ee108de1d32f21df285315c5488d886
This reverts commit 6ffbe97859.
Reason for revert: Droidcop-triggered revert due to breakage b/146543543
Change-Id: Ie9a5b2f6ca5dbc8d3c6cafe70e34838d74e45c56
Bug: 146543543
Bug: 137795072
Test: Built both svelte and non-svelte versions. Ran enormous numbers
Test: of performance testing.
Test: Ran scudo unit tests.
Test: Ran bionic unit tests.
Change-Id: Iec6c98f2bdf6e0d5a6d18dff0c0883fac391c6d5
This is a no-op (kernel returns -EINVAL) if the kernel doesn't understand
the prctl.
Bug: 144799191
Change-Id: I8708e92e31d7a60b2847ae2bc242e46dafb77680
For reasons explained in the code comment, go back to roughly our old
code. The "new" tests are just the old tests resurrected.
This also passes the current toybox xargs tests, which were the
motivation for going back on our earlier decision.
Test: bionic and toybox tests
Change-Id: I33cbcc04107efe81fdbc8166dc9ae844e471173e
Currently, scudo doesn't call libc's malloc initialisers. This causes
problems with any functionality that relies on an initialised__libc_globals
inside of bionic malloc's stubs (e.g. malloc()).
This manifests in two ways (that I can think of):
1. Dispatch tables don't work with scudo, so malloc_debug has never
worked in an executable linked against scudo.
2. Allocators that require initialisation and are called from bionic
malloc's stubs (GWP-ASan) never get initialised.
Bug: 135634846
Test: atest bionic-unit-tests-scudo
Change-Id: I3e3344d7d510ce4e8d3709cd69c8cb0fe5adedda
pthread_atfork may call malloc() during its once-init. This causes
problems with allocators (GWP-ASan) that require explicit initialisation
before calls to malloc().
Bug: 135634846
Test: atest bionic
Change-Id: I1810a00465db99d5aa34fa6f74dea5908a628d3a
Right now, when we read a system property, we first (assuming we've
already looked up the property's prop_info) read the property's serial
number; if we find that the low bit (the dirty bit) in the serial
number is set, we futex-wait for that serial number to become
non-dirty. By doing so, we spare readers from seeing partially-updated
property values if they race with the property service's non-atomic
memcpy to the property value slot. (The futex-wait here isn't
essential to the algorithm: spinning while dirty would suffice,
although it'd be somewhat less efficient.)
The problem with this approach is that readers can wait on the
property service process, potentially causing delays due to scheduling
variance. Property reads are not guaranteed to complete in finite time
right now.
This change makes property reads wait-free and ensures that they
complete in finite time in all cases. In the new approach, we prevent
value tearing by backing up each property we're about to modify and
directing readers to the backup copy if they try to read a property
with the dirty bit set.
(The wait freedom is limited to the case of readers racing against
*one* property update. A writer can still delay readers by rapidly
updating a property --- but after this change, readers can't hang due
to PID 1 scheduling delays.)
I considered adding explicit atomic access to short property values,
but between binary compatibility with the existing property database
and the need to carefully handle transitions of property values
between "short" (compatible with atomics) and "long" (incompatible
with atomics) length domains, I figured the complexity wasn't worth it
and that making property reads wait-free would be adequate.
Test: boots
Bug: 143561649
Change-Id: Ifd3108aedba5a4b157b66af6ca0a4ed084bd5982
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
Using ifuncs allows the linker to select faster versions of libc functions
like strcmp, making linking faster.
The linker continues to first initialize TLS, then call the ifunc
resolvers. There are small amounts of code in Bionic that need to avoid
calling functions selected using ifuncs (generally string.h APIs). I've
tried to compile those pieces with -ffreestanding. Maybe it's unnecessary,
but maybe it could help avoid compiler-inserted memset calls, and maybe
it will be useful later on.
The ifuncs are called in a special early pass using special
__rel[a]_iplt_start / __rel[a]_iplt_end symbols. The linker will encounter
the ifuncs again as R_*_IRELATIVE dynamic relocations, so they're skipped
on the second pass.
Break linker_main.cpp into its own liblinker_main library so it can be
compiled with -ffreestanding.
On walleye, this change fixes a recent 2.3% linker64 start-up time
regression (156.6ms -> 160.2ms), but it also helps the 32-bit time by
about 1.9% on the same benchmark. I'm measuring the run-time using a
synthetic benchmark based on loading libandroid_servers.so.
Test: bionic unit tests, manual benchmarking
Bug: none
Merged-In: Ieb9446c2df13a66fc0d377596756becad0af6995
Change-Id: Ieb9446c2df13a66fc0d377596756becad0af6995
(cherry picked from commit 772bcbb0c2)
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
Upstream keeps rearranging the deckchairs for these, so let's just
switch to the [roughly] one-liners rather than track that...
Test: treehugger
Change-Id: If655cf7a7f316657de44d41fadd43a8c55ee6f23
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
