Fix gdb attaching to the wrong process 29/320829/2
authorKrzysztof Malysa <k.malysa@samsung.com>
Fri, 7 Mar 2025 18:55:23 +0000 (19:55 +0100)
committerKrzysztof Malysa <k.malysa@samsung.com>
Wed, 12 Mar 2025 11:24:54 +0000 (12:24 +0100)
In the wild, I experienced a case where gumd process terminated with
failed assertion due to the main process exiting and spawned a child
process to exec gdb and get the backtrace. However, just after spawning
the child, the process gets killed because runner's main process exits.
This caused the spawned child to be reparented to PID 1. Then the child
obtained the pid to debug via getppid(), which returned 1, causing
execution of gdb --pid 1.

This could be harmless, but if this was done
under strace -ff it resulted in deadlock in some GDB subprocess
effectively freezing systemd (PID 1). Consequences included indefinite
freeze in ssh clients trying to connect to the emulator - it was quite
problematic impediment in work.

Now the pid obtained using getpid() before forking. Moreover, getting
killed by SIGKILL in case the parent process died was implemented for
the GDB's main process.

Change-Id: I19251d266d6c4bbd7875b9c4ae56f97f4d94e180

src/framework/src/gdbbacktrace.cpp

index 44e00afea5ec122f94322ac81487195081d2cc72..ad773a6c7589e2d33d8519280ba10ebe57bbcaa3 100644 (file)
@@ -31,6 +31,7 @@
 #include <iostream>
 #include <regex>
 #include <sstream>
+#include <sys/prctl.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <sys/wait.h>
@@ -108,9 +109,14 @@ std::string backtraceRead(const std::string &filename)
     return result.str();
 }
 
-void runChild()
+void runChild(pid_t ppid)
 {
     try {
+        if (prctl(PR_SET_PDEATHSIG, SIGKILL, 0, 0, 0)) // Don't outlive parent process
+            exit(EXIT_FAILURE);
+        // If the parent died before prctl(), we won't be SIGKILLed
+        if (ppid != getppid())
+            std::terminate();
         std::string filename = TMP_FILE_PREFIX + std::to_string(getpid());
         int devnullWrite = TEMP_FAILURE_RETRY(open(DEV_NULL.c_str(), O_WRONLY));
         if (-1 == devnullWrite)
@@ -127,8 +133,8 @@ void runChild()
             exit(EXIT_FAILURE);
         if (-1 == TEMP_FAILURE_RETRY(dup2(devnullRead, STDIN_FILENO)))
             exit(EXIT_FAILURE);
-        std::string ppid = std::to_string(getppid());
-        execl("/usr/bin/gdb", "gdb", "--batch", "-n", "-ex", "bt", "--pid", ppid.c_str(),
+        std::string ppidStr = std::to_string(ppid);
+        execl("/usr/bin/gdb", "gdb", "--batch", "-n", "-ex", "bt", "--pid", ppidStr.c_str(),
               (char*)nullptr);
         // gdb failed to start...
     } catch (...) {
@@ -145,13 +151,14 @@ std::string gdbbacktrace(void)
     // Try to become root again so gdb and unlink is possible
     setegid(0);
     seteuid(0);
+    pid_t parentPid = getpid();
     pid_t childPid = fork();
     switch (childPid) {
         case -1:
             printColor("fork() failed", errno);
             return ret;
         case 0:
-            runChild();
+            runChild(parentPid);
         default:
             break;
     }