Fix pidfd reuse race condition

encounter · encounter · commit bb4bee83b7bb · 2025-10-09T16:52:12.000-06:00
diff --git a/src/processes.cpp b/src/processes.cpp
@@ -106,9 +106,11 @@ bool ProcessManager::addProcess(Pin<ProcessObject> po) {
 	if (!po) {
 		return false;
 	}
+	pid_t pid;
 	int pidfd;
 	{
 		std::lock_guard lk(po->m);
+		pid = po->pid;
 		pidfd = po->pidfd;
 		if (pidfd < 0) {
 			return false;
@@ -128,7 +130,7 @@ bool ProcessManager::addProcess(Pin<ProcessObject> po) {
 		std::lock_guard lk(m);
 		mReg.emplace(pidfd, std::move(po));
 	}
-	DEBUG_LOG("ProcessManager: tracking pidfd %d\n", pidfd);
+	DEBUG_LOG("ProcessManager: registered pid %d with pidfd %d\n", pid, pidfd);
 	wake();
 	return true;
 }
@@ -181,7 +183,6 @@ void ProcessManager::checkPidfd(int pidfd) {
 			return;
 		}
 		epoll_ctl(mEpollFd, EPOLL_CTL_DEL, pidfd, nullptr);
-		close(pidfd);
 	}
 
 	DEBUG_LOG("ProcessManager: pidfd %d exited: code=%d status=%d\n", pidfd, si.si_code, si.si_status);
@@ -190,10 +191,14 @@ void ProcessManager::checkPidfd(int pidfd) {
 	{
 		std::shared_lock lk(m);
 		auto it = mReg.find(pidfd);
-		if (it == mReg.end()) {
-			return;
+		if (it != mReg.end()) {
+			po = std::move(it->second);
+			mReg.erase(it);
 		}
-		po = it->second.clone();
+	}
+	close(pidfd);
+	if (!po) {
+		return;
 	}
 	{
 		std::lock_guard lk(po->m);
@@ -205,14 +210,6 @@ void ProcessManager::checkPidfd(int pidfd) {
 	}
 	po->cv.notify_all();
 	po->notifyWaiters(false);
-
-	{
-		std::lock_guard lk(m);
-		auto it = mReg.find(pidfd);
-		if (it != mReg.end()) {
-			mReg.erase(it);
-		}
-	}
 }
 
 ProcessManager &processes() {

Original file line number	Diff line number	Diff line change
`@@ -106,9 +106,11 @@ bool ProcessManager::addProcess(Pin<ProcessObject> po) {`
`106`	`106`	`if (!po) {`
`107`	`107`	`return false;`
`108`	`108`	`}`
	`109`	`+ pid_t pid;`
`109`	`110`	`int pidfd;`
`110`	`111`	`{`
`111`	`112`	`std::lock_guard lk(po->m);`
	`113`	`+ pid = po->pid;`
`112`	`114`	`pidfd = po->pidfd;`
`113`	`115`	`if (pidfd < 0) {`
`114`	`116`	`return false;`
`@@ -128,7 +130,7 @@ bool ProcessManager::addProcess(Pin<ProcessObject> po) {`
`128`	`130`	`std::lock_guard lk(m);`
`129`	`131`	`mReg.emplace(pidfd, std::move(po));`
`130`	`132`	`}`
`131`		`- DEBUG_LOG("ProcessManager: tracking pidfd %d\n", pidfd);`
	`133`	`+ DEBUG_LOG("ProcessManager: registered pid %d with pidfd %d\n", pid, pidfd);`
`132`	`134`	`wake();`
`133`	`135`	`return true;`
`134`	`136`	`}`
`@@ -181,7 +183,6 @@ void ProcessManager::checkPidfd(int pidfd) {`
`181`	`183`	`return;`
`182`	`184`	`}`
`183`	`185`	`epoll_ctl(mEpollFd, EPOLL_CTL_DEL, pidfd, nullptr);`
`184`		`- close(pidfd);`
`185`	`186`	`}`
`186`	`187`
`187`	`188`	`DEBUG_LOG("ProcessManager: pidfd %d exited: code=%d status=%d\n", pidfd, si.si_code, si.si_status);`
`@@ -190,10 +191,14 @@ void ProcessManager::checkPidfd(int pidfd) {`
`190`	`191`	`{`
`191`	`192`	`std::shared_lock lk(m);`
`192`	`193`	`auto it = mReg.find(pidfd);`
`193`		`- if (it == mReg.end()) {`
`194`		`- return;`
	`194`	`+ if (it != mReg.end()) {`
	`195`	`+ po = std::move(it->second);`
	`196`	`+ mReg.erase(it);`
`195`	`197`	`}`
`196`		`- po = it->second.clone();`
	`198`	`+ }`
	`199`	`+ close(pidfd);`
	`200`	`+ if (!po) {`
	`201`	`+ return;`
`197`	`202`	`}`
`198`	`203`	`{`
`199`	`204`	`std::lock_guard lk(po->m);`
`@@ -205,14 +210,6 @@ void ProcessManager::checkPidfd(int pidfd) {`
`205`	`210`	`}`
`206`	`211`	`po->cv.notify_all();`
`207`	`212`	`po->notifyWaiters(false);`
`208`		`-`
`209`		`- {`
`210`		`- std::lock_guard lk(m);`
`211`		`- auto it = mReg.find(pidfd);`
`212`		`- if (it != mReg.end()) {`
`213`		`- mReg.erase(it);`
`214`		`- }`
`215`		`- }`
`216`	`213`	`}`
`217`	`214`
`218`	`215`	`ProcessManager &processes() {`