Commit graph

2104 commits

Author SHA1 Message Date
Elliott Hughes
9d66092bfd mbrtoc16: explain the line that has no test coverage.
We could remove this line, but it seems reasonable to leave it in for
clarification/safety, especially if it's moved after the common success
case?

Test: treehugger
Change-Id: I5f7e0da8397f80018e6d55321b26371790087f5c
2021-11-18 10:11:07 -08:00
Elliott Hughes
afd8fc3f35 Merge "Optimize the mbs fast path slightly." 2021-11-16 22:13:01 +00:00
Elliott Hughes
2c96639eb2 Optimize the mbs fast path slightly.
From a logcat profile:
```
     |--95.06%-- convertPrintable(char*, char const*, unsigned long)
     |    |--13.95%-- [hit in function]
     |    |
     |    |--35.96%-- mbrtoc32
     |    |    |--82.72%-- [hit in function]
     |    |    |
     |    |    |--11.07%-- mbsinit
     |    |    |
     |    |    |--5.96%-- @plt
```
I think we'd assumed that mbsinit() would be inlined, but since these
functions aren't all in wchar.cpp it wasn't being. This change moves the
implementation into a (more clearly named) inline function so we can
trivially reclaim that 11%+6%.

Benchmarks before:
```
-------------------------------------------------------------------
Benchmark                         Time             CPU   Iterations
-------------------------------------------------------------------
BM_stdlib_mbrtowc_1            8.03 ns         7.95 ns     87144997
BM_stdlib_mbrtowc_2            22.0 ns         21.8 ns     32002437
BM_stdlib_mbrtowc_3            30.0 ns         29.7 ns     23517699
BM_stdlib_mbrtowc_4            37.4 ns         37.1 ns     18895204
BM_stdlib_mbstowcs_ascii     792373 ns       782484 ns          890 bytes_per_second=609.389M/s
BM_stdlib_mbstowcs_wide    15836785 ns     15678316 ns           44 bytes_per_second=30.4138M/s
```

Benchmarks after:
```
-------------------------------------------------------------------
Benchmark                         Time             CPU   Iterations
-------------------------------------------------------------------
BM_stdlib_mbrtowc_1            5.76 ns         5.72 ns    121863813
BM_stdlib_mbrtowc_2            17.1 ns         16.9 ns     41487260
BM_stdlib_mbrtowc_3            24.2 ns         24.0 ns     29141629
BM_stdlib_mbrtowc_4            30.3 ns         30.1 ns     23229291
BM_stdlib_mbstowcs_ascii     783506 ns       775389 ns          903 bytes_per_second=614.965M/s
BM_stdlib_mbstowcs_wide    12787003 ns     12672642 ns           55 bytes_per_second=37.6273M/s
```

Bug: http://b/206523398
Test: treehugger
Change-Id: If8c6c39880096ddd2cbd323c68dca82e9849ace6
2021-11-16 11:03:19 -08:00
Elliott Hughes
b6b7e2ee2e Add the missing '--' to shell invocations.
This came up with POSIX recently. Doesn't seem like it matters since
everyone's had this wrong for 40 years, but "meh" --- it's a trivial
fix, and it's strictly correct even if nobody needs this, so let's just
do it...

(Geoff Clare pointed out that my app compat concern "what if someone's
relying on this bug to pass flags to the shell?" isn't relevant because
while you can indeed do that, you then can't pass a command!)

Bug: https://austingroupbugs.net/view.php?id=1440
Test: treehugger
Change-Id: I64f6440da55e2dc29d0136ee62007197d2f00d46
2021-11-04 17:29:35 -07:00
Elliott Hughes
7a2386bf89 Don't open /dev/null until we need to.
This saves a couple of syscalls in the common case, and also lets static
binaries run in a chroot without /dev/null as long as
stdin/stdout/stderr are actually connected to something (which the
toybox maintainer tried to do).

Test: manual with strace
Change-Id: Ic9a28896a07304a3bd428acfd9ddca9d22015f6e
2021-10-28 09:55:27 -07:00
Christopher Ferris
11526e2fc6 Add execinfo functionality.
Bug: 27877410

Test: Add new unit tests.
Change-Id: Id5d7eb27a23f50e99a04f5ee1ab64047ba269bab
2021-10-20 21:53:07 +00:00
Elliott Hughes
cf59e19e22 Add preadv2/pwritev2 wrappers.
They're in glibc, though not in musl.

Also add basic doc comments to the whole of <sys/uio.h>.

Bug: http://b/203002492
Test: treehugger
Change-Id: Ic607f7f349e5b7c9bf66c25b7bd68f827da530d6
2021-10-18 12:58:47 -07:00
Daniele Di Proietto
b6d3c78244 malloc_heapprofd: Avoid a spurious error log
In the following scenario:

* Heapprofd wants to profile a process.
* The process receives the heapprofd signal, so it sets up the ephemeral
  hooks.
* The process does not perform any allocation, so the proper heapprofd
  hook is never installed.
* Heapprofd terminates.
* Now heapprofd wants to start a new profiling session.
* The process receives the heapprofd signal (again).

In the signal handler, no action is needed at this point. The ephemeral
hooks are already setup, so, at the next malloc, the proper heapprofd
hooks will be installed.

Before this commit, the code logged an error message, but still worked
correctly.

This commit basically just skips the error_log below.

Example of the error message that is now suppressed:

```
process: heapprofd: failed to transition kInitialState ->
kInstallingEphemeralHook. current state (possible race): 2
```

Tested by:
* Running a process that calls malloc on input from stdin.
* (Optional, tested both cases) Enable GWP-Asan by calling
  `android_mallopt(M_INITIALIZE_GWP_ASAN, ...`. The call will return
  success.
* Attaching heapprofd:
```
external/perfetto/tools/heap_profile -i 1 -p `adb shell pidof <...>`
```
* Detaching heapprofd (CTRL-C). The trace will be empty.
* (If not enabled before) Enabling GWP-Asan. The call will fail (because
  GWP-Asan detects heapprofd hooks).
* Reattaching heapprofd.
* Triggering some malloc()s in the process. The error log from above
  will not appear in `adb logcat`.
* Detaching heapprofd (CTRL-C). The trace will NOT be empty.

Bug: 192258849
Change-Id: I01699b10ecd19e52e1e77f83fcca955ebd885942
2021-10-07 17:25:00 +01:00
Treehugger Robot
5b4913a599 Merge "Treat static binaries "the same" for the profiling signals." 2021-09-30 16:45:49 +00:00
Lalit Maganti
e8cc2c32ac Merge "bionic: fix broken end atrace events" 2021-09-30 10:45:38 +00:00
Elliott Hughes
377193745d Treat static binaries "the same" for the profiling signals.
Strictly this still isn't quite the same, because they won't actually be
profiled, but at least they won't *crash* now if they're sent a
profiling signal.

Bug: http://b/201497662
Test: treehugger
Change-Id: I0728492eed77584cd850d28897056996387e6671
2021-09-29 17:10:02 -07:00
Lalit Maganti
2aa3f7cb26 bionic: fix broken end atrace events
When calling write on an FD for trace_marker, it is expected that the
pointer passed will be paged into memory. If this isn't the case, the
kernel will ignore the string passed and instead write "<faulted>" to
the ring buffer.

For end events, we were passing a constant string which resides in
the rodata section of the ELF file. If this section is paged out, we
end up not closing atrace stacks correctly leading to very broken traces.

For even more context, see the associated bug.

Fix this issue by reading the constant string to the stack first
which should mean the string is always paged in.

Bug: 197620214
Change-Id: I6a444ac6fe83a6a9fb696c5621e392eca7e9437a
2021-09-29 18:33:27 +01:00
Christopher Ferris
8f9713e237 Fix broken return code of M_INITIALIZE_GWP_ASAN.
When calling android_mallopt using M_INITIALIZE_GWP_ASAN, nothing
was being returned. Fix this, add a test, and also refactor the
code a bit so dynamic and static share the same code.

Test: Unit tests pass in dynamic and static versions.
Test: Passed using both jemalloc and scudo.
Change-Id: Ibe54b6ccabdbd44d2378892e793df393978bc02b
2021-09-20 18:07:07 -07:00
Colin Cross
048f24ed2a Export fts as a static library for use with musl
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
2021-09-08 15:53:10 -07:00
Colin Cross
69bcb8be27 Compile fts.c in libc_openbsd_ndk
fts.c is from openbsd and has compatibility macros to make it compile
as part of bionic.  Move it into libc_openbsd_ndk where it will
get the workarounds from -include openbsd-compat.h instead.

Test: m libc
Change-Id: I213d423af8f010e39460b611e902acbf3561ae7a
2021-09-08 13:26:46 -07:00
Florian Mayer
a4ffabe79b Merge "Reland "Fix GWP hooks not being restored, leading to crashes."" 2021-08-03 07:52:58 +00:00
Florian Mayer
3a0ced8539 Reland "Fix GWP hooks not being restored, leading to crashes."
If the DispatchReset fails, the subsequent iteration has the wrong
idea of what the "original" table is, and if a subsequent DispatchReset
succeeds it unhooks them.

Repro in https://r.android.com/1767868.

Bug: 193012939
Bug: 189776979
Change-Id: I30445c053fcb785669f75d9c83056926d850edce
2021-07-30 17:59:17 +01:00
Colin Cross
695af0da30 POSIX strerror_r returns an error number, not -1
The posix spec says strerror_r returns a positive error number,  not
-1 and set errno.

Test: bionic-unit-tests-static
Change-Id: I6a12d50d046f9caac299bf3bff63e6c9496c1b6f
2021-07-30 09:39:21 -07:00
Bowgo Tsai
8f14b65032 Revert "Adding system property tracing"
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)
2021-07-21 09:15:41 +08:00
Bowgo Tsai
13a960f0ed Revert "bionic_systrace: moving global static variables"
This reverts commit 1e1c7845aa.

Reason for revert: makes property get/set non-reentrant

Bug: 193050299
Test: build and boot a device
Change-Id: If59e3dc25684a3c2b1d3ff74f995311afe6c6e89
Merged-In: If59e3dc25684a3c2b1d3ff74f995311afe6c6e89
(cherry picked from commit 3ec21f527a)
2021-07-21 09:10:41 +08:00
Treehugger Robot
2ef1cd3f44 Merge "Allow the kernel to upgrade ASYNC mode processes to SYNC mode." 2021-07-02 17:58:43 +00:00
Peter Collingbourne
48bf46b968 Allow the kernel to upgrade ASYNC mode processes to SYNC mode.
On devices where the performance of ASYNC mode is similar to SYNC
mode on certain CPUs, OEMs may choose to configure the kernel to
prefer SYNC mode on those CPUs by writing the value "sync" to the
sysfs node: /sys/devices/system/cpu/cpu<N>/mte_tcf_preferred

The kernel will only respect the per-CPU preference if the user program
allows this by specifying the preferred mode as a member of a set of
allowed modes. Since only kernels with r.android.com/1754670 support
specifying multiple modes, fall back to trying to specify a single
mode if that doesn't work.

Bug: 189966263
Change-Id: Ie7ada3b073178b7967f0819cbdadc2d8e3a2c648
2021-07-01 15:39:32 -07:00
Elliott Hughes
79dbdc3c22 Document more clearly that we don't plan on using faccessat2(2).
Test: treehugger
Change-Id: Idea76ab97865bf26c1f6b16200ba2c7d1fe50ee8
2021-06-25 09:16:18 -07:00
Peter Collingbourne
be1c013280 Merge "Disable return PAC in __pthread_start." 2021-06-09 01:14:29 +00:00
Peter Collingbourne
26d83ba7ab Disable return PAC in __pthread_start.
This function doesn't return, but it does appear in stack traces. Avoid
using return PAC in this function because we may end up resetting IA,
which may confuse unwinders due to mismatching keys.

Bug: 189808795
Change-Id: I953da9078acd1d43eb7a47fb11f75caa0099fa12
2021-06-08 16:03:41 -07:00
Florian Mayer
85c7838bd9 Fix dangling pointer in heapprofd API.
We would dlopen heapprofd_client.so, which has a static initializer [1]
that passes a pointer to of its functions to heapprofd_client_api.so.
If we dlclose heapprofd_client.so, this pointer is dangling.

[1]: https://cs.android.com/android/platform/superproject/+/master:external/perfetto/src/profiling/memory/malloc_interceptor_bionic_hooks.cc?q=symbol:g_heap_id

Bug: 189332777
Change-Id: Ia4a9d9dd7c89eceec86c6fac5f4b66de85d7604e
2021-06-02 14:48:53 +01:00
Bram Bonné
5a7f3ef9a8 Enable RTM_GETLINK restrictions on all apps
Extend existing restrictions targeting only apps with API level >= 30 to
all apps.

Actual enforcement happens in SELinux. This change just prevents
logspam.

To be merged when automerge to sc-dev ends.

Bug: 170188668
Test: atest bionic-unit-tests-static
Test: atest NetworkInterfaceTest
Test: Connect to Wi-Fi network
Test: atest CtsSelinuxTargetSdk27TestCases
Test: atest CtsSelinuxTargetSdk28TestCasesTest: atest
CtsSelinuxTargetSdk29TestCases
Test: atest CtsSelinuxTargetSdkCurrentTestCases

Change-Id: If1761354216b23a1e55e6b9606de452899afff0c
2021-04-30 15:52:27 +02:00
Peter Collingbourne
dcbacd676f Avoid prctl(PR_PAC_RESET_KEYS) on devices without PAC support.
Processes loaded from vendor partitions may have their own sandboxes
that would reject the prctl. Because no devices launched with PAC
enabled before S, we can avoid issues on upgrading devices by checking
for PAC support before issuing the prctl.

Bug: 186117046
Change-Id: I9905b963df01c9007d9fb4527273062ea87a5075
2021-04-22 12:17:01 -07:00
Bram Bonné
f2bb4e6cec Merge "Revert "Reland: Soft-enable MAC address restrictions with allowlist."" 2021-04-09 07:56:36 +00:00
Josh Gao
44ec9c3252 Merge "Disable fdtrack post-fork." 2021-04-09 01:08:52 +00:00
Bram Bonné
bca8a4474b Revert "Reland: Soft-enable MAC address restrictions with allowlist."
Revert "Updates CTS tests for MAC address restrictions."

Revert submission 1528409-mac-softrestrict

Reason for revert: App compatibility
Reverted Changes:
I74a50b990:Return anonymized MAC for apps targeting SDK < 30
I8738f7912:Reland: Soft-enable MAC address restrictions with ...
Id13670747:Updates CTS tests for MAC address restrictions.

Change-Id: I64e17cb04acf2862bc657e60694067a456b4f936
2021-04-08 11:39:33 +02:00
Josh Gao
dcc97c0887 Disable fdtrack post-fork.
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
2021-04-07 19:00:45 -07:00
Peter Collingbourne
2b9719e361 Merge "Reset PAC keys on thread creation instead of on zygote fork." 2021-04-06 23:02:29 +00:00
Peter Collingbourne
811d180e89 Reset PAC keys on thread creation instead of on zygote fork.
Resetting PAC keys on fork appears to lead to a number of problems. One
problem is that we are constrained in where we can run C++ code after
forking, and with ART those places are implementation-defined. For
example, in app zygotes, ART turns out to insert "interpreter frames"
in the stack trace. Returning into these interpreter frames may lead
to crashes due to failing the ROP protection check on return.

It seems better to reset keys on thread creation instead. We only need
to reset IA because only this key needs to be reset for reverse-edge
PAC, and resetting the other keys may be incompatible with future ABIs.

Chrome (and potentially other applications) has a sandbox that prevents
the use of the prctl, so we restrict its use to applications targeting
S and above.

Bug: 183024045
Change-Id: I1e6502a7d7df319d424e2b0f653aad9a343ae71b
2021-03-25 14:07:33 -07:00
Elliott Hughes
13a761032f scandir: remove dead code.
This is the second or third time I've scratched my head wondering why
this destructor has no coverage. I was tempted to leave it in with a
comment saying it should never be called, but that seemed sillier than
just replacing it with an assertion.

Test: treehugger
Change-Id: I3442d9f8a391fae668e77c6888a4457ededee494
2021-03-16 16:20:38 -07:00
Peter Collingbourne
03e961e392 Merge "Teach debuggerd to pass the secondary ring buffer to __scudo_get_error_info()." 2021-03-11 01:15:49 +00:00
Peter Collingbourne
6ba27e04df Merge "Add some slack at the end of large allocations when target SDK level < S." 2021-03-09 01:15:54 +00:00
Elliott Hughes
b82f5cfeb2 Improve <sys/xattr.h> coverage.
Also fix a comment copy & paste mistake and some formatting.

Test: treehugger
Change-Id: I0af3ab2eb4f180f86b0ab7d2af260f0f30692fdd
2021-03-08 14:09:43 -08:00
Peter Collingbourne
2659d7b6c2 Add some slack at the end of large allocations when target SDK level < S.
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
2021-03-05 14:29:17 -08:00
Elliott Hughes
f443817ab6 Remove unused (and empty) file.
Spotted while looking at our shiny new coverage numbers. Though how the
change that removed the code from this file without removing the file
made it through code review... Clearly I wasn't paying attention that
day!

Test: treehugger
Change-Id: Id61bb48bae60660d2e5ba9b26f00a68e51157c6d
2021-03-03 14:31:35 -08:00
Josh Gao
974721431d Merge "Add wrappers for pidfd_{open,getfd,send_signal}." 2021-03-01 21:06:17 +00:00
Bram Bonné
dddf75f990 Merge "Add additional app to netlink appcompat allowlist." 2021-02-26 16:42:37 +00:00
Bram Bonné
82c3d89758 Add additional app to netlink appcompat allowlist.
Bug: 180726036
Test: Confirm app no longer errors at start
Change-Id: I9f1c99a13bbfb8dbdf977d52c67a64d400fd9821
2021-02-26 13:26:21 +00:00
Josh Gao
3de19151e5 Add wrappers for pidfd_{open,getfd,send_signal}.
Bug: http://b/172518739
Test: `/data/nativetest64/bionic-unit-tests/bionic-unit-tests --gtest_filter="*pidfd*"` on blueline
Change-Id: Ibae32bbedbcf26535a80a5cbfb55ce180906b610
2021-02-25 13:55:12 -08:00
Treehugger Robot
6161970fa2 Merge "Make __libc_init_scudo() weak for native bridge." 2021-02-24 00:26:50 +00:00
Peter Collingbourne
bf917866f5 Make __libc_init_scudo() weak for native bridge.
__libc_init_scudo() calls directly into the allocator, bypassing the
normal guest to host transition in the native bridge. Therefore we
need to let the native bridge override it with a no-op.

Bug: 159352723
Change-Id: I642c7a058e483cc09335290f66b9c053150fca06
2021-02-23 13:18:03 -08:00
Elliott Hughes
20c023fdb2 iconv(3): ignore src_bytes_left if src_bytes is null.
This is undefined behavior, but glibc and macOS are both lenient, and
someone hit this in the wild, so we may as well be lenient too. (The
only cost is that it's now slightly easier to write code that works on
everything except old versions of Android.)

Bug: https://issuetracker.google.com/180598400
Test: treehugger
Change-Id: Ia217169ea6283cc53f4fbf71e5abfa08356c2049
2021-02-18 10:37:22 -08:00
Peter Collingbourne
2753fc8ee5 Teach debuggerd to pass the secondary ring buffer to __scudo_get_error_info().
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
2021-02-12 12:30:52 -08:00
Mitch Phillips
bfa3688606 Merge "[MemInit] Remove old API, introduce new MemInit API." 2021-02-01 23:32:24 +00:00
Mitch Phillips
9cad8424ff [MemInit] Remove old API, introduce new MemInit API.
Introduces new heap-zero-init API. We've realised that it's better to be
able to individually control MTE and heap zero-init. Having
heap-zero-init not be controllable without affecting MTE affects our
ability to turn off heap-zero-init in zygote-forked applications.

Bug: 135772972
Test: On FVP: atest -s localhost:5555 malloc#zero_init \
Test: malloc#disable_mte heap_tagging_level
Change-Id: I8c6722502733259934c699f4f1269eaf1641a09f
2021-01-25 15:19:31 -08:00