summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJosh Gao2018-02-13 15:16:17 -0600
committerElliott Hughes2018-04-13 19:34:20 -0500
commit1cc7bd80a6bc7f0908ae05b904cf9a0609609880 (patch)
tree6ade74f70af25a28e6164f287814de91c85530c1 /debuggerd
parent07de83831f9a2bf67e1c4a4df72baed42b26c7e9 (diff)
downloadplatform-system-core-1cc7bd80a6bc7f0908ae05b904cf9a0609609880.tar.gz
platform-system-core-1cc7bd80a6bc7f0908ae05b904cf9a0609609880.tar.xz
platform-system-core-1cc7bd80a6bc7f0908ae05b904cf9a0609609880.zip
debuggerd: remove maximum abort message length.
Let the logging implementation be the imposer of limits. Bug: http://b/64759619 Test: debuggerd_test Change-Id: I8bc73bf2301ce071668993b740880224846a4e75
Diffstat (limited to 'debuggerd')
-rw-r--r--debuggerd/debuggerd_test.cpp11
-rw-r--r--debuggerd/libdebuggerd/tombstone.cpp14
-rw-r--r--debuggerd/libdebuggerd/utility.cpp21
3 files changed, 27 insertions, 19 deletions
diff --git a/debuggerd/debuggerd_test.cpp b/debuggerd/debuggerd_test.cpp
index 397ff2f11..e410be910 100644
--- a/debuggerd/debuggerd_test.cpp
+++ b/debuggerd/debuggerd_test.cpp
@@ -354,7 +354,14 @@ TEST_F(CrasherTest, abort_message) {
354 int intercept_result; 354 int intercept_result;
355 unique_fd output_fd; 355 unique_fd output_fd;
356 StartProcess([]() { 356 StartProcess([]() {
357 android_set_abort_message("abort message goes here"); 357 // Arrived at experimentally;
358 // logd truncates at 4062.
359 // strlen("Abort message: ''") is 17.
360 // That's 4045, but we also want a NUL.
361 char buf[4045 + 1];
362 memset(buf, 'x', sizeof(buf));
363 buf[sizeof(buf) - 1] = '\0';
364 android_set_abort_message(buf);
358 abort(); 365 abort();
359 }); 366 });
360 StartIntercept(&output_fd); 367 StartIntercept(&output_fd);
@@ -366,7 +373,7 @@ TEST_F(CrasherTest, abort_message) {
366 373
367 std::string result; 374 std::string result;
368 ConsumeFd(std::move(output_fd), &result); 375 ConsumeFd(std::move(output_fd), &result);
369 ASSERT_MATCH(result, R"(Abort message: 'abort message goes here')"); 376 ASSERT_MATCH(result, R"(Abort message: 'x{4045}')");
370} 377}
371 378
372TEST_F(CrasherTest, abort_message_backtrace) { 379TEST_F(CrasherTest, abort_message_backtrace) {
diff --git a/debuggerd/libdebuggerd/tombstone.cpp b/debuggerd/libdebuggerd/tombstone.cpp
index 140ef6d23..55d6204ac 100644
--- a/debuggerd/libdebuggerd/tombstone.cpp
+++ b/debuggerd/libdebuggerd/tombstone.cpp
@@ -239,19 +239,23 @@ static void dump_abort_message(log_t* log, Memory* process_memory, uint64_t addr
239 return; 239 return;
240 } 240 }
241 241
242 char msg[512]; 242 // The length field includes the length of the length field itself.
243 if (length >= sizeof(msg)) { 243 if (length < sizeof(size_t)) {
244 _LOG(log, logtype::HEADER, "Abort message too long: claimed length = %zd\n", length); 244 _LOG(log, logtype::HEADER, "Abort message header malformed: claimed length = %zd\n", length);
245 return; 245 return;
246 } 246 }
247 247
248 if (!process_memory->ReadFully(address + sizeof(length), msg, length)) { 248 length -= sizeof(size_t);
249
250 std::vector<char> msg(length);
251 if (!process_memory->ReadFully(address + sizeof(length), &msg[0], length)) {
249 _LOG(log, logtype::HEADER, "Failed to read abort message: %s\n", strerror(errno)); 252 _LOG(log, logtype::HEADER, "Failed to read abort message: %s\n", strerror(errno));
250 return; 253 return;
251 } 254 }
252 255
256 // The abort message should be null terminated already, but just in case...
253 msg[length] = '\0'; 257 msg[length] = '\0';
254 _LOG(log, logtype::HEADER, "Abort message: '%s'\n", msg); 258 _LOG(log, logtype::HEADER, "Abort message: '%s'\n", &msg[0]);
255} 259}
256 260
257static void dump_all_maps(log_t* log, BacktraceMap* map, Memory* process_memory, uint64_t addr) { 261static void dump_all_maps(log_t* log, BacktraceMap* map, Memory* process_memory, uint64_t addr) {
diff --git a/debuggerd/libdebuggerd/utility.cpp b/debuggerd/libdebuggerd/utility.cpp
index d1538653d..1ad180073 100644
--- a/debuggerd/libdebuggerd/utility.cpp
+++ b/debuggerd/libdebuggerd/utility.cpp
@@ -74,25 +74,22 @@ void _LOG(log_t* log, enum logtype ltype, const char* fmt, ...) {
74 && (log->crashed_tid == log->current_tid); 74 && (log->crashed_tid == log->current_tid);
75 static bool write_to_kmsg = should_write_to_kmsg(); 75 static bool write_to_kmsg = should_write_to_kmsg();
76 76
77 char buf[512]; 77 std::string msg;
78 va_list ap; 78 va_list ap;
79 va_start(ap, fmt); 79 va_start(ap, fmt);
80 vsnprintf(buf, sizeof(buf), fmt, ap); 80 android::base::StringAppendV(&msg, fmt, ap);
81 va_end(ap); 81 va_end(ap);
82 82
83 size_t len = strlen(buf); 83 if (msg.empty()) return;
84 if (len <= 0) {
85 return;
86 }
87 84
88 if (write_to_tombstone) { 85 if (write_to_tombstone) {
89 TEMP_FAILURE_RETRY(write(log->tfd, buf, len)); 86 TEMP_FAILURE_RETRY(write(log->tfd, msg.c_str(), msg.size()));
90 } 87 }
91 88
92 if (write_to_logcat) { 89 if (write_to_logcat) {
93 __android_log_buf_write(LOG_ID_CRASH, ANDROID_LOG_FATAL, LOG_TAG, buf); 90 __android_log_buf_write(LOG_ID_CRASH, ANDROID_LOG_FATAL, LOG_TAG, msg.c_str());
94 if (log->amfd_data != nullptr) { 91 if (log->amfd_data != nullptr) {
95 *log->amfd_data += buf; 92 *log->amfd_data += msg;
96 } 93 }
97 94
98 if (write_to_kmsg) { 95 if (write_to_kmsg) {
@@ -100,11 +97,11 @@ void _LOG(log_t* log, enum logtype ltype, const char* fmt, ...) {
100 if (kmsg_fd.get() >= 0) { 97 if (kmsg_fd.get() >= 0) {
101 // Our output might contain newlines which would otherwise be handled by the android logger. 98 // Our output might contain newlines which would otherwise be handled by the android logger.
102 // Split the lines up ourselves before sending to the kernel logger. 99 // Split the lines up ourselves before sending to the kernel logger.
103 if (buf[len - 1] == '\n') { 100 if (msg.back() == '\n') {
104 buf[len - 1] = '\0'; 101 msg.back() = '\0';
105 } 102 }
106 103
107 std::vector<std::string> fragments = android::base::Split(buf, "\n"); 104 std::vector<std::string> fragments = android::base::Split(msg, "\n");
108 for (const std::string& fragment : fragments) { 105 for (const std::string& fragment : fragments) {
109 static constexpr char prefix[] = "<3>DEBUG: "; 106 static constexpr char prefix[] = "<3>DEBUG: ";
110 struct iovec iov[3]; 107 struct iovec iov[3];