summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMaksim Nikulin <mnikulin@plesk.com>2019-01-23 12:19:29 +0700
committerNikita Popov <nikita.ppv@gmail.com>2019-07-22 10:32:58 +0200
commitbdf24f8d6d9d495ece354d6fd2dd6ed169198a2e (patch)
tree221e99bc6f249ad5e831a960c1ac4eeb05bb7024
parent82f35ab0890e192e76665194a245365d5d249638 (diff)
downloadphp-git-bdf24f8d6d9d495ece354d6fd2dd6ed169198a2e.tar.gz
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
-rw-r--r--NEWS4
-rw-r--r--sapi/fpm/fpm/fpm_events.c15
-rw-r--r--sapi/fpm/tests/bug77185-reload-robustness.phpt46
3 files changed, 64 insertions, 1 deletions
diff --git a/NEWS b/NEWS
index 721773e4b8..63389a0b6b 100644
--- a/NEWS
+++ b/NEWS
@@ -6,6 +6,10 @@ PHP NEWS
. Fixed bug #77946 (Bad cURL resources returned by curl_multi_info_read()).
(Abyr Valg)
+- FPM:
+ . Fixed bug #77185 (Use-after-free in FPM master event handling).
+ (Maksim Nikulin)
+
- Standard:
. Fixed bug #69100 (Bus error from stream_copy_to_stream (file -> SSL stream)
with invalid length). (Nikita)
diff --git a/sapi/fpm/fpm/fpm_events.c b/sapi/fpm/fpm/fpm_events.c
index 1750fab103..81db9f8867 100644
--- a/sapi/fpm/fpm/fpm_events.c
+++ b/sapi/fpm/fpm/fpm_events.c
@@ -34,6 +34,7 @@
#define fpm_event_set_timeout(ev, now) timeradd(&(now), &(ev)->frequency, &(ev)->timeout);
static void fpm_event_cleanup(int which, void *arg);
+static void fpm_postponed_children_bury(struct fpm_event_s *ev, short which, void *arg);
static void fpm_got_signal(struct fpm_event_s *ev, short which, void *arg);
static struct fpm_event_s *fpm_event_queue_isset(struct fpm_event_queue_s *queue, struct fpm_event_s *ev);
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);
static struct fpm_event_module_s *module;
static struct fpm_event_queue_s *fpm_event_queue_timer = NULL;
static struct fpm_event_queue_s *fpm_event_queue_fd = NULL;
+static struct fpm_event_s children_bury_timer;
static void fpm_event_cleanup(int which, void *arg) /* {{{ */
{
@@ -51,6 +53,12 @@ static void fpm_event_cleanup(int which, void *arg) /* {{{ */
}
/* }}} */
+static void fpm_postponed_children_bury(struct fpm_event_s *ev, short which, void *arg) /* {{{ */
+{
+ fpm_children_bury();
+}
+/* }}} */
+
static void fpm_got_signal(struct fpm_event_s *ev, short which, void *arg) /* {{{ */
{
char c;
@@ -72,7 +80,12 @@ static void fpm_got_signal(struct fpm_event_s *ev, short which, void *arg) /* {{
switch (c) {
case 'C' : /* SIGCHLD */
zlog(ZLOG_DEBUG, "received SIGCHLD");
- fpm_children_bury();
+ /* epoll_wait() may report signal fd before read events for a finished child
+ * in the same bunch of events. Prevent immediate free of the child structure
+ * and so the fpm_event_s instance. Otherwise use after free happens during
+ * attemp to process following read event. */
+ fpm_event_set_timer(&children_bury_timer, 0, &fpm_postponed_children_bury, NULL);
+ fpm_event_add(&children_bury_timer, 0);
break;
case 'I' : /* SIGINT */
zlog(ZLOG_DEBUG, "received SIGINT");
diff --git a/sapi/fpm/tests/bug77185-reload-robustness.phpt b/sapi/fpm/tests/bug77185-reload-robustness.phpt
new file mode 100644
index 0000000000..3780176416
--- /dev/null
+++ b/sapi/fpm/tests/bug77185-reload-robustness.phpt
@@ -0,0 +1,46 @@
+--TEST--
+FPM: bug77185 - Reload robustness
+--SKIPIF--
+<?php include "skipif.inc"; ?>
+--FILE--
+<?php
+
+require_once "tester.inc";
+
+$workers = 10;
+$loops = 10;
+
+$cfg = <<<EOT
+[global]
+error_log = {{FILE:LOG}}
+pid = {{FILE:PID}}
+[unconfined]
+listen = {{ADDR}}
+pm = static
+pm.max_children = $workers
+catch_workers_output = true
+EOT;
+
+$tester = new FPM\Tester($cfg);
+$tester->start();
+$tester->expectLogStartNotices();
+for ($i = 0; $i < $loops; $i++) {
+ $tester->signal('USR2');
+ $tester->expectLogNotice('Reloading in progress ...');
+ $tester->expectLogNotice('reloading: .*');
+ $tester->expectLogNotice('using inherited socket fd=\d+, "127.0.0.1:\d+"');
+ $tester->expectLogStartNotices();
+}
+$tester->terminate();
+$tester->expectLogTerminatingNotices();
+$tester->close();
+
+?>
+Done
+--EXPECT--
+Done
+--CLEAN--
+<?php
+require_once "tester.inc";
+FPM\Tester::clean();
+?>