From 900f6649e20263516da11f45fd665e31b9ede942 Mon Sep 17 00:00:00 2001 From: David Sarkisyan <281478990+srkyn@users.noreply.github.com> Date: Sat, 15 Aug 2026 16:31:04 -0400 Subject: [PATCH] contain: close parent-death signal setup race --- contain.cc | 27 ++++++++++++++++++++++++--- contain.h | 4 +++- subproc.cc | 14 ++++++++++---- 3 files changed, 37 insertions(+), 8 deletions(-) diff --git a/contain.cc b/contain.cc index e8affd6..105d3b1 100644 --- a/contain.cc +++ b/contain.cc @@ -26,6 +26,7 @@ #include #include #include +#include #include #include #include @@ -94,11 +95,31 @@ static bool containDropPrivs(nsj_t* nsj) { return true; } -static bool containPrepareEnv(nsj_t* nsj) { +bool setupParentDeathSignal(int parent_fd, pid_t expected_parent) { if (prctl(PR_SET_PDEATHSIG, SIGKILL, 0, 0, 0) == -1) { PLOG_E("prctl(PR_SET_PDEATHSIG, SIGKILL)"); return false; } + if (parent_fd != -1) { + struct pollfd pfd = {.fd = parent_fd, .events = POLLIN, .revents = 0}; + if (poll(&pfd, 1, 0) == -1) { + PLOG_E("poll(parent_fd=%d)", parent_fd); + return false; + } + if (pfd.revents & (POLLHUP | POLLERR | POLLNVAL)) { + LOG_E("Parent process exited before PR_SET_PDEATHSIG was installed"); + return false; + } + } else if (getppid() != expected_parent) { + LOG_E("Parent process changed before PR_SET_PDEATHSIG was installed"); + return false; + } + return true; +} + +static bool containPrepareEnv(nsj_t* nsj, int parent_fd, pid_t expected_parent) { + /* Effective UID/GID changes clear PDEATHSIG, so restore it after containUserNs(). */ + RETURN_ON_FAILURE(setupParentDeathSignal(parent_fd, expected_parent)); unsigned long personality = 0; if (nsj->njc.persona_addr_compat_layout()) { personality |= ADDR_COMPAT_LAYOUT; @@ -379,7 +400,7 @@ bool setupFD(nsj_t* nsj, int fd_in, int fd_out, int fd_err) { return true; } -bool containProc(nsj_t* nsj) { +bool containProc(nsj_t* nsj, int parent_fd, pid_t expected_parent) { RETURN_ON_FAILURE(containUserNs(nsj)); RETURN_ON_FAILURE(containInitPidNs(nsj)); RETURN_ON_FAILURE(containInitMountNs(nsj)); @@ -394,7 +415,7 @@ bool containProc(nsj_t* nsj) { RETURN_ON_FAILURE(containCoreSched(nsj)); RETURN_ON_FAILURE(containTSC(nsj)); RETURN_ON_FAILURE(containSetLimits(nsj)); - RETURN_ON_FAILURE(containPrepareEnv(nsj)); + RETURN_ON_FAILURE(containPrepareEnv(nsj, parent_fd, expected_parent)); RETURN_ON_FAILURE(containMakeFdsCOE(nsj)); return true; diff --git a/contain.h b/contain.h index 2b657af..dbfc969 100644 --- a/contain.h +++ b/contain.h @@ -23,13 +23,15 @@ #define NS_CONTAIN_H #include +#include #include "nsjail.h" namespace contain { bool setupFD(nsj_t* nsj, int fd_in, int fd_out, int fd_err); -bool containProc(nsj_t* nsj); +bool setupParentDeathSignal(int parent_fd, pid_t expected_parent); +bool containProc(nsj_t* nsj, int parent_fd, pid_t expected_parent); } // namespace contain diff --git a/subproc.cc b/subproc.cc index d96827d..19212ca 100644 --- a/subproc.cc +++ b/subproc.cc @@ -161,7 +161,12 @@ static const std::string concatArgs(const std::vector& argv) { return ret; } -static void newProc(nsj_t* nsj, int netfd, int fd_in, int fd_out, int fd_err, int pipefd) { +static void newProc( + nsj_t* nsj, int netfd, int fd_in, int fd_out, int fd_err, int pipefd, pid_t expected_parent) { + /* Arm before setup begins; containProc() re-arms after changing credentials. */ + if (!contain::setupParentDeathSignal(pipefd, expected_parent)) { + return; + } if (nsj->njc.has_oom_score_adj()) { std::string score = std::to_string(nsj->njc.oom_score_adj()); if (!util::writeBufToFile( @@ -206,7 +211,7 @@ static void newProc(nsj_t* nsj, int netfd, int fd_in, int fd_out, int fd_err, in } } } - if (!contain::containProc(nsj)) { + if (!contain::containProc(nsj, pipefd, expected_parent)) { return; } if (!nsj->njc.keep_env()) { @@ -524,11 +529,12 @@ pid_t runChild(nsj_t* nsj, int netfd, int fd_in, int fd_out, int fd_err) { flags |= (nsj->njc.clone_newtime() ? CLONE_NEWTIME : 0); if (nsj->njc.mode() == nsjail::Mode::EXECVE) { + const pid_t expected_parent = getppid(); LOG_D("unshare(flags: %s)", cloneFlagsToStr(flags).c_str()); if (unshare(flags) == -1) { PLOG_F("unshare(%s)", cloneFlagsToStr(flags).c_str()); } - newProc(nsj, netfd, fd_in, fd_out, fd_err, -1); + newProc(nsj, netfd, fd_in, fd_out, fd_err, -1, expected_parent); LOG_F("Launching new process failed"); } @@ -546,7 +552,7 @@ pid_t runChild(nsj_t* nsj, int netfd, int fd_in, int fd_out, int fd_err) { pid_t pid = cloneProc(flags, SIGCHLD); if (pid == 0) { close(parent_fd); - newProc(nsj, netfd, fd_in, fd_out, fd_err, child_fd); + newProc(nsj, netfd, fd_in, fd_out, fd_err, child_fd, 0); util::writeToFd(child_fd, &kSubprocErrorChar, sizeof(kSubprocErrorChar)); LOG_F("Launching child process failed"); }