Set CLOEXEC on listened/accepted sockets in the FPM children

Closes GH-11708

Co-authored-by: Jakub Zelenka <[email protected]>

Author: Mikhail Galanin

NEWS

@@ -11,6 +11,10 @@ PHP                                                                        NEWS
   . Fixed GH-11952 (Confusing warning when blocking entity loading via
     libxml_set_external_entity_loader). (nielsdos)
 
+- FPM:
+  . Fixed bug #76067 (system() function call leaks php-fpm listening sockets).
+    (Mikhail Galanin, Jakub Zelenka)
+
 - Standard:
   . Added $before_needle argument to strrchr(). (HypeMC)
 

main/fastcgi.c

@@ -1423,6 +1423,16 @@ int fcgi_accept_request(fcgi_request *req)
 					return -1;
 				}
 
+#if defined(F_SETFD) && defined(FD_CLOEXEC)
+				int fd_attrs = fcntl(req->fd, F_GETFD);
+				if (0 > fd_attrs) {
+					fcgi_log(FCGI_WARNING, "failed to get attributes of the connection socket");
+				}
+				if (0 > fcntl(req->fd, F_SETFD, fd_attrs | FD_CLOEXEC)) {
+					fcgi_log(FCGI_WARNING, "failed to change attribute of the connection socket");
+				}
+#endif
+
 #ifdef _WIN32
 				break;
 #else

sapi/fpm/fpm/fpm_children.c

@@ -167,6 +167,24 @@ struct fpm_child_s *fpm_child_find(pid_t pid) /* {{{ */
 }
 /* }}} */
 
+static int fpm_child_cloexec(void)
+{
+	/* get listening socket attributes so it can be extended */
+	int attrs = fcntl(fpm_globals.listening_socket, F_GETFD);
+	if (0 > attrs) {
+		zlog(ZLOG_WARNING, "failed to get attributes of listening socket, errno: %d", errno);
+		return -1;
+	}
+
+	/* set CLOEXEC to prevent the descriptor leaking to child processes */
+	if (0 > fcntl(fpm_globals.listening_socket, F_SETFD, attrs | FD_CLOEXEC)) {
+		zlog(ZLOG_WARNING, "failed to change attribute of listening socket");
+		return -1;
+	}
+
+	return 0;
+}
+
 static void fpm_child_init(struct fpm_worker_pool_s *wp) /* {{{ */
 {
 	fpm_globals.max_requests = wp->config->pm_max_requests;
@@ -178,7 +196,8 @@ static void fpm_child_init(struct fpm_worker_pool_s *wp) /* {{{ */
 	    0 > fpm_unix_init_child(wp)   ||
 	    0 > fpm_signals_init_child()  ||
 	    0 > fpm_env_init_child(wp)    ||
-	    0 > fpm_php_init_child(wp)) {
+	    0 > fpm_php_init_child(wp)    ||
+	    0 > fpm_child_cloexec()) {
 
 		zlog(ZLOG_ERROR, "[pool %s] child failed to initialize", wp->config->name);
 		exit(FPM_EXIT_SOFTWARE);

sapi/fpm/tests/socket-close-on-exec.phpt

@@ -0,0 +1,77 @@
+--TEST--
+FPM: Set CLOEXEC on the listen and connection socket
+--SKIPIF--
+<?php
+require_once "skipif.inc";
+FPM\Tester::skipIfShellCommandFails('lsof -v 2> /dev/null');
+?>
+--FILE--
+<?php
+
+require_once "tester.inc";
+
+$cfg = <<<'EOT'
+[global]
+error_log = {{FILE:LOG}}
+[unconfined]
+listen = {{ADDR}}
+pm = static
+pm.max_children = 1
+pm.status_listen = {{ADDR[status]}}
+pm.status_path = /status
+env[PATH] = $PATH
+EOT;
+
+$code = <<<'EOT'
+<?php
+
+$fpmPort = $_GET['fpm_port'];
+
+echo "My sockets (expect to see 2 of them - one ESTABLISHED and one LISTEN):\n";
+
+$mypid = getmypid();
+$ph = popen("/bin/sh -c 'lsof -Pn -p$mypid' 2>&1 | grep TCP | grep '127.0.0.1:$fpmPort'", 'r');
+echo stream_get_contents($ph);
+pclose($ph);
+
+echo "\n";
+
+/*
+We expect that both LISTEN (inherited from the master process) and ACCEPTED (ESTABLISHED)
+have F_CLOEXEC and should not appear in the created process.
+
+We grep out sockets from non-FPM port, because they may appear in the environment,
+e.g. PHP-FPM can inherit them from the test-runner. We cannot control the runner,
+but we can test how the FPM behaves.
+*/
+
+echo "Sockets after exec(), expected to be empty:\n";
+$ph = popen("/bin/sh -c 'lsof -Pn -p\$\$' 2>&1 | grep TCP | grep '127.0.0.1:$fpmPort'", 'r');
+var_dump(stream_get_contents($ph));
+pclose($ph);
+
+EOT;
+
+$tester = new FPM\Tester($cfg, $code);
+$tester->start();
+$tester->expectLogStartNotices();
+$tester->request(query: 'fpm_port='.$tester->getPort())->printBody();
+$tester->terminate();
+$tester->expectLogTerminatingNotices();
+$tester->close();
+
+?>
+Done
+--EXPECTF--
+My sockets (expect to see 2 of them - one ESTABLISHED and one LISTEN):
+%S 127.0.0.1:%d->127.0.0.1:%d (ESTABLISHED)
+%S 127.0.0.1:%d (LISTEN)
+
+Sockets after exec(), expected to be empty:
+string(0) ""
+Done
+--CLEAN--
+<?php
+require_once "tester.inc";
+FPM\Tester::clean();
+?>

sapi/fpm/tests/tester.inc

@@ -330,6 +330,19 @@ class Tester
         }
     }
 
+    /**
+     * Skip test if supplied shell command fails.
+     *
+     * @param string $command
+     */
+    static public function skipIfShellCommandFails($command)
+    {
+        $output = system($command, $code);
+        if ($code) {
+            die("skip command '$command' faield with code $code and output: $output");
+        }
+    }
+
     /**
      * Skip if posix extension not loaded.
      */