Skip to content

Set CLOEXEC on the listen socket when forking FPM children #11708

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions main/fastcgi.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
21 changes: 20 additions & 1 deletion sapi/fpm/fpm/fpm_children.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand Down
77 changes: 77 additions & 0 deletions sapi/fpm/tests/socket-close-on-exec.phpt
Original file line number Diff line number Diff line change
@@ -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();
?>
13 changes: 13 additions & 0 deletions sapi/fpm/tests/tester.inc
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down