From 102953735c13943694ece1698676426c88392104 Mon Sep 17 00:00:00 2001
From: Jakub Zelenka <bukka@php.net>
Date: Sat, 15 Apr 2023 16:07:09 +0100
Subject: [PATCH] Fix GH-10461: Postpone FPM child freeing in event loop

This is to prevent after free accessing of the child event that might
happen when child is killed and the message is delivered at that same
time.

Also fixes GH-10889 and properly fixes GH-8517 that was not previously
fixed correctly.
---
 NEWS                           |  4 ++++
 sapi/fpm/fpm/fpm_children.c    | 25 ++++++++++++++++++++++++-
 sapi/fpm/fpm/fpm_children.h    |  5 +++--
 sapi/fpm/fpm/fpm_process_ctl.c |  2 +-
 sapi/fpm/fpm/fpm_stdio.c       | 10 ++++------
 5 files changed, 36 insertions(+), 10 deletions(-)

diff --git a/NEWS b/NEWS
index a835815dd17..c01d4d36dfa 100644
--- a/NEWS
+++ b/NEWS
@@ -14,6 +14,10 @@ PHP                                                                        NEWS
   . Fixed bug GH-10834 (exif_read_data() cannot read smaller stream wrapper
     chunk sizes). (nielsdos)
 
+- FPM:
+  . Fixed bug GH-10461 (PHP-FPM segfault due to after free usage of
+    child->ev_std(out|err)). (Jakub Zelenka)
+
 - Hash:
   . Fixed bug GH-11180 (hash_file() appears to be restricted to 3 arguments).
     (nielsdos)
diff --git a/sapi/fpm/fpm/fpm_children.c b/sapi/fpm/fpm/fpm_children.c
index 2f8e3dc4d0a..1c9780e3de3 100644
--- a/sapi/fpm/fpm/fpm_children.c
+++ b/sapi/fpm/fpm/fpm_children.c
@@ -63,10 +63,27 @@ static void fpm_child_free(struct fpm_child_s *child) /* {{{ */
 }
 /* }}} */
 
+static void fpm_postponed_child_free(struct fpm_event_s *ev, short which, void *arg)
+{
+	struct fpm_child_s *child = (struct fpm_child_s *) arg;
+
+	if (child->fd_stdout != -1) {
+		fpm_event_del(&child->ev_stdout);
+		close(child->fd_stdout);
+	}
+	if (child->fd_stderr != -1) {
+		fpm_event_del(&child->ev_stderr);
+		close(child->fd_stderr);
+	}
+
+	fpm_child_free((struct fpm_child_s *) child);
+}
+
 static void fpm_child_close(struct fpm_child_s *child, int in_event_loop) /* {{{ */
 {
 	if (child->fd_stdout != -1) {
 		if (in_event_loop) {
+			child->postponed_free = true;
 			fpm_event_fire(&child->ev_stdout);
 		}
 		if (child->fd_stdout != -1) {
@@ -76,6 +93,7 @@ static void fpm_child_close(struct fpm_child_s *child, int in_event_loop) /* {{{
 
 	if (child->fd_stderr != -1) {
 		if (in_event_loop) {
+			child->postponed_free = true;
 			fpm_event_fire(&child->ev_stderr);
 		}
 		if (child->fd_stderr != -1) {
@@ -83,7 +101,12 @@ static void fpm_child_close(struct fpm_child_s *child, int in_event_loop) /* {{{
 		}
 	}
 
-	fpm_child_free(child);
+	if (in_event_loop && child->postponed_free) {
+		fpm_event_set_timer(&child->ev_free, 0, &fpm_postponed_child_free, child);
+		fpm_event_add(&child->ev_free, 1000);
+	} else {
+		fpm_child_free(child);
+	}
 }
 /* }}} */
 
diff --git a/sapi/fpm/fpm/fpm_children.h b/sapi/fpm/fpm/fpm_children.h
index 679c34ba038..fe06eb3ba84 100644
--- a/sapi/fpm/fpm/fpm_children.h
+++ b/sapi/fpm/fpm/fpm_children.h
@@ -23,12 +23,13 @@ struct fpm_child_s {
 	struct fpm_child_s *prev, *next;
 	struct timeval started;
 	struct fpm_worker_pool_s *wp;
-	struct fpm_event_s ev_stdout, ev_stderr;
+	struct fpm_event_s ev_stdout, ev_stderr, ev_free;
 	int shm_slot_i;
 	int fd_stdout, fd_stderr;
 	void (*tracer)(struct fpm_child_s *);
 	struct timeval slow_logged;
-	int idle_kill;
+	bool idle_kill;
+	bool postponed_free;
 	pid_t pid;
 	int scoreboard_i;
 	struct zlog_stream *log_stream;
diff --git a/sapi/fpm/fpm/fpm_process_ctl.c b/sapi/fpm/fpm/fpm_process_ctl.c
index 48eb0003d49..7a55d98b046 100644
--- a/sapi/fpm/fpm/fpm_process_ctl.c
+++ b/sapi/fpm/fpm/fpm_process_ctl.c
@@ -318,7 +318,7 @@ static void fpm_pctl_kill_idle_child(struct fpm_child_s *child) /* {{{ */
 	if (child->idle_kill) {
 		fpm_pctl_kill(child->pid, FPM_PCTL_KILL);
 	} else {
-		child->idle_kill = 1;
+		child->idle_kill = true;
 		fpm_pctl_kill(child->pid, FPM_PCTL_QUIT);
 	}
 }
diff --git a/sapi/fpm/fpm/fpm_stdio.c b/sapi/fpm/fpm/fpm_stdio.c
index 78326924acd..8f71e8cbfcd 100644
--- a/sapi/fpm/fpm/fpm_stdio.c
+++ b/sapi/fpm/fpm/fpm_stdio.c
@@ -181,10 +181,7 @@ static void fpm_stdio_child_said(struct fpm_event_s *ev, short which, void *arg)
 	if (!arg) {
 		return;
 	}
-	child = fpm_child_find((intptr_t) arg);
-	if (!child) {
-		return;
-	}
+	child = (struct fpm_child_s *) arg;
 
 	is_stdout = (fd == child->fd_stdout);
 	if (is_stdout) {
@@ -277,6 +274,7 @@ stdio_read:
 
 		fpm_event_del(event);
 
+		child->postponed_free = true;
 		if (is_stdout) {
 			close(child->fd_stdout);
 			child->fd_stdout = -1;
@@ -330,10 +328,10 @@ int fpm_stdio_parent_use_pipes(struct fpm_child_s *child) /* {{{ */
 	child->fd_stdout = fd_stdout[0];
 	child->fd_stderr = fd_stderr[0];
 
-	fpm_event_set(&child->ev_stdout, child->fd_stdout, FPM_EV_READ, fpm_stdio_child_said, (void *) (intptr_t) child->pid);
+	fpm_event_set(&child->ev_stdout, child->fd_stdout, FPM_EV_READ, fpm_stdio_child_said, child);
 	fpm_event_add(&child->ev_stdout, 0);
 
-	fpm_event_set(&child->ev_stderr, child->fd_stderr, FPM_EV_READ, fpm_stdio_child_said, (void *) (intptr_t) child->pid);
+	fpm_event_set(&child->ev_stderr, child->fd_stderr, FPM_EV_READ, fpm_stdio_child_said, child);
 	fpm_event_add(&child->ev_stderr, 0);
 	return 0;
 }