Commit graph

470 commits

Author SHA1 Message Date
Treehugger Robot
372d8a2931 Merge "debuggerd_client: properly wait for completion." 2017-03-28 03:21:39 +00:00
Josh Gao
ae9d7676a5 debuggerd_client: properly wait for completion.
Use an intermediate pipe to detect and report when a requested dump has
completed.

Bug: http://b/35241370
Bug: http://b/35813071
Test: debuggerd_test
Test: manually triggered a background ANR
Change-Id: If14aedf6071288360f1a7853d5a2ee79db121759
2017-03-27 16:11:38 -07:00
Elliott Hughes
f5d727f221 Merge ""Requested dump for tid XXX" message shouldn't be fatal." 2017-03-24 23:29:23 +00:00
Elliott Hughes
561da6aa82 "Requested dump for tid XXX" message shouldn't be fatal.
This just means we were asked to dump, not that something necessarily went
wrong.

Bug: http://b/36191903
Test: builds
Change-Id: I5638b38f3a13081b1e971512f43238010febb59c
2017-03-23 23:04:27 -07:00
Christopher Ferris
9642232f13 Initialize si_code in siginfo.
There was at least one failure due to si_code being unitialized
and then examined.

Test: Run the 32 bit and 64 bit version of the unit tests on angler.
Change-Id: I5455a2cd29afafcd26a49f696e61141bb48478dc
2017-03-22 18:06:57 -07:00
Christopher Ferris
f5e568e653 Do not access device maps.
It's possible that a device map has memory controlled by a single entry
device driver. Thus, you can deadlock if a process is touching that
device memory and we try to unwind it and also touch that device memory.
Simply skip any attempts to step through, or get function names from
device memory maps.

Bug: 36130325

Test: Ran new unit tests, ran bionic unit tests, ran art ThreadStress.
Change-Id: Ibc62d7ec8106c619ee08968f05e04aea55d7cbfa
2017-03-22 14:55:05 -07:00
Josh Gao
7390d96ff2 Merge "crash_dump: fetch process/thread names before dropping privileges." 2017-03-16 17:45:18 +00:00
Josh Gao
57f58f8e4a crash_dump: fetch process/thread names before dropping privileges.
Processes that don't have dumpable set to 1 cannot have their
process/thread names read by processes that don't have all of their
capabilities. Fetch these names in crash_dump before dropping
privileges.

Bug: http://b/36237221
Test: debuggerd_test
Test: debuggerd -b `pidof android.hardware.bluetooth@1.0-service`
Change-Id: I174769e7b3c1ea9f11f9c8cbdff83028a4225783
2017-03-15 23:30:14 -07:00
Dan Willemsen
e0cd1e043d Enable more modules on linux_bionic builds
Bug: 31559095
Test: Enable host bionic, run soong
Change-Id: Ib4ebd909322cf464b6a40040e4b60ece7d905b6f
2017-03-15 15:44:00 -07:00
Tom Cherry
5b4eb23cfd Remove extraneous .clang-format files
The .clang-format files in the base, debuggerd, adb, libprocinfo, and
fastboot subdirectories each differ slightly from the top level
.clang-format-2 and .clang-format-4, but not in a substantially
meaningful way, as the source files in those directories have not been
re-formatted with clang-format.  Therefore, let's reduce the
differences and use only the two top level clang-format files.

Secondly perform some small clean-up of the top level .clang-format
files.  AllowShortBlocksOnASingleLine is already false in the Google
style, so it can be removed.  AllowShortFunctionsOnASingleLine should
not change between the -2 and -4 versions, so leave it at the Google
default style in both, which is 'All'.

The diff stats for these changes are:

./base/
Old:
640 insertions(+), 531 deletions(-)
New:
563 insertions(+), 808 deletions(-)

./debuggerd/
Old:
910 insertions(+), 886 deletions(-)
New:
991 insertions(+), 1023 deletions(-)

./adb/
Old:
2623 insertions(+), 2886 deletions(-)
New:
2655 insertions(+), 3103 deletions(-)

./libprocinfo/
Old:
2 insertions(+), 1 deletion(-)
New:
4 insertions(+), 18 deletions(-)

./fastboot/
Old:
618 insertions(+), 743 deletions(-)
New:
726 insertions(+), 882 deletions(-)

./init/
Old:
1755 insertions(+), 1866 deletions(-)
New:
1715 insertions(+), 1952 deletions(-)

Test: Above clang-format stats
Change-Id: I3f7b8ab0660c8394c5008ba95ea15e70dd22b55b
2017-03-14 14:06:31 -07:00
Josh Gao
c7fe0600cc crash_dump: fix warnings, turn on -Werror.
Test: mma
Change-Id: I0722fef7b513be976cbbe89f73e8bb7138a80442
2017-03-13 14:13:29 -07:00
Josh Gao
428daafc5b crash_dump: improve logging for when a process dies prematurely.
If a process that's getting dumped dies before crash_dump starts (e.g.
because seccomp immediately kills it after it execs crash_dump),
improve the error message to not just say "target died before we could
attach".

Bug: http://b/36077710
Test: inserted an exit in the handler, inspected output
Change-Id: I7d394c66d60d328b096b15654b3648e1ed711728
2017-03-10 14:52:34 -08:00
Josh Gao
ec91809dae debuggerd_handler: restore errno.
Bug: http://b/31448909
Test: mma
Change-Id: I737d66e8bed5fb31c2558f68608d3df460fa73c9
2017-03-10 14:44:54 -08:00
Josh Gao
9eb4eb1811 libdebuggerd: add compatibility shim.
Avoid breaking internal code when AOSP automerges to internal.

This will be reverted after fixing up the uses on the other end.

Bug: http://b/35858739
Test: treehugger
Change-Id: If1ee03d8d7c218d3ad9f451cfe9a9077753dda02
2017-03-09 12:13:16 -08:00
Josh Gao
e1aa0ca58a debuggerd_handler: implement missing fallback functionality.
Allow the fallback implementation to dump traces and create tombstones
in seccomped processes.

Bug: http://b/35858739
Test: debuggerd -b `pidof media.codec`; killall -ABRT media.codec
Change-Id: I381b283de39a66d8900f1c320d32497d6f2b4ec4
2017-03-09 11:26:05 -08:00
Chenjie Luo
97258aad8a Define _LOG as a weak symbol.
So _LOG could be overridden by customized logging
implementations in non-Android systems.

Bug: 35919515
Test: Test on device
Change-Id: I0885c15353c0b1bf66f6f156e7f502f326b85d57
2017-03-06 15:04:32 -08:00
Treehugger Robot
56e89ade33 Merge changes Ib69a206f,If57cc175
* changes:
  tombstoned: turn off signal handlers.
  tombstoned: create tombstones with 0640 permissions.
2017-03-06 22:26:17 +00:00
Josh Gao
55f79a5953 tombstoned: turn off signal handlers.
Don't try to connect to ourselves in a signal handler (e.g. if someone
does `killall -ABRT tombstoned`).

Test: killall -ABRT tombstoned
Change-Id: Ib69a206f741acb523c9f2883d474c940b6ebfab2
2017-03-06 12:30:25 -08:00
Josh Gao
8830c95def tombstoned: create tombstones with 0640 permissions.
Make tombstones group readable to allow them to be picked up by the
dropbox service.

Bug: http://b/35979630
Test: killall -ABRT rild; dumpsys dropbox
Change-Id: If57cc17563c80d5b5c4887b0937905bffef6b231
2017-03-06 12:30:25 -08:00
Elliott Hughes
12b7129406 Small debuggerd improvements.
Include the ABI in seccomp causes.

Slightly improved command-line usage information.

Fix crasher for seccomp failures.

Bug: N/A
Test: crasher
Change-Id: Ie419ecfe72ee4f5ccf49c927be18350a58a66a90
2017-03-02 19:01:20 -08:00
Josh Gao
b038995d29 Merge "debuggerd: remove obsolete dumpable check." 2017-03-01 22:59:01 +00:00
Josh Gao
981761bbb2 debuggerd: remove obsolete dumpable check.
PR_SET_DUMPABLE is ignored now.

Bug: http://b/35872161
Test: debuggerd -b `pidof surfaceflinger`
Change-Id: Iefd090f2b762d454d1e6ce8061ff5f992974267c
2017-03-01 11:55:16 -08:00
Elliott Hughes
b7788fd454 There's no longer a limit to property names.
Bug: http://b/33926793
Test: boots
Change-Id: I8554d7af74e064c114cf817f5a2ba1247fa2a2db
2017-02-28 14:12:54 -08:00
Treehugger Robot
94aabe47db Merge "debuggerd_test: add capability test." 2017-02-17 21:20:11 +00:00
Josh Gao
502cfd22ba debuggerd_test: add capability test.
Also, remove the dependency on crasher.

Bug: http://b/35100921
Bug: http://b/35241370
Test: /data/nativetest/debuggerd_test/debuggerd_test32
Test: /data/nativetest64/debuggerd_test/debuggerd_test64
Change-Id: I318f6de764d435251417953bf175ba321b59981f
2017-02-17 11:22:21 -08:00
Elliott Hughes
da9e3958d6 Add crasher check for passing a bad pthread_t to pthread_join.
Bug: http://b/35455349
Test: manual
Change-Id: If09454c7104a1e6de7c0edb50ee52118b7ca5eaa
2017-02-17 10:26:48 -08:00
Josh Gao
5ad965bf41 crash_dump: fix overflow.
`1 << 32` overflows, resulting in bogus PR_CAP_AMBIENT_RAISE attempts,
and breaking dumping for processes with capabilities in the top 32 bits.

Bug: http://b/35241370
Test: debuggerd -b `pidof com.android.bluetooth`
Change-Id: I29c45a8bd36bdeb3492c9f74599993c139821088
2017-02-16 20:16:58 -08:00
Josh Gao
2a18b822d5 crash_dump: remove unneeded/faulty checks.
We already check our /proc/`getppid()` fd every time we attach a thread, so
these were unneeded at best. The one that happened after dropping
capabilities was actively wrong, though, because /proc/pid access
checks happen on every operation. (only on some kernels?)

Also, add a check that getppid() doesn't change after opening
/proc/getppid().

Bug: http://b/35241370
Test: debuggerd -b `pidof com.android.bluetooth`
Change-Id: I807439d8c2afd027f3c382face50167a8a7946c4
2017-02-16 19:26:09 -08:00
Josh Gao
c7cd48af58 Merge "libdebuggerd_handler: in-process crash dumping for seccomped processes." 2017-02-16 23:11:52 +00:00
Josh Gao
a70f11331f Merge "crash_dump: make output fd O_APPEND." 2017-02-16 22:43:59 +00:00
Josh Gao
e73c932373 libdebuggerd_handler: in-process crash dumping for seccomped processes.
Do an in-process unwind for processes that have PR_SET_NO_NEW_PRIVS
enabled.

Bug: http://b/34684590
Test: debuggerd_test, killall -ABRT media.codec
Change-Id: I62562ec2c419d6643970100ab1cc0288982a1eed
2017-02-15 17:03:44 -08:00
Josh Gao
f6ad5851e6 crash_dump: fix typos in error messages.
Bug: http://b/34760032
Bug: http://b/35367169
Test: mm
Change-Id: I45fa002d4ca616a41524583228987ab1197a125e
2017-02-15 17:03:30 -08:00
Josh Gao
8a7e703912 crash_dump: make output fd O_APPEND.
Bug: http://b/35209835
Test: mma
Change-Id: I447e3cfa3361f9c8b4b3335d0abccd1fe4c98e0f
2017-02-15 16:25:27 -08:00
Josh Gao
60515bf9f1 debuggerd_handler: don't use snprintf in handler.
snprintf isn't safe to call in the linker after initialization, because
it uses MB_CUR_MAX which is implemented via pthread_getspecific, which
uses TLS slots shared with libc. If the TLS slots are assigned in a
different order between libc.so and the linker, MB_CUR_MAX will
evaluate to an incorrect value, and lead to snprintf doing bad things.

Switch to __libc_format_buffer.

Bug: http://b/35367169
Test: debuggerd -b `pidof zygote`
Change-Id: I9d315cf63e5f3fd2f4545d6e3f707cdbe94ec606
2017-02-15 12:24:09 -08:00
Josh Gao
2f11a25a48 debuggerd_handler: set PR_SET_DUMPABLE before running crash_dump.
Set and restore PR_SET_DUMPABLE when performing a dump, so that
processes that have it implicitly cleared (e.g. services that acquire
filesystem capabilities) still get crash dumps.

Bug: http://b/35174939
Test: debuggerd -b `pidof surfaceflinger`
Change-Id: Ife933c10086e546726dec12a7efa3f9cedfeea60
2017-02-14 21:19:38 -08:00
Josh Gao
d2069632bd debuggerd_handler: raise capabilities before running crash_dump.
Raise CapInh and CapAmb after forking to exec crash_dump, so that it
can ptrace us.

Bug: http://b/35174939
Test: debuggerd -b `pidof surfaceflinger`
Change-Id: I32567010a3603cfa494aae9dc0e3ce73fb86b590
2017-02-14 14:40:47 -08:00
Josh Gao
91ad653c82 crasher: add a case that uses PR_SET_NO_NEW_PRIVS.
Bug: http://b/34684590
Test: crasher no_new_privs
Change-Id: I400d599116e3f3c27f5ea46d260e288cf900e156
2017-02-14 14:40:47 -08:00
Josh Gao
c3c8c029ec debuggerd_handler: don't use waitpid(..., __WCLONE).
waitpid(..., __WCLONE) fails with ECHILD when passed an explicit PID to
wait for. __WALL and __WCLONE don't seem to be necessary when waiting
for a specific pid, so just pass 0 in the flags instead.

Bug: http://b/35327712
Test: /data/nativetest/debuggerd_test/debuggerd_test32 --gtest_filter="*zombie*"
Change-Id: I3dd7a1bdf7ff35fdfbf631429c089ef4e3172855
2017-02-13 17:01:24 -08:00
Josh Gao
c24cc8a9e5 crash_dump: collect open files before dropping caps.
/proc/<pid>/fd is also limited by ptrace_may_access.

Test: manual inspection of "debuggerd -b `pidof zygote`"
Change-Id: I1a28c21c0438fe8729bd8e041c6b418d6a84c586
2017-02-07 13:36:08 -08:00
Josh Gao
7a0ee64f9d debuggerd_test: improve error when crasher fails to exec.
Bug: http://b/35100742
Test: rm /system/bin/crasher && /data/nativetest/debugerd_test/debuggerd_test32
Change-Id: I02faec3b7f7ef62bb8a2ac2af730506e3d28e03e
2017-02-07 13:36:08 -08:00
Josh Gao
a7d7eb6d2a debuggerd_test: fix crasher path.
https://android-review.googlesource.com/#/c/331200 moved crasher to
using soong, which changed its location from /system/xbin/crasher to
/system/bin/crasher.

Bug: http://b/35100742
Test: /data/nativetest/debuggerd_test/debuggerd_test32
Test: /data/nativetest64/debuggerd_test/debuggerd_test64
Change-Id: I16a2050b257277023773cc0c960b5ab36e0c7cd4
2017-02-07 13:13:48 -08:00
Josh Gao
347164cc59 crash_dump: read /proc/<pid>/maps before dropping capabilities.
Reading /proc/<pid>/maps does a ptrace_may_access check, which will
fail if we have fewer capabilities than the target, even if we've
already ptraced it.

Bug: http://b/35070339
Test: debuggerd -b `pidof zygote`
Change-Id: I984a061022bd945a7950b88f6d579e1bd735e893
2017-02-07 12:35:51 -08:00
Josh Gao
54ef57d0b8 debuggerd_handler: fix prctl return value check.
Fixed this when I tested on internal, but failed to copy the fix over
when submitting to AOSP.

Bug: http://b/35070339
Test: `adb bugreport` on angler
Change-Id: Ib84d212e5f890958cd21f5c018fbc6f368138d1e
2017-02-06 21:10:48 -08:00
Josh Gao
279cb8b39a Merge changes from topic 'debuggerd_ambient'
* changes:
  debuggerd_handler: don't use clone(..., SIGCHLD, ...)
  crash_dump: drop capabilities after we ptrace attach.
  crash_dump: use /proc/<pid> fd to check tid process membership.
  debuggerd_handler: raise ambient capset before execing.
  Revert "Give crash_dump CAP_SYS_PTRACE."
2017-02-06 18:37:55 +00:00
Josh Gao
b3ee52e4d0 debuggerd_handler: don't use clone(..., SIGCHLD, ...)
Processes that handle SIGCHLD can race with the crash handler to wait
on the crash_dump process. Use clone flags that cause the forked
child's death to not be reported via SIGCHLD, and don't bail out of
dumping when waitpid returns ECHILD (in case another thread is already
in a waitpid(..., __WALL))

Note that the use of waitid was switched to waitpid, because waitid
doesn't support __WCLONE until kernel version 4.7.

Bug: none
Test: "debuggerd -b `pidof zygote64`" a few times (failed roughly 50%
      of the time previously)
Change-Id: Ia41a26a61f13c6f9aa85c4c2f88aef8d279d35ad
2017-02-02 13:54:39 -08:00
Josh Gao
85bcaf68d3 crash_dump: drop capabilities after we ptrace attach.
Bug: http://b/34853272
Test: debuggerd -b `pidof system_server`
Test: debuggerd -b `pidof zygote`
Change-Id: Ic1e1a4b0eb1f561621800cd4cc9a5b848fc5ffd8
2017-02-02 13:54:38 -08:00
Josh Gao
fe90276aee crash_dump: use /proc/<pid> fd to check tid process membership.
Bug: http://b/34759490
Test: /data/nativetest/debuggerd_test/debuggerd_test32
Test: debuggerd -b `pidof system_server`
Test: debuggerd -b `pidof zygote`
Change-Id: I627692b44977335a9568cd765ad28205f0a61327
2017-02-02 13:54:38 -08:00
Josh Gao
7ae426c731 debuggerd_handler: raise ambient capset before execing.
Raise the ambient capability set to match CapEff so that crash_dump can
inherit all of the capabilities of the dumped process to be able to
ptrace. Note that selinux will prevent crash_dump from actually use
any of the capabilities.

Bug: http://b/34853272
Test: debuggerd -b `pidof system_server`
Test: debuggerd -b `pidof zygote`
Change-Id: I1fe69eff54c1c0a5b3ec63f6fa504b2681c47a88
2017-02-02 13:54:38 -08:00
Elliott Hughes
f4ae6203a9 Merge "Better seccomp/kuser_helper diagnostics from debuggerd." 2017-02-02 17:37:25 +00:00
Elliott Hughes
0ba535976f Better seccomp/kuser_helper diagnostics from debuggerd.
Also switch to Android.bp for crasher.

Bug: http://b/34629282 (seccomp)
Bug: http://b/34705831 (seccomp)
Bug: http://b/34884086 (kuser_helpers)
Test: manual
Change-Id: I8ee79c635518faeba751742919af69a505b5e3e1
2017-02-01 18:43:03 -08:00