From bdf24f8d6d9d495ece354d6fd2dd6ed169198a2e Mon Sep 17 00:00:00 2001
From: Maksim Nikulin <mnikulin@plesk.com>
Date: Wed, 23 Jan 2019 12:19:29 +0700
Subject: [PATCH] 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
---
 NEWS                                          |  4 ++
 sapi/fpm/fpm/fpm_events.c                     | 15 +++++-
 .../fpm/tests/bug77185-reload-robustness.phpt | 46 +++++++++++++++++++
 3 files changed, 64 insertions(+), 1 deletion(-)
 create mode 100644 sapi/fpm/tests/bug77185-reload-robustness.phpt

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();
+?>
-- 
2.40.0