From f5e568e653d0dd6bccc86d1a60db5a2573f75f0e Mon Sep 17 00:00:00 2001 From: Christopher Ferris Date: Wed, 22 Mar 2017 13:18:31 -0700 Subject: [PATCH] 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 --- debuggerd/libdebuggerd/tombstone.cpp | 2 +- include/backtrace/Backtrace.h | 6 +- include/backtrace/BacktraceMap.h | 4 + libbacktrace/Backtrace.cpp | 11 +- libbacktrace/UnwindCurrent.cpp | 12 +- libbacktrace/UnwindPtrace.cpp | 18 ++- libbacktrace/backtrace_test.cpp | 166 +++++++++++++++++++++++++++ 7 files changed, 213 insertions(+), 6 deletions(-) diff --git a/debuggerd/libdebuggerd/tombstone.cpp b/debuggerd/libdebuggerd/tombstone.cpp index c23da4415..0c3844941 100644 --- a/debuggerd/libdebuggerd/tombstone.cpp +++ b/debuggerd/libdebuggerd/tombstone.cpp @@ -283,7 +283,7 @@ static void dump_stack_segment( if (BacktraceMap::IsValid(map) && !map.name.empty()) { line += " " + map.name; uintptr_t offset = 0; - std::string func_name(backtrace->GetFunctionName(stack_data[i], &offset)); + std::string func_name(backtrace->GetFunctionName(stack_data[i], &offset, &map)); if (!func_name.empty()) { line += " (" + func_name; if (offset) { diff --git a/include/backtrace/Backtrace.h b/include/backtrace/Backtrace.h index c896ab806..4f73a658c 100644 --- a/include/backtrace/Backtrace.h +++ b/include/backtrace/Backtrace.h @@ -104,8 +104,10 @@ public: virtual bool Unwind(size_t num_ignore_frames, ucontext_t* context = NULL) = 0; // Get the function name and offset into the function given the pc. - // If the string is empty, then no valid function name was found. - virtual std::string GetFunctionName(uintptr_t pc, uintptr_t* offset); + // If the string is empty, then no valid function name was found, + // or the pc is not in any valid map. + virtual std::string GetFunctionName(uintptr_t pc, uintptr_t* offset, + const backtrace_map_t* map = NULL); // Fill in the map data associated with the given pc. virtual void FillInMap(uintptr_t pc, backtrace_map_t* map); diff --git a/include/backtrace/BacktraceMap.h b/include/backtrace/BacktraceMap.h index df48dfe2b..8ab0dfabb 100644 --- a/include/backtrace/BacktraceMap.h +++ b/include/backtrace/BacktraceMap.h @@ -33,6 +33,10 @@ #include #include +// Special flag to indicate a map is in /dev/. However, a map in +// /dev/ashmem/... does not set this flag. +static constexpr int PROT_DEVICE_MAP = 0x8000; + struct backtrace_map_t { uintptr_t start = 0; uintptr_t end = 0; diff --git a/libbacktrace/Backtrace.cpp b/libbacktrace/Backtrace.cpp index 0d2e11bdf..4354f0b5a 100644 --- a/libbacktrace/Backtrace.cpp +++ b/libbacktrace/Backtrace.cpp @@ -52,7 +52,16 @@ Backtrace::~Backtrace() { } } -std::string Backtrace::GetFunctionName(uintptr_t pc, uintptr_t* offset) { +std::string Backtrace::GetFunctionName(uintptr_t pc, uintptr_t* offset, const backtrace_map_t* map) { + backtrace_map_t map_value; + if (map == nullptr) { + FillInMap(pc, &map_value); + map = &map_value; + } + // If no map is found, or this map is backed by a device, then return nothing. + if (map->start == 0 || (map->flags & PROT_DEVICE_MAP)) { + return ""; + } std::string func_name = GetFunctionNameRaw(pc, offset); return func_name; } diff --git a/libbacktrace/UnwindCurrent.cpp b/libbacktrace/UnwindCurrent.cpp index 4862d9dab..3c509e61a 100644 --- a/libbacktrace/UnwindCurrent.cpp +++ b/libbacktrace/UnwindCurrent.cpp @@ -127,7 +127,7 @@ bool UnwindCurrent::UnwindFromContext(size_t num_ignore_frames, ucontext_t* ucon if (num_ignore_frames == 0) { // GetFunctionName is an expensive call, only do it if we are // keeping the frame. - frame->func_name = GetFunctionName(frame->pc, &frame->func_offset); + frame->func_name = GetFunctionName(frame->pc, &frame->func_offset, &frame->map); if (num_frames > 0) { // Set the stack size for the previous frame. backtrace_frame_data_t* prev = &frames_.at(num_frames-1); @@ -143,6 +143,16 @@ bool UnwindCurrent::UnwindFromContext(size_t num_ignore_frames, ucontext_t* ucon frames_.resize(0); } } + // If the pc is in a device map, then don't try to step. + if (frame->map.flags & PROT_DEVICE_MAP) { + break; + } + // Verify the sp is not in a device map too. + backtrace_map_t map; + FillInMap(frame->sp, &map); + if (map.flags & PROT_DEVICE_MAP) { + break; + } ret = unw_step (cursor.get()); } while (ret > 0 && num_frames < MAX_BACKTRACE_FRAMES); diff --git a/libbacktrace/UnwindPtrace.cpp b/libbacktrace/UnwindPtrace.cpp index 5c73bd66b..42ac1bc07 100644 --- a/libbacktrace/UnwindPtrace.cpp +++ b/libbacktrace/UnwindPtrace.cpp @@ -136,12 +136,28 @@ bool UnwindPtrace::Unwind(size_t num_ignore_frames, ucontext_t* ucontext) { FillInMap(frame->pc, &frame->map); - frame->func_name = GetFunctionName(frame->pc, &frame->func_offset); + frame->func_name = GetFunctionName(frame->pc, &frame->func_offset, &frame->map); num_frames++; + // If the pc is in a device map, then don't try to step. + if (frame->map.flags & PROT_DEVICE_MAP) { + break; + } } else { + // If the pc is in a device map, then don't try to step. + backtrace_map_t map; + FillInMap(pc, &map); + if (map.flags & PROT_DEVICE_MAP) { + break; + } num_ignore_frames--; } + // Verify the sp is not in a device map. + backtrace_map_t map; + FillInMap(sp, &map); + if (map.flags & PROT_DEVICE_MAP) { + break; + } ret = unw_step (&cursor); } while (ret > 0 && num_frames < MAX_BACKTRACE_FRAMES); diff --git a/libbacktrace/backtrace_test.cpp b/libbacktrace/backtrace_test.cpp index 9ca373a7e..24e48cdb4 100644 --- a/libbacktrace/backtrace_test.cpp +++ b/libbacktrace/backtrace_test.cpp @@ -42,6 +42,7 @@ #include #include +#include #include #include #include @@ -1436,6 +1437,171 @@ TEST(libbacktrace, remote_get_function_name_before_unwind) { FinishRemoteProcess(pid); } +static void SetUcontextSp(uintptr_t sp, ucontext_t* ucontext) { +#if defined(__arm__) + ucontext->uc_mcontext.arm_sp = sp; +#elif defined(__aarch64__) + ucontext->uc_mcontext.sp = sp; +#elif defined(__i386__) + ucontext->uc_mcontext.gregs[REG_ESP] = sp; +#elif defined(__x86_64__) + ucontext->uc_mcontext.gregs[REG_RSP] = sp; +#else + UNUSED(sp); + UNUSED(ucontext); + ASSERT_TRUE(false) << "Unsupported architecture"; +#endif +} + +static void SetUcontextPc(uintptr_t pc, ucontext_t* ucontext) { +#if defined(__arm__) + ucontext->uc_mcontext.arm_pc = pc; +#elif defined(__aarch64__) + ucontext->uc_mcontext.pc = pc; +#elif defined(__i386__) + ucontext->uc_mcontext.gregs[REG_EIP] = pc; +#elif defined(__x86_64__) + ucontext->uc_mcontext.gregs[REG_RIP] = pc; +#else + UNUSED(pc); + UNUSED(ucontext); + ASSERT_TRUE(false) << "Unsupported architecture"; +#endif +} + +static void SetUcontextLr(uintptr_t lr, ucontext_t* ucontext) { +#if defined(__arm__) + ucontext->uc_mcontext.arm_lr = lr; +#elif defined(__aarch64__) + ucontext->uc_mcontext.regs[30] = lr; +#elif defined(__i386__) + // The lr is on the stack. + ASSERT_TRUE(lr != 0); + ASSERT_TRUE(ucontext != nullptr); +#elif defined(__x86_64__) + // The lr is on the stack. + ASSERT_TRUE(lr != 0); + ASSERT_TRUE(ucontext != nullptr); +#else + UNUSED(lr); + UNUSED(ucontext); + ASSERT_TRUE(false) << "Unsupported architecture"; +#endif +} + +static constexpr size_t DEVICE_MAP_SIZE = 1024; + +static void SetupDeviceMap(void** device_map) { + // Make sure that anything in a device map will result in fails + // to read. + android::base::unique_fd device_fd(open("/dev/zero", O_RDONLY | O_CLOEXEC)); + + *device_map = mmap(nullptr, 1024, PROT_READ, MAP_PRIVATE, device_fd, 0); + ASSERT_TRUE(*device_map != MAP_FAILED); + + // Make sure the map is readable. + ASSERT_EQ(0, reinterpret_cast(*device_map)[0]); +} + +static void UnwindFromDevice(Backtrace* backtrace, void* device_map) { + uintptr_t device_map_uint = reinterpret_cast(device_map); + + backtrace_map_t map; + backtrace->FillInMap(device_map_uint, &map); + // Verify the flag is set. + ASSERT_EQ(PROT_DEVICE_MAP, map.flags & PROT_DEVICE_MAP); + + // Quick sanity checks. + size_t offset; + ASSERT_EQ(std::string(""), backtrace->GetFunctionName(device_map_uint, &offset)); + ASSERT_EQ(std::string(""), backtrace->GetFunctionName(device_map_uint, &offset, &map)); + ASSERT_EQ(std::string(""), backtrace->GetFunctionName(0, &offset)); + + uintptr_t cur_func_offset = reinterpret_cast(&test_level_one) + 1; + // Now verify the device map flag actually causes the function name to be empty. + backtrace->FillInMap(cur_func_offset, &map); + ASSERT_TRUE((map.flags & PROT_DEVICE_MAP) == 0); + ASSERT_NE(std::string(""), backtrace->GetFunctionName(cur_func_offset, &offset, &map)); + map.flags |= PROT_DEVICE_MAP; + ASSERT_EQ(std::string(""), backtrace->GetFunctionName(cur_func_offset, &offset, &map)); + + ucontext_t ucontext; + + // Create a context that has the pc in the device map, but the sp + // in a non-device map. + memset(&ucontext, 0, sizeof(ucontext)); + SetUcontextSp(reinterpret_cast(&ucontext), &ucontext); + SetUcontextPc(device_map_uint, &ucontext); + SetUcontextLr(cur_func_offset, &ucontext); + + ASSERT_TRUE(backtrace->Unwind(0, &ucontext)); + + // The buffer should only be a single element. + ASSERT_EQ(1U, backtrace->NumFrames()); + const backtrace_frame_data_t* frame = backtrace->GetFrame(0); + ASSERT_EQ(device_map_uint, frame->pc); + ASSERT_EQ(reinterpret_cast(&ucontext), frame->sp); + + // Check what happens when skipping the first frame. + ASSERT_TRUE(backtrace->Unwind(1, &ucontext)); + ASSERT_EQ(0U, backtrace->NumFrames()); + + // Create a context that has the sp in the device map, but the pc + // in a non-device map. + memset(&ucontext, 0, sizeof(ucontext)); + SetUcontextSp(device_map_uint, &ucontext); + SetUcontextPc(cur_func_offset, &ucontext); + SetUcontextLr(cur_func_offset, &ucontext); + + ASSERT_TRUE(backtrace->Unwind(0, &ucontext)); + + // The buffer should only be a single element. + ASSERT_EQ(1U, backtrace->NumFrames()); + frame = backtrace->GetFrame(0); + ASSERT_EQ(cur_func_offset, frame->pc); + ASSERT_EQ(device_map_uint, frame->sp); + + // Check what happens when skipping the first frame. + ASSERT_TRUE(backtrace->Unwind(1, &ucontext)); + ASSERT_EQ(0U, backtrace->NumFrames()); +} + +TEST(libbacktrace, unwind_disallow_device_map_local) { + void* device_map; + SetupDeviceMap(&device_map); + + // Now create an unwind object. + std::unique_ptr backtrace( + Backtrace::Create(BACKTRACE_CURRENT_PROCESS, BACKTRACE_CURRENT_THREAD)); + ASSERT_TRUE(backtrace); + + UnwindFromDevice(backtrace.get(), device_map); + + munmap(device_map, DEVICE_MAP_SIZE); +} + +TEST(libbacktrace, unwind_disallow_device_map_remote) { + void* device_map; + SetupDeviceMap(&device_map); + + // Fork a process to do a remote backtrace. + pid_t pid; + CreateRemoteProcess(&pid); + + // Now create an unwind object. + std::unique_ptr backtrace(Backtrace::Create(pid, pid)); + + // TODO: Currently unwind from context doesn't work on remote + // unwind. Keep this test because the new unwinder should support + // this eventually, or we can delete this test. + // properly with unwind from context. + // UnwindFromDevice(backtrace.get(), device_map); + + FinishRemoteProcess(pid); + + munmap(device_map, DEVICE_MAP_SIZE); +} + #if defined(ENABLE_PSS_TESTS) #include "GetPss.h"