Skip to content

Commit bdf24f8

Browse files
plmnikulinnikic
authored andcommitted
Prevent use after free in fpm_event_epoll_wait
epoll event backend does not guarantee that child input/output events are reported before SIGCHILD due to finished worker. While a bunch of events received by epoll is being processed, child-related structures may be removed before dispatching of an I/O event for the same child. The result may be attempt to access to memory region allocated for another purpose, segfault of the master process, and unavailable web sites. Postpone processing of SIGCHILD events till other events in the same bunch are processed. Fix Bug #62418 php-fpm master process crashes Fix Bug #65398 Race condition between SIGCHLD and child stdout/stderr event leads to segfault Fix Bug #75112 php-fpm crashing, hard to reproduce Fix Bug #77114 php-fpm master segfaults in fpm_event_epoll_wait/fpm_event_fire Fix Bug #77185 Use-after-free in FPM master event handling
1 parent 82f35ab commit bdf24f8

File tree

3 files changed

+64
-1
lines changed

3 files changed

+64
-1
lines changed

NEWS

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@ PHP NEWS
66
. Fixed bug #77946 (Bad cURL resources returned by curl_multi_info_read()).
77
(Abyr Valg)
88

9+
- FPM:
10+
. Fixed bug #77185 (Use-after-free in FPM master event handling).
11+
(Maksim Nikulin)
12+
913
- Standard:
1014
. Fixed bug #69100 (Bus error from stream_copy_to_stream (file -> SSL stream)
1115
with invalid length). (Nikita)

sapi/fpm/fpm/fpm_events.c

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
#define fpm_event_set_timeout(ev, now) timeradd(&(now), &(ev)->frequency, &(ev)->timeout);
3535

3636
static void fpm_event_cleanup(int which, void *arg);
37+
static void fpm_postponed_children_bury(struct fpm_event_s *ev, short which, void *arg);
3738
static void fpm_got_signal(struct fpm_event_s *ev, short which, void *arg);
3839
static struct fpm_event_s *fpm_event_queue_isset(struct fpm_event_queue_s *queue, struct fpm_event_s *ev);
3940
static int fpm_event_queue_add(struct fpm_event_queue_s **queue, struct fpm_event_s *ev);
@@ -43,6 +44,7 @@ static void fpm_event_queue_destroy(struct fpm_event_queue_s **queue);
4344
static struct fpm_event_module_s *module;
4445
static struct fpm_event_queue_s *fpm_event_queue_timer = NULL;
4546
static struct fpm_event_queue_s *fpm_event_queue_fd = NULL;
47+
static struct fpm_event_s children_bury_timer;
4648

4749
static void fpm_event_cleanup(int which, void *arg) /* {{{ */
4850
{
@@ -51,6 +53,12 @@ static void fpm_event_cleanup(int which, void *arg) /* {{{ */
5153
}
5254
/* }}} */
5355

56+
static void fpm_postponed_children_bury(struct fpm_event_s *ev, short which, void *arg) /* {{{ */
57+
{
58+
fpm_children_bury();
59+
}
60+
/* }}} */
61+
5462
static void fpm_got_signal(struct fpm_event_s *ev, short which, void *arg) /* {{{ */
5563
{
5664
char c;
@@ -72,7 +80,12 @@ static void fpm_got_signal(struct fpm_event_s *ev, short which, void *arg) /* {{
7280
switch (c) {
7381
case 'C' : /* SIGCHLD */
7482
zlog(ZLOG_DEBUG, "received SIGCHLD");
75-
fpm_children_bury();
83+
/* epoll_wait() may report signal fd before read events for a finished child
84+
* in the same bunch of events. Prevent immediate free of the child structure
85+
* and so the fpm_event_s instance. Otherwise use after free happens during
86+
* attemp to process following read event. */
87+
fpm_event_set_timer(&children_bury_timer, 0, &fpm_postponed_children_bury, NULL);
88+
fpm_event_add(&children_bury_timer, 0);
7689
break;
7790
case 'I' : /* SIGINT */
7891
zlog(ZLOG_DEBUG, "received SIGINT");
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
--TEST--
2+
FPM: bug77185 - Reload robustness
3+
--SKIPIF--
4+
<?php include "skipif.inc"; ?>
5+
--FILE--
6+
<?php
7+
8+
require_once "tester.inc";
9+
10+
$workers = 10;
11+
$loops = 10;
12+
13+
$cfg = <<<EOT
14+
[global]
15+
error_log = {{FILE:LOG}}
16+
pid = {{FILE:PID}}
17+
[unconfined]
18+
listen = {{ADDR}}
19+
pm = static
20+
pm.max_children = $workers
21+
catch_workers_output = true
22+
EOT;
23+
24+
$tester = new FPM\Tester($cfg);
25+
$tester->start();
26+
$tester->expectLogStartNotices();
27+
for ($i = 0; $i < $loops; $i++) {
28+
$tester->signal('USR2');
29+
$tester->expectLogNotice('Reloading in progress ...');
30+
$tester->expectLogNotice('reloading: .*');
31+
$tester->expectLogNotice('using inherited socket fd=\d+, "127.0.0.1:\d+"');
32+
$tester->expectLogStartNotices();
33+
}
34+
$tester->terminate();
35+
$tester->expectLogTerminatingNotices();
36+
$tester->close();
37+
38+
?>
39+
Done
40+
--EXPECT--
41+
Done
42+
--CLEAN--
43+
<?php
44+
require_once "tester.inc";
45+
FPM\Tester::clean();
46+
?>

0 commit comments

Comments
 (0)