From df65cab17a2257cd12ab70b33a3f7d210bc00b71 Mon Sep 17 00:00:00 2001 From: cptpcrd <31829097+cptpcrd@users.noreply.github.com> Date: Sun, 21 May 2023 12:10:44 -0400 Subject: [PATCH 1/3] Open command pipes as close-on-exec Avoids a race where the pipe could be inherited by another process spawning at about the same time. If the other process didn't exit quickly (e.g. if it was a custom script that did its own looping), it would keep the write end of the pipe open, and so reading from the pipe to try to get the command's output would block. This bug manifested as some custom modules randomly not appearing in the bar, requiring a reload to fix. The custom script had run and exited, but the pipe had been inherited by another process, and the thread that updated the module's output was blocked trying to read from it. --- include/util/command.hpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/include/util/command.hpp b/include/util/command.hpp index c9f238c1..eff9581f 100644 --- a/include/util/command.hpp +++ b/include/util/command.hpp @@ -4,6 +4,7 @@ #include #include #include +#include #ifdef __linux__ #include @@ -68,7 +69,11 @@ inline int close(FILE* fp, pid_t pid) { inline FILE* open(const std::string& cmd, int& pid) { if (cmd == "") return nullptr; int fd[2]; - if (pipe(fd) != 0) { + // Open the pipe with the close-on-exec flag set, so it will not be inherited + // by any other subprocesses launched by other threads (which could result in + // the pipe staying open after this child dies, causing us to hang when trying + // to read from it) + if (pipe2(fd, O_CLOEXEC) != 0) { spdlog::error("Unable to pipe fd"); return nullptr; } From 6163be687d00496c6144f25da290dc59c24bd58f Mon Sep 17 00:00:00 2001 From: cptpcrd <31829097+cptpcrd@users.noreply.github.com> Date: Sun, 21 May 2023 12:13:17 -0400 Subject: [PATCH 2/3] Open network module eventfd as close-on-exec Ensures that it is not leaked to child processes. --- src/modules/network.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/network.cpp b/src/modules/network.cpp index a61edd5c..ef8b2bd4 100644 --- a/src/modules/network.cpp +++ b/src/modules/network.cpp @@ -188,7 +188,7 @@ void waybar::modules::Network::createEventSocket() { throw std::runtime_error("Can't create epoll"); } { - ev_fd_ = eventfd(0, EFD_NONBLOCK); + ev_fd_ = eventfd(0, EFD_NONBLOCK|EFD_CLOEXEC); struct epoll_event event; memset(&event, 0, sizeof(event)); event.events = EPOLLIN | EPOLLET; From 5cbbfd5c8a4737411c57db61073aa919d884e37d Mon Sep 17 00:00:00 2001 From: cptpcrd <31829097+cptpcrd@users.noreply.github.com> Date: Sun, 21 May 2023 12:15:49 -0400 Subject: [PATCH 3/3] Close pipe if fork() fails when spawning processes Prevents potential file descriptor leakage, albeit in a bit of an edge case. --- include/util/command.hpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/util/command.hpp b/include/util/command.hpp index eff9581f..fe0543d2 100644 --- a/include/util/command.hpp +++ b/include/util/command.hpp @@ -82,6 +82,8 @@ inline FILE* open(const std::string& cmd, int& pid) { if (child_pid < 0) { spdlog::error("Unable to exec cmd {}, error {}", cmd.c_str(), strerror(errno)); + ::close(fd[0]); + ::close(fd[1]); return nullptr; }