debuggerd: rethrow the full signal we receive, always.
The previous code assumed that returning would be sufficient to rethrow
signals like SIGSEGV. This is not true, for example, in the case where a
SIGSEGV is sent via kill(2). We were previously only sending the signal
to ourselves in some cases, because using kill(2) would lose information
in the siginfo_t argument. Use rt_tgsigqueueinfo(2) instead to preserve
its contents.
Bug: http://b/27367422
Change-Id: I1be822818d5905461979c7e12dc4e9c25049273b
diff --git a/linker/debugger.cpp b/linker/debugger.cpp
index b7e84ac..d4c7928 100644
--- a/linker/debugger.cpp
+++ b/linker/debugger.cpp
@@ -39,6 +39,7 @@
#include <sys/mman.h>
#include <sys/prctl.h>
#include <sys/socket.h>
+#include <sys/syscall.h>
#include <sys/un.h>
#include <unistd.h>
@@ -61,7 +62,9 @@
DEBUGGER_ACTION_DUMP_BACKTRACE,
};
-/* message sent over the socket */
+// Message sent over the socket.
+// NOTE: Any changes to this structure must also be reflected in
+// system/core/include/cutils/debugger.h.
struct __attribute__((packed)) debugger_msg_t {
int32_t action;
pid_t tid;
@@ -267,26 +270,32 @@
send_debuggerd_packet(info);
- // Remove our net so we fault for real when we return.
+ // We need to return from the signal handler so that debuggerd can dump the
+ // thread that crashed, but returning here does not guarantee that the signal
+ // will be thrown again, even for SIGSEGV and friends, since the signal could
+ // have been sent manually. Resend the signal with rt_tgsigqueueinfo(2) to
+ // preserve the SA_SIGINFO contents.
signal(signal_number, SIG_DFL);
- // These signals are not re-thrown when we resume. This means that
- // crashing due to (say) SIGABRT doesn't work the way you'd expect it
- // to. We work around this by throwing them manually. We don't want
- // to do this for *all* signals because it'll screw up the si_addr for
- // faults like SIGSEGV. It does screw up the si_code, which is why we
- // passed that to debuggerd above.
- switch (signal_number) {
- case SIGABRT:
- case SIGFPE:
-#if defined(SIGSTKFLT)
- case SIGSTKFLT:
-#endif
- case SIGTRAP:
- tgkill(getpid(), gettid(), signal_number);
- break;
- default: // SIGILL, SIGBUS, SIGSEGV
- break;
+ struct siginfo si;
+ if (!info) {
+ memset(&si, 0, sizeof(si));
+ si.si_code = SI_USER;
+ si.si_pid = getpid();
+ si.si_uid = getuid();
+ info = &si;
+ } else if (info->si_code >= 0 || info->si_code == SI_TKILL) {
+ // rt_tgsigqueueinfo(2)'s documentation appears to be incorrect on kernels
+ // that contain commit 66dd34a (3.9+). The manpage claims to only allow
+ // negative si_code values that are not SI_TKILL, but 66dd34a changed the
+ // check to allow all si_code values in calls coming from inside the house.
+ }
+
+ int rc = syscall(SYS_rt_tgsigqueueinfo, getpid(), gettid(), signal_number, info);
+ if (rc != 0) {
+ __libc_format_log(ANDROID_LOG_FATAL, "libc", "failed to resend signal during crash: %s",
+ strerror(errno));
+ _exit(0);
}
}
diff --git a/tests/signal_test.cpp b/tests/signal_test.cpp
index f8fdc3f..8c1e834 100644
--- a/tests/signal_test.cpp
+++ b/tests/signal_test.cpp
@@ -14,12 +14,14 @@
* limitations under the License.
*/
+#include <errno.h>
#include <signal.h>
+#include <sys/syscall.h>
+#include <sys/types.h>
+#include <unistd.h>
#include <gtest/gtest.h>
-#include <errno.h>
-
#include "ScopedSignalHandler.h"
static size_t SIGNAL_MIN() {
@@ -375,3 +377,36 @@
ASSERT_EQ(0, sigprocmask(SIG_SETMASK, &original_set, NULL));
}
+
+#if defined(__BIONIC__)
+TEST(signal, rt_tgsigqueueinfo) {
+ // Test whether rt_tgsigqueueinfo allows sending arbitrary si_code values to self.
+ // If this fails, your kernel needs commit 66dd34a to be backported.
+ static constexpr char error_msg[] =
+ "\nPlease ensure that the following kernel patch has been applied:\n"
+ "* https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=66dd34ad31e5963d72a700ec3f2449291d322921\n";
+ static siginfo received;
+
+ struct sigaction handler = {};
+ handler.sa_sigaction = [](int, siginfo_t* siginfo, void*) { received = *siginfo; };
+ handler.sa_flags = SA_SIGINFO;
+
+ ASSERT_EQ(0, sigaction(SIGUSR1, &handler, nullptr));
+
+ siginfo sent = {};
+
+ sent.si_code = SI_TKILL;
+ ASSERT_EQ(0, syscall(SYS_rt_tgsigqueueinfo, getpid(), gettid(), SIGUSR1, &sent))
+ << "rt_tgsigqueueinfo failed: " << strerror(errno) << error_msg;
+ ASSERT_EQ(sent.si_code, received.si_code) << "rt_tgsigqueueinfo modified si_code, expected "
+ << sent.si_code << ", received " << received.si_code
+ << error_msg;
+
+ sent.si_code = SI_USER;
+ ASSERT_EQ(0, syscall(SYS_rt_tgsigqueueinfo, getpid(), gettid(), SIGUSR1, &sent))
+ << "rt_tgsigqueueinfo failed: " << strerror(errno) << error_msg;
+ ASSERT_EQ(sent.si_code, received.si_code) << "rt_tgsigqueueinfo modified si_code, expected "
+ << sent.si_code << ", received " << received.si_code
+ << error_msg;
+}
+#endif
