Merge "Fix scudo fault address processing."
diff --git a/debuggerd/Android.bp b/debuggerd/Android.bp
index ad0231d..5c7d847 100644
--- a/debuggerd/Android.bp
+++ b/debuggerd/Android.bp
@@ -293,6 +293,13 @@
"libdebuggerd/test/utility_test.cpp",
],
+ product_variables: {
+ malloc_not_svelte: {
+ srcs: ["libdebuggerd/test/scudo_test.cpp"],
+ header_libs: ["scudo_headers"],
+ },
+ },
+
target: {
android: {
srcs: [
diff --git a/debuggerd/libdebuggerd/include/libdebuggerd/scudo.h b/debuggerd/libdebuggerd/include/libdebuggerd/scudo.h
index a506859..68bfd5b 100644
--- a/debuggerd/libdebuggerd/include/libdebuggerd/scudo.h
+++ b/debuggerd/libdebuggerd/include/libdebuggerd/scudo.h
@@ -34,9 +34,10 @@
class ScudoCrashData {
public:
- ScudoCrashData() = delete;
+ ScudoCrashData() = default;
~ScudoCrashData() = default;
- ScudoCrashData(unwindstack::Memory* process_memory, const ProcessInfo& process_info);
+
+ bool SetErrorInfo(unwindstack::Memory* process_memory, const ProcessInfo& process_info);
bool CrashIsMine() const;
diff --git a/debuggerd/libdebuggerd/scudo.cpp b/debuggerd/libdebuggerd/scudo.cpp
index 27fae25..9483e59 100644
--- a/debuggerd/libdebuggerd/scudo.cpp
+++ b/debuggerd/libdebuggerd/scudo.cpp
@@ -14,6 +14,11 @@
* limitations under the License.
*/
+#include <stdint.h>
+#include <unistd.h>
+
+#include <vector>
+
#include "libdebuggerd/scudo.h"
#include "libdebuggerd/tombstone.h"
@@ -25,54 +30,92 @@
#include "tombstone.pb.h"
-std::unique_ptr<char[]> AllocAndReadFully(unwindstack::Memory* process_memory, uint64_t addr,
- size_t size) {
- auto buf = std::make_unique<char[]>(size);
- if (!process_memory->ReadFully(addr, buf.get(), size)) {
- return std::unique_ptr<char[]>();
- }
- return buf;
-}
-
-ScudoCrashData::ScudoCrashData(unwindstack::Memory* process_memory,
- const ProcessInfo& process_info) {
+bool ScudoCrashData::SetErrorInfo(unwindstack::Memory* process_memory,
+ const ProcessInfo& process_info) {
if (!process_info.has_fault_address) {
- return;
+ return false;
}
- auto stack_depot = AllocAndReadFully(process_memory, process_info.scudo_stack_depot,
- __scudo_get_stack_depot_size());
- auto region_info = AllocAndReadFully(process_memory, process_info.scudo_region_info,
- __scudo_get_region_info_size());
- auto ring_buffer = AllocAndReadFully(process_memory, process_info.scudo_ring_buffer,
- __scudo_get_ring_buffer_size());
+ std::vector<char> stack_depot(__scudo_get_stack_depot_size());
+ if (!process_memory->ReadFully(process_info.scudo_stack_depot, stack_depot.data(),
+ stack_depot.size())) {
+ return false;
+ }
+ std::vector<char> region_info(__scudo_get_region_info_size());
+ if (!process_memory->ReadFully(process_info.scudo_region_info, region_info.data(),
+ region_info.size())) {
+ return false;
+ }
+ std::vector<char> ring_buffer(__scudo_get_ring_buffer_size());
+ if (!process_memory->ReadFully(process_info.scudo_ring_buffer, ring_buffer.data(),
+ ring_buffer.size())) {
+ return false;
+ }
+
+ uintptr_t page_size = getpagesize();
untagged_fault_addr_ = process_info.untagged_fault_address;
- uintptr_t fault_page = untagged_fault_addr_ & ~(PAGE_SIZE - 1);
+ uintptr_t fault_page = untagged_fault_addr_ & ~(page_size - 1);
- uintptr_t memory_begin = fault_page - PAGE_SIZE * 16;
- if (memory_begin > fault_page) {
- return;
+ // Attempt to get 16 pages before the fault page and 16 pages after.
+ constexpr size_t kExtraPages = 16;
+ std::vector<char> memory(page_size * (kExtraPages * 2 + 1));
+
+ // Read faulting page first.
+ size_t memory_index = kExtraPages;
+ if (!process_memory->ReadFully(fault_page, &memory[memory_index * page_size], page_size)) {
+ return false;
}
- uintptr_t memory_end = fault_page + PAGE_SIZE * 16;
- if (memory_end < fault_page) {
- return;
+ // Attempt to read the pages after the fault page, stop as soon as we
+ // fail to read.
+ uintptr_t read_addr = fault_page;
+ if (!__builtin_add_overflow(fault_page, page_size, &read_addr)) {
+ memory_index++;
+ for (size_t i = 0; i < kExtraPages; i++, memory_index++) {
+ if (!process_memory->ReadFully(read_addr, &memory[memory_index * page_size], page_size)) {
+ break;
+ }
+ if (__builtin_add_overflow(read_addr, page_size, &read_addr)) {
+ break;
+ }
+ }
+ }
+ uintptr_t memory_end = read_addr;
+
+ // Attempt to read the pages before the fault page, stop as soon as we
+ // fail to read.
+ memory_index = kExtraPages;
+ if (fault_page > 0) {
+ read_addr = fault_page - page_size;
+ for (size_t i = 0; i < kExtraPages; i++, memory_index--) {
+ if (!process_memory->ReadFully(read_addr, &memory[(memory_index - 1) * page_size],
+ page_size)) {
+ break;
+ }
+ if (read_addr == 0) {
+ memory_index--;
+ break;
+ }
+ read_addr -= page_size;
+ }
+ }
+ size_t start_memory_index = memory_index;
+ uintptr_t memory_begin = fault_page - (kExtraPages - memory_index) * page_size;
+
+ std::vector<long> memory_tags((memory_end - memory_begin) / kTagGranuleSize);
+ read_addr = memory_begin;
+ for (size_t i = 0; i < memory_tags.size(); i++) {
+ memory_tags[i] = process_memory->ReadTag(read_addr);
+ read_addr += kTagGranuleSize;
}
- auto memory = std::make_unique<char[]>(memory_end - memory_begin);
- for (auto i = memory_begin; i != memory_end; i += PAGE_SIZE) {
- process_memory->ReadFully(i, memory.get() + i - memory_begin, PAGE_SIZE);
- }
+ __scudo_get_error_info(
+ &error_info_, process_info.maybe_tagged_fault_address, stack_depot.data(), region_info.data(),
+ ring_buffer.data(), &memory[start_memory_index * page_size],
+ reinterpret_cast<const char*>(memory_tags.data()), memory_begin, memory_end - memory_begin);
- auto memory_tags = std::make_unique<char[]>((memory_end - memory_begin) / kTagGranuleSize);
- for (auto i = memory_begin; i != memory_end; i += kTagGranuleSize) {
- memory_tags[(i - memory_begin) / kTagGranuleSize] = process_memory->ReadTag(i);
- }
-
- __scudo_get_error_info(&error_info_, process_info.maybe_tagged_fault_address, stack_depot.get(),
- region_info.get(), ring_buffer.get(), memory.get(), memory_tags.get(),
- memory_begin, memory_end - memory_begin);
+ return true;
}
bool ScudoCrashData::CrashIsMine() const {
diff --git a/debuggerd/libdebuggerd/test/scudo_test.cpp b/debuggerd/libdebuggerd/test/scudo_test.cpp
new file mode 100644
index 0000000..d8fc6a7
--- /dev/null
+++ b/debuggerd/libdebuggerd/test/scudo_test.cpp
@@ -0,0 +1,233 @@
+/*
+ * Copyright (C) 2022 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <stdlib.h>
+#include <unistd.h>
+
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+#include "libdebuggerd/scudo.h"
+#include "libdebuggerd/types.h"
+#include "unwindstack/Memory.h"
+
+#include "log_fake.h"
+
+#include <inttypes.h>
+
+// This needs to match the kExtraPages from ScudoCrashData::SetErrorInfo.
+constexpr uint64_t kMaxPages = 16;
+
+class MemoryAlwaysZero : public unwindstack::Memory {
+ public:
+ MemoryAlwaysZero() = default;
+ virtual ~MemoryAlwaysZero() = default;
+
+ size_t Read(uint64_t addr, void* buffer, size_t size) override {
+ if (test_unreadable_addrs_.count(addr) != 0) {
+ return 0;
+ }
+ test_read_addrs_.insert(addr);
+ memset(buffer, 0, size);
+ return size;
+ }
+
+ void TestAddUnreadableAddress(uint64_t addr) { test_unreadable_addrs_.insert(addr); }
+
+ void TestClearAddresses() {
+ test_read_addrs_.clear();
+ test_unreadable_addrs_.clear();
+ }
+
+ std::set<uint64_t>& test_read_addrs() { return test_read_addrs_; }
+
+ private:
+ std::set<uint64_t> test_unreadable_addrs_;
+
+ std::set<uint64_t> test_read_addrs_;
+};
+
+TEST(ScudoTest, no_fault_address) {
+ MemoryAlwaysZero process_memory;
+ ProcessInfo info;
+ info.has_fault_address = false;
+ info.untagged_fault_address = 0x5000;
+ info.scudo_stack_depot = 0x1000;
+ info.scudo_region_info = 0x2000;
+ info.scudo_ring_buffer = 0x3000;
+
+ ScudoCrashData crash;
+ ASSERT_FALSE(crash.SetErrorInfo(&process_memory, info));
+
+ info.has_fault_address = true;
+ ASSERT_TRUE(crash.SetErrorInfo(&process_memory, info));
+}
+
+TEST(ScudoTest, scudo_data_read_check) {
+ MemoryAlwaysZero process_memory;
+ ProcessInfo info;
+ info.has_fault_address = true;
+ info.untagged_fault_address = 0x5000;
+ info.scudo_stack_depot = 0x1000;
+ info.scudo_region_info = 0x2000;
+ info.scudo_ring_buffer = 0x3000;
+
+ ScudoCrashData crash;
+ ASSERT_TRUE(crash.SetErrorInfo(&process_memory, info));
+
+ // Stack Depot unreadable
+ process_memory.TestClearAddresses();
+ process_memory.TestAddUnreadableAddress(0x1000);
+ ASSERT_FALSE(crash.SetErrorInfo(&process_memory, info));
+
+ // The Region Info doesn't exist for 32 bit.
+#if defined(__LP64__)
+ // Region Info unreadable
+ process_memory.TestClearAddresses();
+ process_memory.TestAddUnreadableAddress(0x2000);
+ ASSERT_FALSE(crash.SetErrorInfo(&process_memory, info));
+#endif
+
+ // Ring Buffer unreadable
+ process_memory.TestClearAddresses();
+ process_memory.TestAddUnreadableAddress(0x3000);
+ ASSERT_FALSE(crash.SetErrorInfo(&process_memory, info));
+
+ // Verify that with all scudo data readable, the error info works.
+ process_memory.TestClearAddresses();
+ ASSERT_TRUE(crash.SetErrorInfo(&process_memory, info));
+}
+
+TEST(ScudoTest, fault_page_unreadable) {
+ MemoryAlwaysZero process_memory;
+ ProcessInfo info;
+ info.has_fault_address = true;
+ info.untagged_fault_address = 0x5124;
+ info.scudo_stack_depot = 0x1000;
+ info.scudo_region_info = 0x2000;
+ info.scudo_ring_buffer = 0x3000;
+
+ ScudoCrashData crash;
+ ASSERT_TRUE(crash.SetErrorInfo(&process_memory, info));
+
+ uint64_t fault_page = info.untagged_fault_address & ~(getpagesize() - 1);
+ process_memory.TestAddUnreadableAddress(fault_page);
+ ASSERT_FALSE(crash.SetErrorInfo(&process_memory, info));
+}
+
+TEST(ScudoTest, pages_before_fault_unreadable) {
+ MemoryAlwaysZero process_memory;
+ ProcessInfo info;
+ info.has_fault_address = true;
+ info.untagged_fault_address = 0x15124;
+ info.scudo_stack_depot = 0x1000;
+ info.scudo_region_info = 0x2000;
+ info.scudo_ring_buffer = 0x3000;
+
+ ScudoCrashData crash;
+ ASSERT_TRUE(crash.SetErrorInfo(&process_memory, info));
+
+ uint64_t page_size = getpagesize();
+ uint64_t fault_page = info.untagged_fault_address & ~(page_size - 1);
+
+ std::vector<uint64_t> expected_reads = {0x1000, 0x2000, 0x3000};
+ for (size_t i = 0; i <= kMaxPages; i++) {
+ expected_reads.emplace_back(fault_page + i * page_size);
+ }
+
+ // Loop through and make pages before the fault page unreadable.
+ for (size_t i = 1; i <= kMaxPages + 1; i++) {
+ process_memory.TestClearAddresses();
+ uint64_t unreadable_addr = fault_page - i * page_size;
+ SCOPED_TRACE(testing::Message()
+ << "Failed at unreadable address 0x" << std::hex << unreadable_addr);
+ process_memory.TestAddUnreadableAddress(unreadable_addr);
+ ASSERT_TRUE(crash.SetErrorInfo(&process_memory, info));
+ ASSERT_THAT(process_memory.test_read_addrs(),
+ testing::UnorderedElementsAreArray(expected_reads));
+ // Need to add the previous unreadable_addr to the list of expected addresses.
+ expected_reads.emplace_back(unreadable_addr);
+ }
+}
+
+TEST(ScudoTest, pages_after_fault_unreadable) {
+ MemoryAlwaysZero process_memory;
+ ProcessInfo info;
+ info.has_fault_address = true;
+ info.untagged_fault_address = 0x15124;
+ info.scudo_stack_depot = 0x1000;
+ info.scudo_region_info = 0x2000;
+ info.scudo_ring_buffer = 0x3000;
+
+ ScudoCrashData crash;
+ ASSERT_TRUE(crash.SetErrorInfo(&process_memory, info));
+
+ uint64_t page_size = getpagesize();
+ uint64_t fault_page = info.untagged_fault_address & ~(page_size - 1);
+
+ std::vector<uint64_t> expected_reads = {0x1000, 0x2000, 0x3000};
+ for (size_t i = 0; i <= kMaxPages; i++) {
+ expected_reads.emplace_back(fault_page - i * page_size);
+ }
+
+ // Loop through and make pages after the fault page unreadable.
+ for (size_t i = 1; i <= kMaxPages + 1; i++) {
+ process_memory.TestClearAddresses();
+ uint64_t unreadable_addr = fault_page + i * page_size;
+ SCOPED_TRACE(testing::Message()
+ << "Failed at unreadable address 0x" << std::hex << unreadable_addr);
+ process_memory.TestAddUnreadableAddress(unreadable_addr);
+ ASSERT_TRUE(crash.SetErrorInfo(&process_memory, info));
+ ASSERT_THAT(process_memory.test_read_addrs(),
+ testing::UnorderedElementsAreArray(expected_reads));
+ // Need to add the previous unreadable_addr to the list of expected addresses.
+ expected_reads.emplace_back(unreadable_addr);
+ }
+}
+
+// Make sure that if the fault address is low, you won't underflow.
+TEST(ScudoTest, fault_address_low) {
+ MemoryAlwaysZero process_memory;
+ ProcessInfo info;
+ info.has_fault_address = true;
+ info.scudo_stack_depot = 0x21000;
+ info.scudo_region_info = 0x22000;
+ info.scudo_ring_buffer = 0x23000;
+
+ ScudoCrashData crash;
+ ASSERT_TRUE(crash.SetErrorInfo(&process_memory, info));
+
+ uint64_t page_size = getpagesize();
+ for (size_t i = 0; i < kMaxPages + 1; i++) {
+ process_memory.TestClearAddresses();
+ info.untagged_fault_address = 0x124 + i * getpagesize();
+ SCOPED_TRACE(testing::Message()
+ << "Failed with fault address 0x" << std::hex << info.untagged_fault_address);
+ ASSERT_TRUE(crash.SetErrorInfo(&process_memory, info));
+ std::vector<uint64_t> expected_reads = {0x21000, 0x22000, 0x23000};
+ uint64_t fault_page = info.untagged_fault_address & ~(page_size - 1);
+ expected_reads.emplace_back(fault_page);
+ for (size_t j = 1; j <= kMaxPages; j++) {
+ expected_reads.emplace_back(fault_page + j * page_size);
+ }
+ while (fault_page != 0) {
+ fault_page -= page_size;
+ expected_reads.emplace_back(fault_page);
+ }
+ ASSERT_THAT(process_memory.test_read_addrs(),
+ testing::UnorderedElementsAreArray(expected_reads));
+ }
+}
diff --git a/debuggerd/libdebuggerd/tombstone_proto.cpp b/debuggerd/libdebuggerd/tombstone_proto.cpp
index 159ebc8..6e1ce8f 100644
--- a/debuggerd/libdebuggerd/tombstone_proto.cpp
+++ b/debuggerd/libdebuggerd/tombstone_proto.cpp
@@ -193,8 +193,9 @@
static void dump_probable_cause(Tombstone* tombstone, unwindstack::AndroidUnwinder* unwinder,
const ProcessInfo& process_info, const ThreadInfo& main_thread) {
#if defined(USE_SCUDO)
- ScudoCrashData scudo_crash_data(unwinder->GetProcessMemory().get(), process_info);
- if (scudo_crash_data.CrashIsMine()) {
+ ScudoCrashData scudo_crash_data;
+ if (scudo_crash_data.SetErrorInfo(unwinder->GetProcessMemory().get(), process_info) &&
+ scudo_crash_data.CrashIsMine()) {
scudo_crash_data.AddCauseProtos(tombstone, unwinder);
return;
}