From 9d22acc1d769d02aae88f9b343d2043437fca6f2 Mon Sep 17 00:00:00 2001 From: Disconnect3d Date: Sat, 25 Feb 2023 10:40:54 +0100 Subject: [PATCH] Hopefully fix vmmap recursion issues (#1585) * Hopefully fix vmmap recursion issues * fixes * fixes * Add test for issue 1565 * add missing test file * fix makefile (pthread) * fix corefile vmmap case * Fix comments --- pwndbg/gdblib/vmmap.py | 48 +++++++++++---------- tests/gdb-tests/tests/binaries/issue_1565.c | 17 ++++++++ tests/gdb-tests/tests/binaries/makefile | 4 ++ tests/gdb-tests/tests/test_command_vmmap.py | 19 ++++++++ 4 files changed, 66 insertions(+), 22 deletions(-) create mode 100644 tests/gdb-tests/tests/binaries/issue_1565.c diff --git a/pwndbg/gdblib/vmmap.py b/pwndbg/gdblib/vmmap.py index 3c0f15223..acff81868 100644 --- a/pwndbg/gdblib/vmmap.py +++ b/pwndbg/gdblib/vmmap.py @@ -6,7 +6,6 @@ The reason that we need robustness is that not every operating system has /proc/$$/maps, which backs 'info proc mapping'. """ import bisect -import os from typing import Any from typing import List from typing import Optional @@ -88,13 +87,25 @@ def get() -> Tuple[pwndbg.lib.memory.Page, ...]: # Note: debugging a coredump does still show proc.alive == True if not pwndbg.gdblib.proc.alive: return tuple() - pages = [] - pages.extend(proc_pid_maps()) - if ( - not pages - and pwndbg.gdblib.qemu.is_qemu_kernel() - and pwndbg.gdblib.arch.current in ("i386", "x86-64", "aarch64", "riscv:rv64") + if is_corefile(): + return tuple(coredump_maps()) + + proc_maps = proc_pid_maps() + + # The `proc_maps` is usually a tuple of Page objects but it can also be: + # None - when /proc/$pid/maps does not exist/is not available + # tuple() - when the process has no maps yet which happens only during its very early init + # (usually when we attach to a process) + if proc_maps is not None: + return proc_maps + + pages = [] + if pwndbg.gdblib.qemu.is_qemu_kernel() and pwndbg.gdblib.arch.current in ( + "i386", + "x86-64", + "aarch64", + "riscv:rv64", ): # If kernel_vmmap_via_pt is not set to the default value of "deprecated", @@ -112,12 +123,7 @@ def get() -> Tuple[pwndbg.lib.memory.Page, ...]: elif kernel_vmmap == "monitor": pages.extend(kernel_vmmap_via_monitor_info_mem()) - if not pages and is_corefile(): - pages.extend(coredump_maps()) - - # TODO/FIXME: Do we still need it after coredump_maps()? - # Add tests for other cases and see if this is needed e.g. for QEMU user - # if not, remove the code below & cleanup other parts of Pwndbg codebase + # TODO/FIXME: Add tests for QEMU-user targets when this is needed if not pages: # If debuggee is launched from a symlink the debuggee memory maps will be # labeled with symlink path while in normal scenario the /proc/pid/maps @@ -338,13 +344,14 @@ def proc_pid_maps(): Parse the contents of /proc/$PID/maps on the server. Returns: - A list of pwndbg.lib.memory.Page objects. + A tuple of pwndbg.lib.memory.Page objects or None if + /proc/$pid/maps doesn't exist or when we debug a qemu-user target """ # If we debug remotely a qemu-user or qemu-system target, # there is no point of hitting things further if pwndbg.gdblib.qemu.is_qemu(): - return tuple() + return None # Example /proc/$pid/maps # 7f95266fa000-7f95268b5000 r-xp 00000000 08:01 418404 /lib/x86_64-linux-gnu/libc-2.19.so @@ -381,6 +388,10 @@ def proc_pid_maps(): except (OSError, gdb.error): continue else: + return None + + # Process hasn't been fully created yet; it is in Z (zombie) state + if data == "": return tuple() pages = [] @@ -725,10 +736,3 @@ def check_aslr(): # access to procfs. output = gdb.execute("show disable-randomization", to_string=True) return ("is off." in output), "show disable-randomization" - - -@pwndbg.gdblib.events.cont -def mark_pc_as_executable() -> None: - mapping = find(pwndbg.gdblib.regs.pc) - if mapping and not mapping.execute: - mapping.flags |= os.X_OK diff --git a/tests/gdb-tests/tests/binaries/issue_1565.c b/tests/gdb-tests/tests/binaries/issue_1565.c new file mode 100644 index 000000000..0dc802751 --- /dev/null +++ b/tests/gdb-tests/tests/binaries/issue_1565.c @@ -0,0 +1,17 @@ +#include +#include +#include +#include + +void *thread_function(void *arg) { + free(malloc(0x20)); + sleep(100); + return NULL; +} + +int main(void) { + pthread_t thread; + pthread_create(&thread, NULL, thread_function, NULL); + pthread_exit(NULL); + return 0; +} diff --git a/tests/gdb-tests/tests/binaries/makefile b/tests/gdb-tests/tests/binaries/makefile index 731826e95..7a5dbf6cd 100644 --- a/tests/gdb-tests/tests/binaries/makefile +++ b/tests/gdb-tests/tests/binaries/makefile @@ -107,6 +107,10 @@ tls.i386.out: tls.i386.c -target i386-linux-gnu \ -o tls.i386.out tls.i386.c +issue_1565.out: issue_1565.c + @echo "[+] Building issue_1565.out" + ${CC} -g -O0 -o issue_1565.out issue_1565.c -pthread -lpthread + clean : @echo "[+] Cleaning stuff" @rm -f $(COMPILED) $(LINKED) $(COMPILED_ASM) $(LINKED_ASM) $(COMPILED_GO) diff --git a/tests/gdb-tests/tests/test_command_vmmap.py b/tests/gdb-tests/tests/test_command_vmmap.py index 568378b13..83d83fa7d 100644 --- a/tests/gdb-tests/tests/test_command_vmmap.py +++ b/tests/gdb-tests/tests/test_command_vmmap.py @@ -7,6 +7,7 @@ import pwndbg import tests CRASH_SIMPLE_BINARY = tests.binaries.get("crash_simple.out.hardcoded") +BINARY_ISSUE_1565 = tests.binaries.get("issue_1565.out") def get_proc_maps(): @@ -156,3 +157,21 @@ def test_command_vmmap_on_coredump_on_crash_simple_binary(start_binary, unload_f vmmaps = [i.split() for i in vmmaps[2:]] assert_maps() + + +def test_vmmap_issue_1565(start_binary): + """ + https://github.com/pwndbg/pwndbg/issues/1565 + + In tests this bug is reported as: + > gdb.execute("context") + E gdb.error: Error occurred in Python: maximum recursion depth exceeded in comparison + + In a normal GDB session this is reported as: + Exception occurred: context: maximum recursion depth exceeded while calling a Python object () + """ + gdb.execute(f"file {BINARY_ISSUE_1565}") + gdb.execute("break thread_function") + gdb.execute("run") + gdb.execute("next") + gdb.execute("context")