Skip to content

Commit 90ac9e0

Browse files
author
Mikhail Galanin
committed
Set CLOEXEC on listened/accepted sockets in the FPM children
1 parent 4077dad commit 90ac9e0

File tree

3 files changed

+111
-1
lines changed

3 files changed

+111
-1
lines changed

main/fastcgi.c

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

1426+
#ifdef F_SETFD
1427+
# ifdef FD_CLOEXEC
1428+
if (0 > fcntl(req->fd, F_SETFD, fcntl(req->fd, F_GETFD) | FD_CLOEXEC)) {
1429+
fcgi_log(FCGI_WARNING, "failed to change attribute of error_log");
1430+
}
1431+
# endif
1432+
#endif
1433+
14261434
#ifdef _WIN32
14271435
break;
14281436
#else

sapi/fpm/fpm/fpm_children.c

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

170+
static int fpm_child_cloexec(void)
171+
{
172+
/* If PHP code invokes pcntl_fork()/exec(), we don't want the external programm to inherit the descriptor.
173+
If the external process accidentally uses the socket it will likely break the communication */
174+
int attrs = fcntl(fpm_globals.listening_socket, F_GETFD);
175+
if (0 > attrs) {
176+
zlog(ZLOG_WARNING, "failed to get attributes of listening socket, errno: %d", errno);
177+
return -1;
178+
}
179+
180+
/* set CLOEXEC to prevent the descriptor leaking to child processes */
181+
if (0 > fcntl(fpm_globals.listening_socket, F_SETFD, attrs | FD_CLOEXEC)) {
182+
zlog(ZLOG_WARNING, "failed to change attribute of listening socket");
183+
return -1;
184+
}
185+
186+
return 0;
187+
}
188+
170189
static void fpm_child_init(struct fpm_worker_pool_s *wp) /* {{{ */
171190
{
172191
fpm_globals.max_requests = wp->config->pm_max_requests;
@@ -178,7 +197,8 @@ static void fpm_child_init(struct fpm_worker_pool_s *wp) /* {{{ */
178197
0 > fpm_unix_init_child(wp) ||
179198
0 > fpm_signals_init_child() ||
180199
0 > fpm_env_init_child(wp) ||
181-
0 > fpm_php_init_child(wp)) {
200+
0 > fpm_php_init_child(wp) ||
201+
0 > fpm_child_cloexec()) {
182202

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

0 commit comments

Comments
 (0)