Skip to content

Commit 06af583

Browse files
Mikhail Galaninbukka
Mikhail Galanin
andcommitted
Set CLOEXEC on listened/accepted sockets in the FPM children
Co-authored-by: Jakub Zelenka <[email protected]>
1 parent 58ae26a commit 06af583

File tree

4 files changed

+120
-1
lines changed

4 files changed

+120
-1
lines changed

main/fastcgi.c

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1423,6 +1423,16 @@ int fcgi_accept_request(fcgi_request *req)
14231423
return -1;
14241424
}
14251425

1426+
#if defined(F_SETFD) && defined(FD_CLOEXEC)
1427+
int fd_attrs = fcntl(req->fd, F_GETFD);
1428+
if (0 > fd_attrs) {
1429+
fcgi_log(FCGI_WARNING, "failed to get attributes of the connection socket");
1430+
}
1431+
if (0 > fcntl(req->fd, F_SETFD, fd_attrs | FD_CLOEXEC)) {
1432+
fcgi_log(FCGI_WARNING, "failed to change attribute of the connection socket");
1433+
}
1434+
#endif
1435+
14261436
#ifdef _WIN32
14271437
break;
14281438
#else

sapi/fpm/fpm/fpm_children.c

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,24 @@ struct fpm_child_s *fpm_child_find(pid_t pid) /* {{{ */
167167
}
168168
/* }}} */
169169

170+
static int fpm_child_cloexec(void)
171+
{
172+
/* get listening socket attributes so it can be extended */
173+
int attrs = fcntl(fpm_globals.listening_socket, F_GETFD);
174+
if (0 > attrs) {
175+
zlog(ZLOG_WARNING, "failed to get attributes of listening socket, errno: %d", errno);
176+
return -1;
177+
}
178+
179+
/* set CLOEXEC to prevent the descriptor leaking to child processes */
180+
if (0 > fcntl(fpm_globals.listening_socket, F_SETFD, attrs | FD_CLOEXEC)) {
181+
zlog(ZLOG_WARNING, "failed to change attribute of listening socket");
182+
return -1;
183+
}
184+
185+
return 0;
186+
}
187+
170188
static void fpm_child_init(struct fpm_worker_pool_s *wp) /* {{{ */
171189
{
172190
fpm_globals.max_requests = wp->config->pm_max_requests;
@@ -178,7 +196,8 @@ static void fpm_child_init(struct fpm_worker_pool_s *wp) /* {{{ */
178196
0 > fpm_unix_init_child(wp) ||
179197
0 > fpm_signals_init_child() ||
180198
0 > fpm_env_init_child(wp) ||
181-
0 > fpm_php_init_child(wp)) {
199+
0 > fpm_php_init_child(wp) ||
200+
0 > fpm_child_cloexec()) {
182201

183202
zlog(ZLOG_ERROR, "[pool %s] child failed to initialize", wp->config->name);
184203
exit(FPM_EXIT_SOFTWARE);
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
--TEST--
2+
FPM: Set CLOEXEC on the listen and connection socket
3+
--SKIPIF--
4+
<?php
5+
require_once "skipif.inc";
6+
FPM\Tester::skipIfShellCommandFails('lsof -v 2> /dev/null');
7+
?>
8+
--FILE--
9+
<?php
10+
11+
require_once "tester.inc";
12+
13+
$cfg = <<<'EOT'
14+
[global]
15+
error_log = {{FILE:LOG}}
16+
[unconfined]
17+
listen = {{ADDR}}
18+
pm = static
19+
pm.max_children = 1
20+
pm.status_listen = {{ADDR[status]}}
21+
pm.status_path = /status
22+
env[PATH] = $PATH
23+
EOT;
24+
25+
$code = <<<'EOT'
26+
<?php
27+
28+
$fpmPort = $_GET['fpm_port'];
29+
30+
echo "My sockets (expect to see 2 of them - one ESTABLISHED and one LISTEN):\n";
31+
32+
$mypid = getmypid();
33+
$ph = popen("/bin/sh -c 'lsof -Pn -p$mypid' 2>&1 | grep TCP | grep '127.0.0.1:$fpmPort'", 'r');
34+
echo stream_get_contents($ph);
35+
pclose($ph);
36+
37+
echo "\n";
38+
39+
/*
40+
We expect that both LISTEN (inherited from the master process) and ACCEPTED (ESTABLISHED)
41+
have F_CLOEXEC and should not appear in the created process.
42+
43+
We grep out sockets from non-FPM port, because they may appear in the environment,
44+
e.g. PHP-FPM can inherit them from the test-runner. We cannot control the runner,
45+
but we can test how the FPM behaves.
46+
*/
47+
48+
echo "Sockets after exec(), expected to be empty:\n";
49+
$ph = popen("/bin/sh -c 'lsof -Pn -p\$\$' 2>&1 | grep TCP | grep '127.0.0.1:$fpmPort'", 'r');
50+
var_dump(stream_get_contents($ph));
51+
pclose($ph);
52+
53+
EOT;
54+
55+
$tester = new FPM\Tester($cfg, $code);
56+
$tester->start();
57+
$tester->expectLogStartNotices();
58+
$tester->request(query: 'fpm_port='.$tester->getPort())->printBody();
59+
$tester->terminate();
60+
$tester->expectLogTerminatingNotices();
61+
$tester->close();
62+
63+
?>
64+
Done
65+
--EXPECTF--
66+
My sockets (expect to see 2 of them - one ESTABLISHED and one LISTEN):
67+
%S 127.0.0.1:%d->127.0.0.1:%d (ESTABLISHED)
68+
%S 127.0.0.1:%d (LISTEN)
69+
70+
Sockets after exec(), expected to be empty:
71+
string(0) ""
72+
Done
73+
--CLEAN--
74+
<?php
75+
require_once "tester.inc";
76+
FPM\Tester::clean();
77+
?>

sapi/fpm/tests/tester.inc

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,19 @@ class Tester
330330
}
331331
}
332332

333+
/**
334+
* Skip test if supplied shell command fails.
335+
*
336+
* @param string $command
337+
*/
338+
static public function skipIfShellCommandFails($command)
339+
{
340+
$output = system($command, $code);
341+
if ($code) {
342+
die("skip command '$command' faield with code $code and output: $output");
343+
}
344+
}
345+
333346
/**
334347
* Skip if posix extension not loaded.
335348
*/

0 commit comments

Comments
 (0)