Repository: pwndbg/pwndbg
Pull request: Fix attachp crash in tests
This fixes #2144. When running the `attachp` tests locally (with `echo 0 | sudo tee /proc/sys/kernel/yama/ptrace_scope` to enable the tests), they would unexpectedly background the process and cause the testing to halt.

The tests currently causes two parallel bash session to exist, and the tests make sure that running `attachp` in different ways would select the current one. Due to an issue with [child bash processes attempting to read from the terminal](https://stackoverflow.com/questions/61525002/fork-and-execvp-unexpected-outcome-when-used-with-sudo), the `SIGTTIN` signal would be sent to the process, causing them to halt.

To reproduce the crashing/backgrounding, you can do `sudo bash &`, and see the bash process immediately backgrounded.

This didn't seem to be an issue when running tests in Docker, potentially due to the lack of a TTY?

Bash was likely used initially for the tests because it's a long living process that can take arbitrary command-line flags (like `-1` repeatably) without ending immediately. To avoid this issue altogether, this PR replaces `bash` with `sleep`, another command that can live for a long time, and take many command-line flags. We just pass the number 1 repeatably, which it will take happily.


Problem statement:
Issue running attachp gdb-tests
I don't know why this isn't showing up on CI, but for months now I haven't been able to run `./tests.sh` locally without the process being backgrounded and stopped. I can consistently reproduce with:

```
~/code/pwndbg attachp-tests*                                                                                                                                                                                  gsgx@gsgx-desktop 11:01:06 PM
❯ ./tests.sh -v test_attachp_command_attaches_to_procname_resolve_none
ZIGPATH set to /home/gsgx/code/pwndbg/.zig
[+] Building tls.i386.c
/home/gsgx/code/pwndbg/.zig/zig cc \
-Wall -DDEBUG=1 -ggdb -O0 -gdwarf-4 \
-target i386-linux-gnu \
-o tls.i386.out tls.i386.c
[+] Building tls.x86-64.c
/home/gsgx/code/pwndbg/.zig/zig cc \
-Wall -DDEBUG=1 -ggdb -O0 -gdwarf-4 \
-target x86_64-linux-gnu \
-o tls.x86-64.out tls.x86-64.c
[+] Building initialized_heap_x64.out
/home/gsgx/code/pwndbg/.zig/zig cc \
-Wall -DDEBUG=1 -ggdb -O0 -gdwarf-4 \
-target x86_64-linux-gnu \
-o initialized_heap_x64.out initialized_heap.c
[+] Building initialized_heap_i386_big.out
/home/gsgx/code/pwndbg/.zig/zig cc \
-Wall -DDEBUG=1 -ggdb -O0 -gdwarf-4 \
-target i386-linux-gnu \
-o initialized_heap_i386_big.out initialized_heap.c
[+] Building onegadget.x86-64.out
/home/gsgx/code/pwndbg/.zig/zig cc \
-Wall -DDEBUG=1 -ggdb -O0 -gdwarf-4 \
-target x86_64-linux-gnu \
-o onegadget.x86-64.out onegadget.c
[+] Building onegadget.i386.out
/home/gsgx/code/pwndbg/.zig/zig cc \
-Wall -DDEBUG=1 -ggdb -O0 -gdwarf-4 \
-target i386-linux-gnu \
-o onegadget.i386.out onegadget.c

Running tests in parallel
zsh: suspended (tty input)  ./tests.sh -v test_attachp_command_attaches_to_procname_resolve_none

~/code/pwndbg attachp-tests*                                                                                                                                                                                  gsgx@gsgx-desktop 11:01:12 PM
❯ fg
[1]  + continued  ./tests.sh -v test_attachp_command_attaches_to_procname_resolve_none
test_attachp_command_attaches_to_procname_resolve_none                 PASSED

pwndbg: loaded 157 pwndbg commands and 47 shell commands. Type pwndbg [--shell | --all] [filter] for a list.
pwndbg: created $rebase, $base, $ida GDB functions (can be used with print/break)
Launching pytest with args: ['/home/gsgx/code/pwndbg/tests/gdb-tests/tests/test_attachp.py::test_attachp_command_attaches_to_procname_resolve_none', '-vvv', '-s', '--showlocals', '--color=yes']
=========================================================================================================== test session starts ============================================================================================================
platform linux -- Python 3.12.3, pytest-8.0.2, pluggy-1.5.0 -- /usr/bin/python
cachedir: .pytest_cache
rootdir: /home/gsgx/code/pwndbg/tests/gdb-tests
plugins: cov-4.1.0
collecting ... collected 1 item

tests/test_attachp.py::test_attachp_command_attaches_to_procname_resolve_none Launching command: ['gdb', '--silent', '--nx', '--nh', '--command', '../../gdbinit.py', '--eval-command', 'set attachp-resolution-method none', '--eval-command', 'attachp tmpsc6kuviy', '--eval-command', 'quit']
PASSED

============================================================================================================ 1 passed in 3.26s =============================================================================================================

test_attachp_command_attaches_to_procname_resolve_none_no_truncate     PASSED

pwndbg: loaded 157 pwndbg commands and 47 shell commands. Type pwndbg [--shell | --all] [filter] for a list.
pwndbg: created $rebase, $base, $ida GDB functions (can be used with print/break)
Launching pytest with args: ['/home/gsgx/code/pwndbg/tests/gdb-tests/tests/test_attachp.py::test_attachp_command_attaches_to_procname_resolve_none_no_truncate', '-vvv', '-s', '--showlocals', '--color=yes']
=========================================================================================================== test session starts ============================================================================================================
platform linux -- Python 3.12.3, pytest-8.0.2, pluggy-1.5.0 -- /usr/bin/python
cachedir: .pytest_cache
rootdir: /home/gsgx/code/pwndbg/tests/gdb-tests
plugins: cov-4.1.0
collecting ... collected 1 item

tests/test_attachp.py::test_attachp_command_attaches_to_procname_resolve_none_no_truncate Launching command: ['gdb', '--silent', '--nx', '--nh', '--command', '../../gdbinit.py', '--eval-command', 'set attachp-resolution-method none', '--eval-command', 'attachp --no-truncate tmp6g5q4pz5', '--eval-command', 'quit']
PASSED

============================================================================================================ 1 passed in 3.27s =============================================================================================================

Tests completed in 3 seconds

*********************************
********* TESTS SUMMARY *********
*********************************
Tests passed or skipped: 2
Tests failed: 0
```

Note the "zsh: suspended (tty input)  ./tests.sh -v ". The same thing happens with `bash`. I thought it might be related to the parallel tests, but it still happens with `./tests.sh --serial`.

(To reproduce you also need to run `echo 0 | sudo tee /proc/sys/kernel/yama/ptrace_scope`)

Hints from issue discussion:

Commit message: Replace bash with sleep in attachp commands to fix crash due to reading from terminal

Patch to review:
diff --git a/tests/gdb-tests/tests/test_attachp.py b/tests/gdb-tests/tests/test_attachp.py
index c4cee17317e..304d5746e81 100644
--- a/tests/gdb-tests/tests/test_attachp.py
+++ b/tests/gdb-tests/tests/test_attachp.py
@@ -25,14 +25,18 @@
     "Test skipped due to inability to attach (needs sudo or sysctl -w kernel.yama.ptrace_scope=0"
 )
 
+FLAG = "1"
+DEFAULT_SLEEP = "10"
+
 
 @pytest.fixture
-def launched_bash_binary():
+def launched_sleep_binary():
     path = tempfile.mktemp()
-    bash_path = subprocess.check_output(["which", "bash"]).decode().strip()
+    bash_path = subprocess.check_output(["which", "sleep"]).decode().strip()
     subprocess.check_output(["cp", bash_path, path])
 
-    process = subprocess.Popen([path], stdout=subprocess.PIPE, stdin=subprocess.PIPE)
+    # Add a default sleep time so the process lives for at least the length of the test
+    process = subprocess.Popen([path, DEFAULT_SLEEP], stdout=subprocess.PIPE, stdin=subprocess.PIPE)
 
     yield process.pid, path
 
@@ -42,8 +46,8 @@ def launched_bash_binary():
 
 
 @pytest.mark.skipif(can_attach is False, reason=REASON_CANNOT_ATTACH)
-def test_attachp_command_attaches_to_procname(launched_bash_binary):
-    pid, binary_path = launched_bash_binary
+def test_attachp_command_attaches_to_procname(launched_sleep_binary):
+    pid, binary_path = launched_sleep_binary
 
     binary_name = binary_path.split("/")[-1]
     result = run_gdb_with_script(pyafter=f"attachp {binary_name}")
@@ -55,8 +59,8 @@ def test_attachp_command_attaches_to_procname(launched_bash_binary):
 
 
 @pytest.mark.skipif(can_attach is False, reason=REASON_CANNOT_ATTACH)
-def test_attachp_command_attaches_to_pid(launched_bash_binary):
-    pid, binary_path = launched_bash_binary
+def test_attachp_command_attaches_to_pid(launched_sleep_binary):
+    pid, binary_path = launched_sleep_binary
 
     result = run_gdb_with_script(pyafter=f"attachp {pid}")
 
@@ -67,11 +71,11 @@ def test_attachp_command_attaches_to_pid(launched_bash_binary):
 
 
 @pytest.mark.skipif(can_attach is False, reason=REASON_CANNOT_ATTACH)
-def test_attachp_command_attaches_to_procname_resolve_none(launched_bash_binary):
-    pid, binary_path = launched_bash_binary
+def test_attachp_command_attaches_to_procname_resolve_none(launched_sleep_binary):
+    pid, binary_path = launched_sleep_binary
 
     process = subprocess.Popen(
-        [binary_path] + ["-i"] * 1000, stdout=subprocess.PIPE, stdin=subprocess.PIPE
+        [binary_path] + [FLAG] * 1000, stdout=subprocess.PIPE, stdin=subprocess.PIPE
     )
 
     binary_name = binary_path.split("/")[-1]
@@ -88,18 +92,24 @@ def test_attachp_command_attaches_to_procname_resolve_none(launched_bash_binary)
     regex += r"use `attach \<pid\>` to attach\n"
     matches = re.search(regex, result).groups()
 
-    expected = (str(pid), getpass.getuser(), binary_path, str(process.pid), getpass.getuser())
+    expected = (
+        str(pid),
+        getpass.getuser(),
+        f"{binary_path} {DEFAULT_SLEEP}",
+        str(process.pid),
+        getpass.getuser(),
+    )
 
     assert matches[:-1] == expected
-    assert matches[-1].startswith(f"{binary_path} -i -i") and " ... " in matches[-1]
+    assert matches[-1].startswith(f"{binary_path} {FLAG} {FLAG}") and " ... " in matches[-1]
 
 
 @pytest.mark.skipif(can_attach is False, reason=REASON_CANNOT_ATTACH)
-def test_attachp_command_attaches_to_procname_resolve_none_no_truncate(launched_bash_binary):
-    pid, binary_path = launched_bash_binary
+def test_attachp_command_attaches_to_procname_resolve_none_no_truncate(launched_sleep_binary):
+    pid, binary_path = launched_sleep_binary
 
     process = subprocess.Popen(
-        [binary_path] + ["-i"] * 1000, stdout=subprocess.PIPE, stdin=subprocess.PIPE
+        [binary_path] + [FLAG] * 1000, stdout=subprocess.PIPE, stdin=subprocess.PIPE
     )
 
     binary_name = binary_path.split("/")[-1]
@@ -113,22 +123,28 @@ def test_attachp_command_attaches_to_procname_resolve_none_no_truncate(launched_
     regex += r"-+  -+  -+  -+\n"
     regex += r" *([0-9]+) +(\S+) +[0-9:-]+ +(.*)\n"
     regex += r" *([0-9]+) +(\S+) +[0-9:-]+ +(.*)\n"
-    regex += r"(?: +-?(?: -i)+(?: | -)?\n)+"
+    regex += rf"(?: +-?(?: {FLAG})+(?: | -)?\n)+"
     regex += r"use `attach \<pid\>` to attach\n"
     matches = re.search(regex, result).groups()
 
-    expected = (str(pid), getpass.getuser(), binary_path, str(process.pid), getpass.getuser())
+    expected = (
+        str(pid),
+        getpass.getuser(),
+        f"{binary_path} {DEFAULT_SLEEP}",
+        str(process.pid),
+        getpass.getuser(),
+    )
 
     assert matches[:-1] == expected
-    assert matches[-1].startswith(f"{binary_path} -i -i")
+    assert matches[-1].startswith(f"{binary_path} {FLAG} {FLAG}")
 
 
 @pytest.mark.skipif(can_attach is False, reason=REASON_CANNOT_ATTACH)
-def test_attachp_command_attaches_to_procname_resolve_ask(launched_bash_binary):
-    pid, binary_path = launched_bash_binary
+def test_attachp_command_attaches_to_procname_resolve_ask(launched_sleep_binary):
+    pid, binary_path = launched_sleep_binary
 
     process = subprocess.Popen(
-        [binary_path] + ["-i"] * 1000, stdout=subprocess.PIPE, stdin=subprocess.PIPE
+        [binary_path] + [FLAG] * 1000, stdout=subprocess.PIPE, stdin=subprocess.PIPE
     )
 
     binary_name = binary_path.split("/")[-1]
@@ -150,13 +166,13 @@ def test_attachp_command_attaches_to_procname_resolve_ask(launched_bash_binary):
     expected = (
         str(pid),
         getpass.getuser(),
-        binary_path,
+        f"{binary_path} {DEFAULT_SLEEP}",
         str(process.pid),
         getpass.getuser(),
     )
 
     assert matches[:-1] == expected
-    assert matches[-1].startswith(f"{binary_path} -i -i") and " ... " in matches[-1]
+    assert matches[-1].startswith(f"{binary_path} {FLAG} {FLAG}") and " ... " in matches[-1]
 
     matches = re.search(r"Attaching to ([0-9]+)", result).groups()
     assert matches == (str(pid),)
@@ -165,11 +181,11 @@ def test_attachp_command_attaches_to_procname_resolve_ask(launched_bash_binary):
 
 
 @pytest.mark.skipif(can_attach is False, reason=REASON_CANNOT_ATTACH)
-def test_attachp_command_attaches_to_procname_resolve_oldest(launched_bash_binary):
-    pid, binary_path = launched_bash_binary
+def test_attachp_command_attaches_to_procname_resolve_oldest(launched_sleep_binary):
+    pid, binary_path = launched_sleep_binary
 
     process = subprocess.Popen(
-        [binary_path] + ["-i"] * 1000, stdout=subprocess.PIPE, stdin=subprocess.PIPE
+        [binary_path] + [FLAG] * 1000, stdout=subprocess.PIPE, stdin=subprocess.PIPE
     )
 
     binary_name = binary_path.split("/")[-1]
@@ -186,11 +202,11 @@ def test_attachp_command_attaches_to_procname_resolve_oldest(launched_bash_binar
 
 
 @pytest.mark.skipif(can_attach is False, reason=REASON_CANNOT_ATTACH)
-def test_attachp_command_attaches_to_procname_resolve_newest(launched_bash_binary):
-    pid, binary_path = launched_bash_binary
+def test_attachp_command_attaches_to_procname_resolve_newest(launched_sleep_binary):
+    pid, binary_path = launched_sleep_binary
 
     process = subprocess.Popen(
-        [binary_path] + ["-i"] * 1000, stdout=subprocess.PIPE, stdin=subprocess.PIPE
+        [binary_path] + [FLAG] * 1000, stdout=subprocess.PIPE, stdin=subprocess.PIPE
     )
 
     binary_name = binary_path.split("/")[-1]


Code review comments:
diff_hunk: @@ -25,14 +25,18 @@
     "Test skipped due to inability to attach (needs sudo or sysctl -w kernel.yama.ptrace_scope=0"
 )
 
+FLAG = "1"
+DEFAULT_SLEEP = "10"
+
 
 @pytest.fixture
-def launched_bash_binary():
+def launched_sleep_binary():
     path = tempfile.mktemp()
-    bash_path = subprocess.check_output(["which", "bash"]).decode().strip()
+    bash_path = subprocess.check_output(["which", "sleep"]).decode().strip()
path: tests/gdb-tests/tests/test_attachp.py
text: @user1:
```suggestion
    sleep_path = subprocess.check_output(["which", "sleep"]).decode().strip()
```