From ff88fb0d3adbc67a4f94f5ef7e2b5bcc7a96c8f3 Mon Sep 17 00:00:00 2001 From: Christopher Ferris Date: Mon, 4 Nov 2019 18:40:00 -0800 Subject: Fix allocations escaping malloc debug. When using a FILE object for some malloc debug functions, calling fprintf will trigger an allocation to be put in the object. The problem is that these allocations were not allocated by the malloc debug wrapper and they get freed during the fclose as if they are malloc debug allocation. In most cases, the code will detect the bad pointer and leak the memory, but it might also cause a crash. The fix is to avoid using fprintf so that no allocations are made in the object that survive and need to be freed in the fclose call. Change the MallocXmlElem.h to use a file decsriptor not a FILE object. Add new unit and system tests to detect this case. Bug: 143742907 Test: Ran unit and system tests. Test: Ran bionic unit tests. Change-Id: I524392de822a29483aa5be8f14c680e70033eba2 --- libc/malloc_debug/PointerData.cpp | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) (limited to 'libc/malloc_debug/PointerData.cpp') diff --git a/libc/malloc_debug/PointerData.cpp b/libc/malloc_debug/PointerData.cpp index ec7e42dda..b1e28b712 100644 --- a/libc/malloc_debug/PointerData.cpp +++ b/libc/malloc_debug/PointerData.cpp @@ -554,7 +554,7 @@ bool PointerData::Exists(const void* ptr) { return pointers_.count(pointer) != 0; } -void PointerData::DumpLiveToFile(FILE* fp) { +void PointerData::DumpLiveToFile(int fd) { std::vector list; std::lock_guard pointer_guard(pointer_mutex_); @@ -566,13 +566,13 @@ void PointerData::DumpLiveToFile(FILE* fp) { total_memory += info.size * info.num_allocations; } - fprintf(fp, "Total memory: %zu\n", total_memory); - fprintf(fp, "Allocation records: %zd\n", list.size()); - fprintf(fp, "Backtrace size: %zu\n", g_debug->config().backtrace_frames()); - fprintf(fp, "\n"); + dprintf(fd, "Total memory: %zu\n", total_memory); + dprintf(fd, "Allocation records: %zd\n", list.size()); + dprintf(fd, "Backtrace size: %zu\n", g_debug->config().backtrace_frames()); + dprintf(fd, "\n"); for (const auto& info : list) { - fprintf(fp, "z %d sz %8zu num %zu bt", (info.zygote_child_alloc) ? 1 : 0, info.size, + dprintf(fd, "z %d sz %8zu num %zu bt", (info.zygote_child_alloc) ? 1 : 0, info.size, info.num_allocations); FrameInfoType* frame_info = info.frame_info; if (frame_info != nullptr) { @@ -580,22 +580,22 @@ void PointerData::DumpLiveToFile(FILE* fp) { if (frame_info->frames[i] == 0) { break; } - fprintf(fp, " %" PRIxPTR, frame_info->frames[i]); + dprintf(fd, " %" PRIxPTR, frame_info->frames[i]); } } - fprintf(fp, "\n"); + dprintf(fd, "\n"); if (info.backtrace_info != nullptr) { - fprintf(fp, " bt_info"); + dprintf(fd, " bt_info"); for (const auto& frame : *info.backtrace_info) { - fprintf(fp, " {"); + dprintf(fd, " {"); if (frame.map_info != nullptr && !frame.map_info->name.empty()) { - fprintf(fp, "\"%s\"", frame.map_info->name.c_str()); + dprintf(fd, "\"%s\"", frame.map_info->name.c_str()); } else { - fprintf(fp, "\"\""); + dprintf(fd, "\"\""); } - fprintf(fp, " %" PRIx64, frame.rel_pc); + dprintf(fd, " %" PRIx64, frame.rel_pc); if (frame.function_name.empty()) { - fprintf(fp, " \"\" 0}"); + dprintf(fd, " \"\" 0}"); } else { char* demangled_name = __cxa_demangle(frame.function_name.c_str(), nullptr, nullptr, nullptr); @@ -605,11 +605,11 @@ void PointerData::DumpLiveToFile(FILE* fp) { } else { name = frame.function_name.c_str(); } - fprintf(fp, " \"%s\" %" PRIx64 "}", name, frame.function_offset); + dprintf(fd, " \"%s\" %" PRIx64 "}", name, frame.function_offset); free(demangled_name); } } - fprintf(fp, "\n"); + dprintf(fd, "\n"); } } } -- cgit v1.2.3