From ced234782911a1e8b4368a9cc5adc42a56c9a3e3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E6=9D=8E=E9=80=9A=E6=B4=B2?= <zhangsongcui3371@sina.com>
Date: Tue, 11 Jul 2023 11:10:10 +0800
Subject: [PATCH] Processing (Linux): fix potential hang when reading large
 buffer

`write` in child process blocks when the pipe is full. However because we are `waitpid`ing, we don't read the pipe, so that the pipe is never free. Therefore `write` in child process blocks forever and `waitpid` hangs.

Fix #493
---
 src/common/processing_linux.c | 78 ++++++++++-------------------------
 1 file changed, 21 insertions(+), 57 deletions(-)

diff --git a/src/common/processing_linux.c b/src/common/processing_linux.c
index 9a468d83c..6f7dadb5e 100644
--- a/src/common/processing_linux.c
+++ b/src/common/processing_linux.c
@@ -5,55 +5,8 @@
 
 #include <stdlib.h>
 #include <unistd.h>
-#include <sys/wait.h>
-#include <sys/time.h>
 #include <signal.h>
-
-#ifdef __linux__
-    #include <sys/syscall.h>
-    #include <poll.h>
-#endif
-
-int waitpid_timeout(pid_t pid, int* status)
-{
-    if (instance.config.processingTimeout <= 0)
-        return waitpid(pid, status, 0);
-
-    uint32_t timeout = (uint32_t) instance.config.processingTimeout;
-
-    #if defined(__linux__) && defined(SYS_pidfd_open) // musl don't define SYS_pidfd_open
-
-    FF_AUTO_CLOSE_FD int pidfd = (int) syscall(SYS_pidfd_open, pid, 0);
-    if (pidfd >= 0)
-    {
-        int res = poll(&(struct pollfd) { .events = POLLIN, .fd = pidfd }, 1, (int) timeout);
-        if (res > 0)
-            return (int) waitpid(pid, status, WNOHANG);
-        else if (res == 0)
-        {
-            kill(pid, SIGTERM);
-            return -62; // -ETIME
-        }
-        return -1;
-    }
-
-    #endif
-
-    uint64_t start = ffTimeGetTick();
-    while (true)
-    {
-        int res = (int) waitpid(pid, status, WNOHANG);
-        if (res != 0)
-            return res;
-        if (ffTimeGetTick() - start < timeout)
-            ffTimeSleep(timeout / 10);
-        else
-        {
-            kill(pid, SIGTERM);
-            return -62; // -ETIME
-        }
-    }
-}
+#include <poll.h>
 
 const char* ffProcessAppendOutput(FFstrbuf* buffer, char* const argv[], bool useStdErr)
 {
@@ -81,16 +34,27 @@ const char* ffProcessAppendOutput(FFstrbuf* buffer, char* const argv[], bool use
     close(pipes[1]);
 
     int FF_AUTO_CLOSE_FD childPipeFd = pipes[0];
-    int status = -1;
-    if(waitpid_timeout(childPid, &status) < 0)
-        return "waitpid(childPid, &status) failed";
-
-    if (!WIFEXITED(status))
-        return "WIFEXITED(status) == false";
-
-    if(WEXITSTATUS(status) == 901)
-        return "WEXITSTATUS(status) == 901 ( execvp failed )";
+    if (instance.config.processingTimeout >= 0)
+    {
+        struct pollfd pollfd = { childPipeFd, POLLIN, 0 };
+        if (poll(&pollfd, 1, (int) instance.config.processingTimeout) == 0)
+        {
+            kill(childPid, SIGTERM);
+            return "poll(&pollfd, 1, (int) instance.config.processingTimeout) timeout";
+        }
+        else if (pollfd.revents & POLLERR)
+        {
+            kill(childPid, SIGTERM);
+            return "poll(&pollfd, 1, (int) instance.config.processingTimeout) error";
+        }
+        else if (pollfd.revents & POLLHUP)
+        {
+            return "Child process closed its end (nothing to read)";
+        }
+    }
 
+    // Note that we only know we have something to read here
+    // However the child process may still block later
     if(!ffAppendFDBuffer(childPipeFd, buffer))
         return "ffAppendFDBuffer(childPipeFd, buffer) failed";